Skip to content

Commit 51d5379

Browse files
jasontedorjasontedor
authored
Remove lenient URL parameter parsing
Today when parsing a request, Elasticsearch silently ignores incorrect (including parameters with typos) or unused parameters. This is bad as it leads to requests having unintended behavior (e.g., if a user hits the _analyze API and misspell the "tokenizer" then Elasticsearch will just use the standard analyzer, completely against intentions). This commit removes lenient URL parameter parsing. The strategy is simple: when a request is handled and a parameter is touched, we mark it as such. Before the request is actually executed, we check to ensure that all parameters have been consumed. If there are remaining parameters yet to be consumed, we fail the request with a list of the unconsumed parameters. An exception has to be made for parameters that format the response (as opposed to controlling the request); for this case, handlers are able to provide a list of parameters that should be excluded from tripping the unconsumed parameters check because those parameters will be used in formatting the response. Additionally, some inconsistencies between the parameters in the code and in the docs are corrected. Relates #20722
1 parent 2b3760f commit 51d5379

File tree

147 files changed

+1187
-770
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

147 files changed

+1187
-770
lines changed

‎client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/bulk/RestNoopBulkAction.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@
3939
import org.elasticsearch.rest.RestResponse;
4040
import org.elasticsearch.rest.action.RestBuilderListener;
4141

42+
import java.io.IOException;
43+
4244
import static org.elasticsearch.rest.RestRequest.Method.POST;
4345
import static org.elasticsearch.rest.RestRequest.Method.PUT;
4446
import static org.elasticsearch.rest.RestStatus.OK;
@@ -57,7 +59,7 @@ public RestNoopBulkAction(Settings settings, RestController controller) {
5759
}
5860

5961
@Override
60-
public void handleRequest(final RestRequest request, final RestChannel channel, final NodeClient client) throws Exception {
62+
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
6163
BulkRequest bulkRequest = Requests.bulkRequest();
6264
String defaultIndex = request.param("index");
6365
String defaultType = request.param("type");
@@ -75,9 +77,10 @@ public void handleRequest(final RestRequest request, final RestChannel channel,
7577
bulkRequest.add(request.content(), defaultIndex, defaultType, defaultRouting, defaultFields, null, defaultPipeline, null, true);
7678

7779
// short circuit the call to the transport layer
78-
BulkRestBuilderListener listener = new BulkRestBuilderListener(channel, request);
79-
listener.onResponse(bulkRequest);
80-
80+
return channel -> {
81+
BulkRestBuilderListener listener = new BulkRestBuilderListener(channel, request);
82+
listener.onResponse(bulkRequest);
83+
};
8184
}
8285

8386
private static class BulkRestBuilderListener extends RestBuilderListener<BulkRequest> {

‎client/client-benchmark-noop-api-plugin/src/main/java/org/elasticsearch/plugin/noop/action/search/RestNoopSearchAction.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.common.inject.Inject;
2424
import org.elasticsearch.common.settings.Settings;
2525
import org.elasticsearch.rest.BaseRestHandler;
26-
import org.elasticsearch.rest.RestChannel;
2726
import org.elasticsearch.rest.RestController;
2827
import org.elasticsearch.rest.RestRequest;
2928
import org.elasticsearch.rest.action.RestStatusToXContentListener;
@@ -47,8 +46,8 @@ public RestNoopSearchAction(Settings settings, RestController controller) {
4746
}
4847

4948
@Override
50-
public void handleRequest(final RestRequest request, final RestChannel channel, final NodeClient client) throws IOException {
49+
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
5150
SearchRequest searchRequest = new SearchRequest();
52-
client.execute(NoopSearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel));
51+
return channel -> client.execute(NoopSearchAction.INSTANCE, searchRequest, new RestStatusToXContentListener<>(channel));
5352
}
5453
}

