diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RollupRequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RollupRequestConverters.java index 48d297a5eae35..ac7b59040718e 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RollupRequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RollupRequestConverters.java @@ -90,9 +90,7 @@ static Request deleteJob(final DeleteRollupJobRequest deleteRollupJobRequest) th .addPathPartAsIs("_rollup", "job") .addPathPart(deleteRollupJobRequest.getId()) .build(); - Request request = new Request(HttpDelete.METHOD_NAME, endpoint); - request.setEntity(createEntity(deleteRollupJobRequest, REQUEST_BODY_CONTENT_TYPE)); - return request; + return new Request(HttpDelete.METHOD_NAME, endpoint); } static Request search(final SearchRequest request) throws IOException { @@ -114,9 +112,7 @@ static Request getRollupCaps(final GetRollupCapsRequest getRollupCapsRequest) th .addPathPartAsIs("_rollup", "data") .addPathPart(getRollupCapsRequest.getIndexPattern()) .build(); - Request request = new Request(HttpGet.METHOD_NAME, endpoint); - request.setEntity(createEntity(getRollupCapsRequest, REQUEST_BODY_CONTENT_TYPE)); - return request; + return new Request(HttpGet.METHOD_NAME, endpoint); } static Request getRollupIndexCaps(final GetRollupIndexCapsRequest getRollupIndexCapsRequest) throws IOException { @@ -124,8 +120,6 @@ static Request getRollupIndexCaps(final GetRollupIndexCapsRequest getRollupIndex .addCommaSeparatedPathParts(getRollupIndexCapsRequest.indices()) .addPathPartAsIs("_rollup", "data") .build(); - Request request = new Request(HttpGet.METHOD_NAME, endpoint); - request.setEntity(createEntity(getRollupIndexCapsRequest, REQUEST_BODY_CONTENT_TYPE)); - return request; + return new Request(HttpGet.METHOD_NAME, endpoint); } } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/DeleteRollupJobRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/DeleteRollupJobRequest.java index 9b7a322b23827..7302dbca6916c 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/DeleteRollupJobRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/DeleteRollupJobRequest.java @@ -19,22 +19,14 @@ package org.elasticsearch.client.rollup; import org.elasticsearch.client.Validatable; -import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.ToXContentObject; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; -import java.io.IOException; import java.util.Objects; -public class DeleteRollupJobRequest implements Validatable, ToXContentObject { +public class DeleteRollupJobRequest implements Validatable { - private static final ParseField ID_FIELD = new ParseField("id"); private final String id; - public DeleteRollupJobRequest(String id) { this.id = Objects.requireNonNull(id, "id parameter must not be null"); } @@ -43,27 +35,6 @@ public String getId() { return id; } - private static final ConstructingObjectParser PARSER = - new ConstructingObjectParser<>("request", a -> { - return new DeleteRollupJobRequest((String) a[0]); - }); - - static { - PARSER.declareString(ConstructingObjectParser.constructorArg(), ID_FIELD); - } - - public static DeleteRollupJobRequest fromXContent(XContentParser parser) { - return PARSER.apply(parser, null); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(ID_FIELD.getPreferredName(), this.id); - builder.endObject(); - return builder; - } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsRequest.java index b07089cf13876..44593c3d66952 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupCapsRequest.java @@ -21,14 +21,11 @@ import org.elasticsearch.client.Validatable; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.xcontent.ToXContentObject; -import org.elasticsearch.common.xcontent.XContentBuilder; -import java.io.IOException; import java.util.Objects; -public class GetRollupCapsRequest implements Validatable, ToXContentObject { - private static final String ID = "id"; +public class GetRollupCapsRequest implements Validatable { + private final String indexPattern; public GetRollupCapsRequest(final String indexPattern) { @@ -43,14 +40,6 @@ public String getIndexPattern() { return indexPattern; } - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(ID, indexPattern); - builder.endObject(); - return builder; - } - @Override public int hashCode() { return Objects.hash(indexPattern); diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupIndexCapsRequest.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupIndexCapsRequest.java index 6bb0c9b48d614..bbd341b5daf6d 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupIndexCapsRequest.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/rollup/GetRollupIndexCapsRequest.java @@ -21,16 +21,11 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Validatable; import org.elasticsearch.common.Strings; -import org.elasticsearch.common.xcontent.ToXContentObject; -import org.elasticsearch.common.xcontent.XContentBuilder; -import java.io.IOException; import java.util.Arrays; import java.util.Objects; -public class GetRollupIndexCapsRequest implements Validatable, ToXContentObject { - private static final String INDICES = "indices"; - private static final String INDICES_OPTIONS = "indices_options"; +public class GetRollupIndexCapsRequest implements Validatable { private String[] indices; private IndicesOptions options; @@ -60,21 +55,6 @@ public String[] indices() { return indices; } - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - { - builder.array(INDICES, indices); - builder.startObject(INDICES_OPTIONS); - { - options.toXContent(builder, params); - } - builder.endObject(); - } - builder.endObject(); - return builder; - } - @Override public int hashCode() { return Objects.hash(Arrays.hashCode(indices), options); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/DeleteRollupJobRequestTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/DeleteRollupJobRequestTests.java index c1271207d41bc..c4a53c4d14c69 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/DeleteRollupJobRequestTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/rollup/DeleteRollupJobRequestTests.java @@ -18,39 +18,12 @@ */ package org.elasticsearch.client.rollup; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.test.AbstractXContentTestCase; -import org.junit.Before; +import org.elasticsearch.test.ESTestCase; -import java.io.IOException; - -public class DeleteRollupJobRequestTests extends AbstractXContentTestCase { - - private String jobId; - - @Before - public void setUpOptionalId() { - jobId = randomAlphaOfLengthBetween(1, 10); - } - - @Override - protected DeleteRollupJobRequest createTestInstance() { - return new DeleteRollupJobRequest(jobId); - } - - @Override - protected DeleteRollupJobRequest doParseInstance(final XContentParser parser) throws IOException { - return DeleteRollupJobRequest.fromXContent(parser); - } - - @Override - protected boolean supportsUnknownFields() { - return false; - } +public class DeleteRollupJobRequestTests extends ESTestCase { public void testRequireConfiguration() { final NullPointerException e = expectThrows(NullPointerException.class, ()-> new DeleteRollupJobRequest(null)); assertEquals("id parameter must not be null", e.getMessage()); } - } diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 730d9d85b140e..059d085af6222 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -222,7 +222,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th } private void dispatchRequest(RestRequest request, RestChannel channel, RestHandler handler) throws Exception { - final int contentLength = request.content().length(); + final int contentLength = request.contentLength(); if (contentLength > 0) { final XContentType xContentType = request.getXContentType(); if (xContentType == null) { diff --git a/server/src/main/java/org/elasticsearch/rest/RestRequest.java b/server/src/main/java/org/elasticsearch/rest/RestRequest.java index eb7dcc24d555f..512bf72e9c0d3 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestRequest.java +++ b/server/src/main/java/org/elasticsearch/rest/RestRequest.java @@ -209,15 +209,15 @@ public final String path() { } public boolean hasContent() { - return content(false).length() > 0; + return contentLength() > 0; } - public BytesReference content() { - return content(true); + public int contentLength() { + return httpRequest.content().length(); } - protected BytesReference content(final boolean contentConsumed) { - this.contentConsumed = this.contentConsumed | contentConsumed; + public BytesReference content() { + this.contentConsumed = true; return httpRequest.content(); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java index 102dcbcfa11fc..f827d592c3429 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java @@ -19,14 +19,19 @@ package org.elasticsearch.action.admin.indices.forcemerge; -import org.elasticsearch.client.node.NodeClient; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.rest.AbstractRestChannel; +import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestResponse; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.action.admin.indices.RestForceMergeAction; -import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; @@ -34,8 +39,9 @@ import java.util.HashMap; import java.util.Map; -import static org.hamcrest.Matchers.equalTo; -import static org.mockito.Mockito.mock; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; public class RestForceMergeActionTests extends RestActionTestCase { @@ -44,16 +50,27 @@ public void setUpAction() { controller().registerHandler(new RestForceMergeAction()); } + public void testBodyRejection() throws Exception { - final RestForceMergeAction handler = new RestForceMergeAction(); String json = JsonXContent.contentBuilder().startObject().field("max_num_segments", 1).endObject().toString(); final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withContent(new BytesArray(json), XContentType.JSON) + .withMethod(RestRequest.Method.POST) .withPath("/_forcemerge") .build(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> handler.handleRequest(request, new FakeRestChannel(request, randomBoolean(), 1), mock(NodeClient.class))); - assertThat(e.getMessage(), equalTo("request [GET /_forcemerge] does not support having a body")); + + final SetOnce responseSetOnce = new SetOnce<>(); + dispatchRequest(request, new AbstractRestChannel(request, true) { + @Override + public void sendResponse(RestResponse response) { + responseSetOnce.set(response); + } + }); + + final RestResponse response = responseSetOnce.get(); + assertThat(response, notNullValue()); + assertThat(response.status(), is(RestStatus.BAD_REQUEST)); + assertThat(response.content().utf8ToString(), containsString("request [POST /_forcemerge] does not support having a body")); } public void testDeprecationMessage() { @@ -75,4 +92,9 @@ public void testDeprecationMessage() { assertWarnings("setting only_expunge_deletes and max_num_segments at the same time is deprecated " + "and will be rejected in a future version"); } + + protected void dispatchRequest(final RestRequest request, final RestChannel channel) { + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + controller().dispatchRequest(request, channel, threadContext); + } } diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 0e8191f38549d..2d290cf7e0a8c 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.yaml.YamlXContent; import org.elasticsearch.core.internal.io.IOUtils; @@ -482,6 +483,38 @@ public void testDispatchBadRequest() { assertThat(channel.getRestResponse().content().utf8ToString(), containsString("bad request")); } + public void testDoesNotConsumeContent() throws Exception { + final RestRequest.Method method = randomFrom(RestRequest.Method.values()); + restController.registerHandler(method, "/notconsumed", new RestHandler() { + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); + } + + @Override + public boolean canTripCircuitBreaker() { + return false; + } + }); + + final XContentBuilder content = XContentBuilder.builder(randomFrom(XContentType.values()).xContent()) + .startObject().field("field", "value").endObject(); + final FakeRestRequest restRequest = new FakeRestRequest.Builder(xContentRegistry()) + .withPath("/notconsumed") + .withMethod(method) + .withContent(BytesReference.bytes(content), content.contentType()) + .build(); + + final AssertingChannel channel = new AssertingChannel(restRequest, true, RestStatus.OK); + assertFalse(channel.getSendResponseCalled()); + assertFalse(restRequest.isContentConsumed()); + + restController.dispatchRequest(restRequest, channel, new ThreadContext(Settings.EMPTY)); + + assertTrue(channel.getSendResponseCalled()); + assertFalse("RestController must not consume request content", restRequest.isContentConsumed()); + } + public void testDispatchBadRequestUnknownCause() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); diff --git a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java index b0d9847c94732..6e12a96d8a6c2 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestRequestTests.java @@ -83,6 +83,10 @@ public void testHasContentDoesNotConsumesContent() { runConsumesContentTest(RestRequest::hasContent, false); } + public void testContentLengthDoesNotConsumesContent() { + runConsumesContentTest(RestRequest::contentLength, false); + } + private void runConsumesContentTest( final CheckedConsumer consumer, final boolean expected) { final HttpRequest httpRequest = mock(HttpRequest.class); diff --git a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestForgetFollowerAction.java b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestForgetFollowerAction.java index 2b3e71917decd..bb8246a7a780b 100644 --- a/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestForgetFollowerAction.java +++ b/x-pack/plugin/ccr/src/main/java/org/elasticsearch/xpack/ccr/rest/RestForgetFollowerAction.java @@ -9,11 +9,10 @@ import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.rest.BaseRestHandler; -import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.action.RestToXContentListener; import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction; +import org.elasticsearch.xpack.core.ccr.action.ForgetFollowerAction.Request; import java.io.IOException; import java.util.List; @@ -34,18 +33,14 @@ public String getName() { } @Override - protected RestChannelConsumer prepareRequest(final RestRequest restRequest, final NodeClient client) { - final String leaderIndex = restRequest.param("index"); - - return channel -> { - try (XContentParser parser = restRequest.contentOrSourceParamParser()) { - final ForgetFollowerAction.Request request = ForgetFollowerAction.Request.fromXContent(parser, leaderIndex); - client.execute(ForgetFollowerAction.INSTANCE, request, new RestToXContentListener<>(channel)); - } catch (final IOException e) { - channel.sendResponse(new BytesRestResponse(channel, RestStatus.INTERNAL_SERVER_ERROR, e)); - } - }; - + protected RestChannelConsumer prepareRequest(final RestRequest restRequest, final NodeClient client) throws IOException { + final Request request = createRequest(restRequest, restRequest.param("index")); + return channel -> client.execute(ForgetFollowerAction.INSTANCE, request, new RestToXContentListener<>(channel)); } + private static Request createRequest(final RestRequest restRequest, final String leaderIndex) throws IOException { + try (XContentParser parser = restRequest.contentOrSourceParamParser()) { + return Request.fromXContent(parser, leaderIndex); + } + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java index d9747f9e5d577..79a8aeb5ef1b5 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/action/user/RestChangePasswordAction.java @@ -7,6 +7,7 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.license.XPackLicenseState; @@ -79,9 +80,10 @@ public RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient c } final String refresh = request.param("refresh"); + final BytesReference content = request.requiredContent(); return channel -> new SecurityClient(client) - .prepareChangePassword(username, request.requiredContent(), request.getXContentType(), passwordHasher) + .prepareChangePassword(username, content, request.getXContentType(), passwordHasher) .setRefreshPolicy(refresh) .execute(new RestBuilderListener(channel) { @Override