diff --git a/core/src/main/java/org/elasticsearch/common/path/PathTrie.java b/core/src/main/java/org/elasticsearch/common/path/PathTrie.java index 3bf2a9b17ee67..c45f9e359e79f 100644 --- a/core/src/main/java/org/elasticsearch/common/path/PathTrie.java +++ b/core/src/main/java/org/elasticsearch/common/path/PathTrie.java @@ -22,8 +22,6 @@ import com.google.common.collect.ImmutableMap; import org.elasticsearch.common.Strings; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import static org.elasticsearch.common.collect.MapBuilder.newMapBuilder; @@ -33,26 +31,15 @@ */ public class PathTrie { - public static interface Decoder { + public interface Decoder { String decode(String value); } - public static final Decoder NO_DECODER = new Decoder() { - @Override - public String decode(String value) { - return value; - } - }; - private final Decoder decoder; private final TrieNode root; private final char separator; private T rootValue; - public PathTrie() { - this('/', "*", NO_DECODER); - } - public PathTrie(Decoder decoder) { this('/', "*", decoder); } @@ -197,7 +184,7 @@ public T retrieve(String[] path, int index, Map params) { private void put(Map params, TrieNode node, String value) { if (params != null && node.isNamedWildcard()) { - params.put(node.namedWildcard(), value); + params.put(node.namedWildcard(), decoder.decode(value)); } } } @@ -224,7 +211,7 @@ public T retrieve(String path, Map params) { if (path.length() == 0) { return rootValue; } - String[] strings = splitPath(decoder.decode(path)); + String[] strings = Strings.splitStringToArray(path, separator); if (strings.length == 0) { return rootValue; } @@ -235,50 +222,4 @@ public T retrieve(String path, Map params) { } return root.retrieve(strings, index, params); } - - /* - Splits up the url path up by '/' and is aware of - index name expressions that appear between '<' and '>'. - */ - String[] splitPath(final String path) { - if (path == null || path.length() == 0) { - return Strings.EMPTY_ARRAY; - } - int count = 1; - boolean splitAllowed = true; - for (int i = 0; i < path.length(); i++) { - final char currentC = path.charAt(i); - if ('<' == currentC) { - splitAllowed = false; - } else if (currentC == '>') { - splitAllowed = true; - } else if (splitAllowed && currentC == separator) { - count++; - } - } - - final List result = new ArrayList<>(count); - final StringBuilder builder = new StringBuilder(); - - splitAllowed = true; - for (int i = 0; i < path.length(); i++) { - final char currentC = path.charAt(i); - if ('<' == currentC) { - splitAllowed = false; - } else if (currentC == '>') { - splitAllowed = true; - } else if (splitAllowed && currentC == separator) { - if (builder.length() > 0) { - result.add(builder.toString()); - builder.setLength(0); - } - continue; - } - builder.append(currentC); - } - if (builder.length() > 0) { - result.add(builder.toString()); - } - return result.toArray(new String[result.size()]); - } } diff --git a/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java b/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java index aec4fb248883d..1859a8ab566aa 100644 --- a/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java +++ b/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java @@ -19,13 +19,13 @@ package org.elasticsearch.common.path; +import org.elasticsearch.rest.support.RestUtils; import org.elasticsearch.test.ESTestCase; import org.junit.Test; import java.util.HashMap; import java.util.Map; -import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -34,8 +34,15 @@ */ public class PathTrieTests extends ESTestCase { + public static final PathTrie.Decoder NO_DECODER = new PathTrie.Decoder() { + @Override + public String decode(String value) { + return value; + } + }; + public void testPath() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("/a/b/c", "walla"); trie.insert("a/d/g", "kuku"); trie.insert("x/b/c", "lala"); @@ -62,13 +69,13 @@ public void testPath() { } public void testEmptyPath() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("/", "walla"); assertThat(trie.retrieve(""), equalTo("walla")); } public void testDifferentNamesOnDifferentPath() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("/a/{type}", "test1"); trie.insert("/b/{name}", "test2"); @@ -82,7 +89,7 @@ public void testDifferentNamesOnDifferentPath() { } public void testSameNameOnDifferentPath() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("/a/c/{name}", "test1"); trie.insert("/b/{name}", "test2"); @@ -96,7 +103,7 @@ public void testSameNameOnDifferentPath() { } public void testPreferNonWildcardExecution() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("{test}", "test1"); trie.insert("b", "test2"); trie.insert("{test}/a", "test3"); @@ -112,7 +119,7 @@ public void testPreferNonWildcardExecution() { } public void testSamePathConcreteResolution() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("{x}/{y}/{z}", "test1"); trie.insert("{x}/_y/{k}", "test2"); @@ -128,7 +135,7 @@ public void testSamePathConcreteResolution() { } public void testNamedWildcardAndLookupWithWildcard() { - PathTrie trie = new PathTrie<>(); + PathTrie trie = new PathTrie<>(NO_DECODER); trie.insert("x/{test}", "test1"); trie.insert("{test}/a", "test2"); trie.insert("/{test}", "test3"); @@ -156,24 +163,20 @@ public void testNamedWildcardAndLookupWithWildcard() { assertThat(params.get("test"), equalTo("*")); } - public void testSplitPath() { - PathTrie trie = new PathTrie<>(); - assertThat(trie.splitPath("/a/"), arrayContaining("a")); - assertThat(trie.splitPath("/a/b"),arrayContaining("a", "b")); - assertThat(trie.splitPath("/a/b/c"), arrayContaining("a", "b", "c")); - assertThat(trie.splitPath("/a/b/"), arrayContaining("a", "b", "")); - assertThat(trie.splitPath("/a/b//d"), arrayContaining("a", "b", "", "d")); - - assertThat(trie.splitPath("//_search"), arrayContaining("", "_search")); - assertThat(trie.splitPath("//_search"), arrayContaining("", "_search")); - assertThat(trie.splitPath("//_search"), arrayContaining("", "_search")); - assertThat(trie.splitPath("//_search"), arrayContaining("", "_search")); - assertThat(trie.splitPath("//log/_search"), arrayContaining("", "log", "_search")); - - assertThat(trie.splitPath("/,/_search"), arrayContaining(",", "_search")); - assertThat(trie.splitPath("/,/_search"), arrayContaining(",", "_search")); - assertThat(trie.splitPath("/,/_search"), arrayContaining(",", "_search")); - assertThat(trie.splitPath("/,/_search"), arrayContaining(",", "_search")); + //https://github.com/elastic/elasticsearch/issues/14177 + //https://github.com/elastic/elasticsearch/issues/13665 + public void testEscapedSlashWithinUrl() { + PathTrie pathTrie = new PathTrie<>(RestUtils.REST_DECODER); + pathTrie.insert("/{index}/{type}/{id}", "test"); + HashMap params = new HashMap<>(); + assertThat(pathTrie.retrieve("/index/type/a%2Fe", params), equalTo("test")); + assertThat(params.get("index"), equalTo("index")); + assertThat(params.get("type"), equalTo("type")); + assertThat(params.get("id"), equalTo("a/e")); + params.clear(); + assertThat(pathTrie.retrieve("//type/id", params), equalTo("test")); + assertThat(params.get("index"), equalTo("")); + assertThat(params.get("type"), equalTo("type")); + assertThat(params.get("id"), equalTo("id")); } - } diff --git a/core/src/test/java/org/elasticsearch/plugins/SitePluginIT.java b/core/src/test/java/org/elasticsearch/plugins/SitePluginIT.java index 6e62fd9e675e0..8ecc4ee0e16eb 100644 --- a/core/src/test/java/org/elasticsearch/plugins/SitePluginIT.java +++ b/core/src/test/java/org/elasticsearch/plugins/SitePluginIT.java @@ -100,7 +100,7 @@ public void testThatPathsAreNormalized() throws Exception { notFoundUris.add("/_plugin/dummy/%2e%2e/%2e%2e/%2e%2e/%2e%2e/index.html"); notFoundUris.add("/_plugin/dummy/%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2findex.html"); notFoundUris.add("/_plugin/dummy/%2E%2E/%2E%2E/%2E%2E/%2E%2E/index.html"); - notFoundUris.add("/_plugin/dummy/..\\..\\..\\..\\..\\log4j.properties"); + notFoundUris.add("/_plugin/dummy/..%5C..%5C..%5C..%5C..%5Clog4j.properties"); for (String uri : notFoundUris) { HttpResponse response = httpClient().path(uri).execute(); diff --git a/core/src/test/java/org/elasticsearch/test/rest/client/RestClient.java b/core/src/test/java/org/elasticsearch/test/rest/client/RestClient.java index f18f9201d1a9d..5cdf728ef231e 100644 --- a/core/src/test/java/org/elasticsearch/test/rest/client/RestClient.java +++ b/core/src/test/java/org/elasticsearch/test/rest/client/RestClient.java @@ -232,8 +232,9 @@ private HttpRequestBuilder callApiBuilder(String apiName, Map pa httpRequestBuilder.method(RandomizedTest.randomFrom(supportedMethods)); } - //the http method is randomized (out of the available ones with the chosen api) - return httpRequestBuilder.path(RandomizedTest.randomFrom(restApi.getFinalPaths(pathParts))); + //the rest path to use is randomized out of the matching ones (if more than one) + RestPath restPath = RandomizedTest.randomFrom(restApi.getFinalPaths(pathParts)); + return httpRequestBuilder.pathParts(restPath.getPathParts()); } private RestApi restApi(String apiName) { diff --git a/core/src/test/java/org/elasticsearch/test/rest/client/RestPath.java b/core/src/test/java/org/elasticsearch/test/rest/client/RestPath.java new file mode 100644 index 0000000000000..f6e3ddabd5e8f --- /dev/null +++ b/core/src/test/java/org/elasticsearch/test/rest/client/RestPath.java @@ -0,0 +1,97 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.test.rest.client; + +import java.util.*; + +public class RestPath { + private final List parts; + private final List placeholders; + + public RestPath(List parts) { + List pathParts = new ArrayList<>(parts.size()); + for (String part : parts) { + pathParts.add(new PathPart(part, false)); + } + this.parts = pathParts; + this.placeholders = Collections.emptyList(); + } + + public RestPath(String path) { + String[] pathParts = path.split("/"); + List placeholders = new ArrayList<>(); + List parts = new ArrayList<>(); + for (String pathPart : pathParts) { + if (pathPart.length() > 0) { + if (pathPart.startsWith("{")) { + if (pathPart.indexOf('}') != pathPart.length() - 1) { + throw new IllegalArgumentException("more than one parameter found in the same path part: [" + pathPart + "]"); + } + String placeholder = pathPart.substring(1, pathPart.length() - 1); + parts.add(new PathPart(placeholder, true)); + placeholders.add(placeholder); + } else { + parts.add(new PathPart(pathPart, false)); + } + } + } + this.placeholders = placeholders; + this.parts = parts; + } + + public String[] getPathParts() { + String[] parts = new String[this.parts.size()]; + int i = 0; + for (PathPart part : this.parts) { + parts[i++] = part.pathPart; + } + return parts; + } + + public boolean matches(Set params) { + return placeholders.size() == params.size() && placeholders.containsAll(params); + } + + public RestPath replacePlaceholders(Map params) { + List finalPathParts = new ArrayList<>(parts.size()); + for (PathPart pathPart : parts) { + if (pathPart.isPlaceholder) { + String value = params.get(pathPart.pathPart); + if (value == null) { + throw new IllegalArgumentException("parameter [" + pathPart.pathPart + "] missing"); + } + finalPathParts.add(value); + } else { + finalPathParts.add(pathPart.pathPart); + } + } + return new RestPath(finalPathParts); + } + + private static class PathPart { + private final boolean isPlaceholder; + private final String pathPart; + + private PathPart(String pathPart, boolean isPlaceholder) { + this.isPlaceholder = isPlaceholder; + this.pathPart = pathPart; + } + } +} \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/test/rest/client/http/HttpRequestBuilder.java b/core/src/test/java/org/elasticsearch/test/rest/client/http/HttpRequestBuilder.java index 7791cda78411c..b56c99d5f5037 100644 --- a/core/src/test/java/org/elasticsearch/test/rest/client/http/HttpRequestBuilder.java +++ b/core/src/test/java/org/elasticsearch/test/rest/client/http/HttpRequestBuilder.java @@ -86,14 +86,42 @@ public HttpRequestBuilder port(int port) { return this; } + /** + * Sets the path to send the request to. Url encoding needs to be applied by the caller. + * Use {@link #pathParts(String...)} instead if the path needs to be encoded, part by part. + */ public HttpRequestBuilder path(String path) { this.path = path; return this; } + /** + * Sets the path by providing the different parts (without slashes), which will be properly encoded. + */ + public HttpRequestBuilder pathParts(String... path) { + //encode rules for path and query string parameters are different. We use URI to encode the path, and URLEncoder for each query string parameter (see addParam). + //We need to encode each path part separately though, as each one might contain slashes that need to be escaped, which needs to be done manually. + if (path.length == 0) { + this.path = "/"; + return this; + } + StringBuilder finalPath = new StringBuilder(); + for (String pathPart : path) { + try { + finalPath.append('/'); + URI uri = new URI(null, null, null, -1, pathPart, null, null); + //manually escape any slash that each part may contain + finalPath.append(uri.getRawPath().replaceAll("/", "%2F")); + } catch(URISyntaxException e) { + throw new RuntimeException("unable to build uri", e); + } + } + this.path = finalPath.toString(); + return this; + } + public HttpRequestBuilder addParam(String name, String value) { try { - //manually url encode params, since URI does it only partially (e.g. '+' stays as is) this.params.put(name, URLEncoder.encode(value, "utf-8")); return this; } catch (UnsupportedEncodingException e) { @@ -181,19 +209,12 @@ private HttpUriRequest buildRequest() { } private URI buildUri() { - try { - //url encode rules for path and query params are different. We use URI to encode the path, but we manually encode each query param through URLEncoder. - URI uri = new URI(protocol, null, host, port, path, null, null); - //String concatenation FTW. If we use the nicer multi argument URI constructor query parameters will get only partially encoded - //(e.g. '+' will stay as is) hence when trying to properly encode params manually they will end up double encoded (+ becomes %252B instead of %2B). - StringBuilder uriBuilder = new StringBuilder(protocol).append("://").append(host).append(":").append(port).append(uri.getRawPath()); - if (params.size() > 0) { - uriBuilder.append("?").append(Joiner.on('&').withKeyValueSeparator("=").join(params)); - } - return URI.create(uriBuilder.toString()); - } catch(URISyntaxException e) { - throw new IllegalArgumentException("unable to build uri", e); + StringBuilder uriBuilder = new StringBuilder(protocol).append("://").append(host).append(":").append(port).append(path); + if (params.size() > 0) { + uriBuilder.append("?").append(Joiner.on('&').withKeyValueSeparator("=").join(params)); } + //using this constructor no url encoding happens, as we did everything upfront in addParam and pathPart methods + return URI.create(uriBuilder.toString()); } private HttpEntityEnclosingRequestBase addOptionalBody(HttpEntityEnclosingRequestBase requestBase) { diff --git a/core/src/test/java/org/elasticsearch/test/rest/spec/RestApi.java b/core/src/test/java/org/elasticsearch/test/rest/spec/RestApi.java index 0996df45b48fe..f22db1be59455 100644 --- a/core/src/test/java/org/elasticsearch/test/rest/spec/RestApi.java +++ b/core/src/test/java/org/elasticsearch/test/rest/spec/RestApi.java @@ -22,12 +22,11 @@ import com.google.common.collect.Maps; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; +import org.elasticsearch.test.rest.client.RestPath; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * Represents an elasticsearch REST endpoint (api) @@ -41,7 +40,7 @@ public class RestApi { private List params = Lists.newArrayList(); private BODY body = BODY.NOT_SUPPORTED; - public static enum BODY { + public enum BODY { NOT_SUPPORTED, OPTIONAL, REQUIRED } @@ -131,28 +130,18 @@ public boolean isBodyRequired() { * Finds the best matching rest path given the current parameters and replaces * placeholders with their corresponding values received as arguments */ - public String[] getFinalPaths(Map pathParams) { - + public RestPath[] getFinalPaths(Map pathParams) { List matchingRestPaths = findMatchingRestPaths(pathParams.keySet()); if (matchingRestPaths == null || matchingRestPaths.isEmpty()) { throw new IllegalArgumentException("unable to find matching rest path for api [" + name + "] and path params " + pathParams); } - String[] paths = new String[matchingRestPaths.size()]; + RestPath[] restPaths = new RestPath[matchingRestPaths.size()]; for (int i = 0; i < matchingRestPaths.size(); i++) { RestPath restPath = matchingRestPaths.get(i); - String path = restPath.path; - for (Map.Entry paramEntry : restPath.parts.entrySet()) { - // replace path placeholders with actual values - String value = pathParams.get(paramEntry.getValue()); - if (value == null) { - throw new IllegalArgumentException("parameter [" + paramEntry.getValue() + "] missing"); - } - path = path.replace(paramEntry.getKey(), value); - } - paths[i] = path; + restPaths[i] = restPath.replacePlaceholders(pathParams); } - return paths; + return restPaths; } /** @@ -165,15 +154,11 @@ private List findMatchingRestPaths(Set restParams) { List matchingRestPaths = Lists.newArrayList(); RestPath[] restPaths = buildRestPaths(); - for (RestPath restPath : restPaths) { - if (restPath.parts.size() == restParams.size()) { - if (restPath.parts.values().containsAll(restParams)) { - matchingRestPaths.add(restPath); - } + if (restPath.matches(restParams)) { + matchingRestPaths.add(restPath); } } - return matchingRestPaths; } @@ -184,33 +169,4 @@ private RestPath[] buildRestPaths() { } return restPaths; } - - private static class RestPath { - private static final Pattern PLACEHOLDERS_PATTERN = Pattern.compile("(\\{(.*?)})"); - - final String path; - //contains param to replace (e.g. {index}) and param key to use for lookup in the current values map (e.g. index) - final Map parts; - - RestPath(String path) { - this.path = path; - this.parts = extractParts(path); - } - - private static Map extractParts(String input) { - Map parts = Maps.newHashMap(); - Matcher matcher = PLACEHOLDERS_PATTERN.matcher(input); - while (matcher.find()) { - //key is e.g. {index} - String key = input.substring(matcher.start(), matcher.end()); - if (matcher.groupCount() != 2) { - throw new IllegalArgumentException("no lookup key found for param [" + key + "]"); - } - //to be replaced with current value found with key e.g. index - String value = matcher.group(2); - parts.put(key, value); - } - return parts; - } - } }