‎core/src/main/java/org/elasticsearch/action/bulk/BulkRequest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,32 +247,32 @@ public long estimatedSizeInBytes() {
247247
/**
248248
* Adds a framed data in binary format
249249
*/
250-
public BulkRequest add(byte[] data, int from, int length) throws Exception {
250+
public BulkRequest add(byte[] data, int from, int length) throws IOException {
251251
return add(data, from, length, null, null);
252252
}
253253

254254
/**
255255
* Adds a framed data in binary format
256256
*/
257-
public BulkRequest add(byte[] data, int from, int length, @Nullable String defaultIndex, @Nullable String defaultType) throws Exception {
257+
public BulkRequest add(byte[] data, int from, int length, @Nullable String defaultIndex, @Nullable String defaultType) throws IOException {
258258
return add(new BytesArray(data, from, length), defaultIndex, defaultType);
259259
}
260260

261261
/**
262262
* Adds a framed data in binary format
263263
*/
264-
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType) throws Exception {
264+
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType) throws IOException {
265265
return add(data, defaultIndex, defaultType, null, null, null, null, null, true);
266266
}
267267

268268
/**
269269
* Adds a framed data in binary format
270270
*/
271-
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex) throws Exception {
271+
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, boolean allowExplicitIndex) throws IOException {
272272
return add(data, defaultIndex, defaultType, null, null, null, null, null, allowExplicitIndex);
273273
}
274274

275-
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws Exception {
275+
public BulkRequest add(BytesReference data, @Nullable String defaultIndex, @Nullable String defaultType, @Nullable String defaultRouting, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSourceContext, @Nullable String defaultPipeline, @Nullable Object payload, boolean allowExplicitIndex) throws IOException {
276276
XContent xContent = XContentFactory.xContent(data);
277277
int line = 0;
278278
int from = 0;

‎core/src/main/java/org/elasticsearch/action/get/MultiGetRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defau
335335
return add(defaultIndex, defaultType, defaultFields, defaultFetchSource, null, data, allowExplicitIndex);
336336
}
337337

338-
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, @Nullable String defaultRouting, BytesReference data, boolean allowExplicitIndex) throws Exception {
338+
public MultiGetRequest add(@Nullable String defaultIndex, @Nullable String defaultType, @Nullable String[] defaultFields, @Nullable FetchSourceContext defaultFetchSource, @Nullable String defaultRouting, BytesReference data, boolean allowExplicitIndex) throws IOException {
339339
try (XContentParser parser = XContentFactory.xContent(data).createParser(data)) {
340340
XContentParser.Token token;
341341
String currentFieldName = null;

‎core/src/main/java/org/elasticsearch/action/support/tasks/BaseTasksRequest.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public class BaseTasksRequest<Request extends BaseTasksRequest<Request>> extends
4242

4343
public static final String[] ALL_NODES = Strings.EMPTY_ARRAY;
4444

45-
private String[] nodesIds = ALL_NODES;
45+
private String[] nodes = ALL_NODES;
4646

4747
private TimeValue timeout;
4848

@@ -58,7 +58,7 @@ public BaseTasksRequest() {
5858
@Override
5959
public ActionRequestValidationException validate() {
6060
ActionRequestValidationException validationException = null;
61-
if (taskId.isSet() && nodesIds.length > 0) {
61+
if (taskId.isSet() && nodes.length > 0) {
6262
validationException = addValidationError("task id cannot be used together with node ids",
6363
validationException);
6464
}
@@ -81,13 +81,13 @@ public String[] getActions() {
8181
return actions;
8282
}
8383

84-
public final String[] getNodesIds() {
85-
return nodesIds;
84+
public final String[] getNodes() {
85+
return nodes;
8686
}
8787

8888
@SuppressWarnings("unchecked")
89-
public final Request setNodesIds(String... nodesIds) {
90-
this.nodesIds = nodesIds;
89+
public final Request setNodes(String... nodes) {
90+
this.nodes = nodes;
9191
return (Request) this;
9292
}
9393

@@ -142,7 +142,7 @@ public void readFrom(StreamInput in) throws IOException {
142142
super.readFrom(in);
143143
taskId = TaskId.readFromStream(in);
144144
parentTaskId = TaskId.readFromStream(in);
145-
nodesIds = in.readStringArray();
145+
nodes = in.readStringArray();
146146
actions = in.readStringArray();
147147
timeout = in.readOptionalWriteable(TimeValue::new);
148148
}
@@ -152,7 +152,7 @@ public void writeTo(StreamOutput out) throws IOException {
152152
super.writeTo(out);
153153
taskId.writeTo(out);
154154
parentTaskId.writeTo(out);
155-
out.writeStringArrayNullable(nodesIds);
155+
out.writeStringArrayNullable(nodes);
156156
out.writeStringArrayNullable(actions);
157157
out.writeOptionalWriteable(timeout);
158158
}

‎core/src/main/java/org/elasticsearch/action/support/tasks/TasksRequestBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public final RequestBuilder setTaskId(TaskId taskId) {
4848

4949
@SuppressWarnings("unchecked")
5050
public final RequestBuilder setNodesIds(String... nodesIds) {
51-
request.setNodesIds(nodesIds);
51+
request.setNodes(nodesIds);
5252
return (RequestBuilder) this;
5353
}
5454

‎core/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ protected String[] resolveNodes(TasksRequest request, ClusterState clusterState)
125125
if (request.getTaskId().isSet()) {
126126
return new String[]{request.getTaskId().getNodeId()};
127127
} else {
128-
return clusterState.nodes().resolveNodes(request.getNodesIds());
128+
return clusterState.nodes().resolveNodes(request.getNodes());
129129
}
130130
}
131131

‎core/src/main/java/org/elasticsearch/action/termvectors/MultiTermVectorsRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ public List<TermVectorsRequest> getRequests() {
9494
return requests;
9595
}
9696

97-
public void add(TermVectorsRequest template, BytesReference data) throws Exception {
97+
public void add(TermVectorsRequest template, BytesReference data) throws IOException {
9898
XContentParser.Token token;
9999
String currentFieldName = null;
100100
if (data.length() > 0) {

‎core/src/main/java/org/elasticsearch/action/update/UpdateRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ public boolean detectNoop() {
715715
return detectNoop;
716716
}
717717

718-
public UpdateRequest fromXContent(BytesReference source) throws Exception {
718+
public UpdateRequest fromXContent(BytesReference source) throws IOException {
719719
Script script = null;
720720
try (XContentParser parser = XContentFactory.xContent(source).createParser(source)) {
721721
XContentParser.Token token = parser.nextToken();

‎core/src/main/java/org/elasticsearch/common/settings/Settings.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
582582
return builder;
583583
}
584584

585+
public static final Set<String> FORMAT_PARAMS =
586+
Collections.unmodifiableSet(new HashSet<>(Arrays.asList("settings_filter", "flat_settings")));
587+
585588
/**
586589
* Returns <tt>true</tt> if this settings object contains no settings
587590
* @return <tt>true</tt> if this settings object contains no settings

0 commit comments

Comments
 (0)