Rest Controller wildcard registration (#46487)

Registering two different http methods on the same path using different
wildcard names would result in the last wildcard name being active only.
Now throw an exception instead.

Closes #46482
This commit is contained in:
Henning Andersen 2019-09-09 21:48:26 +02:00 committed by Henning Andersen
parent e21deae535
commit 9fce5a99d8
3 changed files with 29 additions and 6 deletions

View File

@ -96,7 +96,12 @@ public class PathTrie<T> {
private void updateKeyWithNamedWildcard(String key) {
this.key = key;
namedWildcard = key.substring(key.indexOf('{') + 1, key.indexOf('}'));
String newNamedWildcard = key.substring(key.indexOf('{') + 1, key.indexOf('}'));
if (namedWildcard != null && newNamedWildcard.equals(namedWildcard) == false) {
throw new IllegalArgumentException("Trying to use conflicting wildcard names for same path: "
+ namedWildcard + " and " + newNamedWildcard);
}
namedWildcard = newNamedWildcard;
}
private void addInnerChild(String key, TrieNode child) {

View File

@ -19,6 +19,7 @@
package org.elasticsearch.common.path;
import org.elasticsearch.common.path.PathTrie.TrieMatchingMode;
import org.elasticsearch.rest.RestUtils;
import org.elasticsearch.test.ESTestCase;
@ -29,8 +30,6 @@ import java.util.Map;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import org.elasticsearch.common.path.PathTrie.TrieMatchingMode;
public class PathTrieTests extends ESTestCase {
public static final PathTrie.Decoder NO_DECODER = new PathTrie.Decoder() {
@ -122,13 +121,13 @@ public class PathTrieTests extends ESTestCase {
PathTrie<String> trie = new PathTrie<>(NO_DECODER);
trie.insert("{testA}", "test1");
trie.insert("{testA}/{testB}", "test2");
trie.insert("a/{testA}", "test3");
trie.insert("a/{testB}", "test3");
trie.insert("{testA}/b", "test4");
trie.insert("{testA}/b/c", "test5");
trie.insert("a/{testB}/c", "test6");
trie.insert("a/b/{testC}", "test7");
trie.insert("{testA}/b/{testB}", "test8");
trie.insert("x/{testA}/z", "test9");
trie.insert("x/{testB}/z", "test9");
trie.insert("{testA}/{testB}/{testC}", "test10");
Map<String, String> params = new HashMap<>();
@ -196,7 +195,7 @@ public class PathTrieTests extends ESTestCase {
trie.insert("a", "test2");
trie.insert("{testA}/{testB}", "test3");
trie.insert("a/{testB}", "test4");
trie.insert("{testB}/b", "test5");
trie.insert("{testA}/b", "test5");
trie.insert("a/b", "test6");
trie.insert("{testA}/b/{testB}", "test7");
trie.insert("x/{testA}/z", "test8");

View File

@ -56,6 +56,7 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
@ -175,6 +176,24 @@ public class RestControllerTests extends ESTestCase {
verify(controller).registerAsDeprecatedHandler(deprecatedMethod, deprecatedPath, handler, deprecationMessage, logger);
}
public void testRegisterSecondMethodWithDifferentNamedWildcard() {
final RestController restController = new RestController(null, null, null, circuitBreakerService, usageService);
RestRequest.Method firstMethod = randomFrom(RestRequest.Method.values());
RestRequest.Method secondMethod =
randomFrom(Arrays.stream(RestRequest.Method.values()).filter(m -> m != firstMethod).collect(Collectors.toList()));
final String path = "/_" + randomAlphaOfLengthBetween(1, 6);
RestHandler handler = mock(RestHandler.class);
restController.registerHandler(firstMethod, path + "/{wildcard1}", handler);
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class,
() -> restController.registerHandler(secondMethod, path + "/{wildcard2}", handler));
assertThat(exception.getMessage(), equalTo("Trying to use conflicting wildcard names for same path: wildcard1 and wildcard2"));
}
public void testRestHandlerWrapper() throws Exception {
AtomicBoolean handlerCalled = new AtomicBoolean(false);
AtomicBoolean wrapperCalled = new AtomicBoolean(false);