Skip to content

Commit

Permalink
Ignore BaseRestHandler unconsumed content check as it's always consum…
Browse files Browse the repository at this point in the history
…ed (opensearch-project#13290)

* Ignore BaseRestHandler unconsumed content check as it's always consumed

Signed-off-by: Daniel Widdis <[email protected]>

* Remove comment, continue to ignore content on Force Merge

Signed-off-by: Daniel Widdis <[email protected]>

* Remove no-body test from RestDeletePitActionTests

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
  • Loading branch information
dbwiddis authored Apr 19, 2024
1 parent f5c3ef9 commit 0282e64
Show file tree
Hide file tree
Showing 5 changed files with 1 addition and 130 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Enabled mockTelemetryPlugin for IT and fixed OOM issues ([#13054](https://github.com/opensearch-project/OpenSearch/pull/13054))
- Fix implement mark() and markSupported() in class FilterStreamInput ([#13098](https://github.com/opensearch-project/OpenSearch/pull/13098))
- Fix snapshot _status API to return correct status for partial snapshots ([#12812](https://github.com/opensearch-project/OpenSearch/pull/12812))
- Ignore BaseRestHandler unconsumed content check as it's always consumed. ([#13290](https://github.com/opensearch-project/OpenSearch/pull/13290))

### Security

Expand Down
4 changes: 0 additions & 4 deletions server/src/main/java/org/opensearch/rest/BaseRestHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,6 @@ public final void handleRequest(RestRequest request, RestChannel channel, NodeCl
throw new IllegalArgumentException(unrecognized(request, unconsumedParams, candidateParams, "parameter"));
}

if (request.hasContent() && request.isContentConsumed() == false) {
throw new IllegalArgumentException("request [" + request.method() + " " + request.path() + "] does not support having a body");
}

usageCount.increment();
// execute the action
action.accept(channel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,45 +32,23 @@

package org.opensearch.action.admin.indices.forcemerge;

import org.opensearch.client.node.NodeClient;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.rest.RestRequest;
import org.opensearch.rest.action.admin.indices.RestForceMergeAction;
import org.opensearch.test.rest.FakeRestChannel;
import org.opensearch.test.rest.FakeRestRequest;
import org.opensearch.test.rest.RestActionTestCase;
import org.junit.Before;

import java.util.HashMap;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.mock;

public class RestForceMergeActionTests extends RestActionTestCase {

@Before
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),
MediaTypeRegistry.JSON
).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"));
}

public void testDeprecationMessage() {
final Map<String, String> params = new HashMap<>();
params.put("only_expunge_deletes", Boolean.TRUE.toString());
Expand Down
79 changes: 0 additions & 79 deletions server/src/test/java/org/opensearch/rest/BaseRestHandlerTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@
import org.opensearch.client.node.NodeClient;
import org.opensearch.common.Table;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.rest.RestHandler.ReplacedRoute;
import org.opensearch.rest.RestHandler.Route;
import org.opensearch.rest.RestRequest.Method;
Expand Down Expand Up @@ -281,81 +277,6 @@ public String getName() {
assertTrue(executed.get());
}

public void testConsumedBody() throws Exception {
final AtomicBoolean executed = new AtomicBoolean();
final BaseRestHandler handler = new BaseRestHandler() {
@Override
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
request.content();
return channel -> executed.set(true);
}

@Override
public String getName() {
return "test_consumed_body";
}
};

try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
new BytesArray(builder.toString()),
MediaTypeRegistry.JSON
).build();
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
handler.handleRequest(request, channel, mockClient);
assertTrue(executed.get());
}
}

public void testUnconsumedNoBody() throws Exception {
final AtomicBoolean executed = new AtomicBoolean();
final BaseRestHandler handler = new BaseRestHandler() {
@Override
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
return channel -> executed.set(true);
}

@Override
public String getName() {
return "test_unconsumed_body";
}
};

final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).build();
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
handler.handleRequest(request, channel, mockClient);
assertTrue(executed.get());
}

public void testUnconsumedBody() throws IOException {
final AtomicBoolean executed = new AtomicBoolean();
final BaseRestHandler handler = new BaseRestHandler() {
@Override
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
return channel -> executed.set(true);
}

@Override
public String getName() {
return "test_unconsumed_body";
}
};

try (XContentBuilder builder = JsonXContent.contentBuilder().startObject().endObject()) {
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
new BytesArray(builder.toString()),
MediaTypeRegistry.JSON
).build();
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> handler.handleRequest(request, channel, mockClient)
);
assertThat(e, hasToString(containsString("request [GET /] does not support having a body")));
assertFalse(executed.get());
}
}

public void testReplaceRoutesMethod() throws Exception {
List<Route> routes = Arrays.asList(new Route(Method.GET, "/path/test"), new Route(Method.PUT, "/path2/test"));
List<ReplacedRoute> replacedRoutes = RestHandler.replaceRoutes(routes, "/prefix", "/deprecatedPrefix");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,31 +82,6 @@ public void deletePits(DeletePitRequest request, ActionListener<DeletePitRespons
}
}

public void testDeleteAllPitWithBody() {
SetOnce<Boolean> pitCalled = new SetOnce<>();
try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {
@Override
public void deletePits(DeletePitRequest request, ActionListener<DeletePitResponse> listener) {
pitCalled.set(true);
assertThat(request.getPitIds(), hasSize(1));
assertThat(request.getPitIds().get(0), equalTo("_all"));
}
}) {
RestDeletePitAction action = new RestDeletePitAction();
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withContent(
new BytesArray("{\"pit_id\": [\"BODY\"]}"),
MediaTypeRegistry.JSON
).withPath("/_all").build();
FakeRestChannel channel = new FakeRestChannel(request, false, 0);

IllegalArgumentException ex = expectThrows(
IllegalArgumentException.class,
() -> action.handleRequest(request, channel, nodeClient)
);
assertTrue(ex.getMessage().contains("request [GET /_all] does not support having a body"));
}
}

public void testDeletePitQueryStringParamsShouldThrowException() {
SetOnce<Boolean> pitCalled = new SetOnce<>();
try (NodeClient nodeClient = new NoOpNodeClient(this.getTestName()) {
Expand Down

0 comments on commit 0282e64

Please sign in to comment.