Clarify wording for the strict REST params message

This commit changes the strict REST parameters message to say that
unconsumed parameters are unrecognized rather than unused. Additionally,
the test is beefed up to include two unused parameters.

Relates #20745
This commit is contained in:
Jason Tedor 2016-10-04 19:47:47 -04:00 committed by GitHub
parent 9b710e8fdd
commit 7d1e3377b8
2 changed files with 16 additions and 3 deletions

View File

@ -30,6 +30,7 @@ import org.elasticsearch.plugins.ActionPlugin;
import java.io.IOException; import java.io.IOException;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Locale;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -63,7 +64,12 @@ public abstract class BaseRestHandler extends AbstractComponent implements RestH
// validate the non-response params // validate the non-response params
if (!unconsumedParams.isEmpty()) { if (!unconsumedParams.isEmpty()) {
throw new IllegalArgumentException("request [" + request.path() + "] contains unused params: " + unconsumedParams.toString()); final String message = String.format(
Locale.ROOT,
"request [%s] contains unrecognized parameters: %s",
request.path(),
unconsumedParams.toString());
throw new IllegalArgumentException(message);
} }
// execute the action // execute the action

View File

@ -33,6 +33,7 @@ import java.util.HashMap;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicBoolean;
import static org.hamcrest.core.AnyOf.anyOf;
import static org.hamcrest.core.StringContains.containsString; import static org.hamcrest.core.StringContains.containsString;
import static org.hamcrest.object.HasToString.hasToString; import static org.hamcrest.object.HasToString.hasToString;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
@ -51,12 +52,18 @@ public class BaseRestHandlerTests extends ESTestCase {
final HashMap<String, String> params = new HashMap<>(); final HashMap<String, String> params = new HashMap<>();
params.put("consumed", randomAsciiOfLength(8)); params.put("consumed", randomAsciiOfLength(8));
params.put("unconsumed", randomAsciiOfLength(8)); params.put("unconsumed-first", randomAsciiOfLength(8));
params.put("unconsumed-second", randomAsciiOfLength(8));
RestRequest request = new FakeRestRequest.Builder().withParams(params).build(); RestRequest request = new FakeRestRequest.Builder().withParams(params).build();
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
final IllegalArgumentException e = final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class))); expectThrows(IllegalArgumentException.class, () -> handler.handleRequest(request, channel, mock(NodeClient.class)));
assertThat(e, hasToString(containsString("request [/] contains unused params: [unconsumed]"))); assertThat(
e,
// we can not rely on ordering of the unconsumed parameters here
anyOf(
hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-first, unconsumed-second]")),
hasToString(containsString("request [/] contains unrecognized parameters: [unconsumed-second, unconsumed-first]"))));
assertFalse(executed.get()); assertFalse(executed.get());
} }