From f3fe3bba74618fb1ea66b3b760b2096fb9c3c91b Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Mon, 21 Sep 2015 20:12:28 +0200 Subject: [PATCH] Index name expressions should not be broken up Closes #13665 --- .../org/elasticsearch/common/Strings.java | 1 - .../elasticsearch/common/path/PathTrie.java | 52 ++++++++++++++++++- .../common/path/PathTrieTests.java | 51 +++++++++++------- .../test/search/80_date_math_index_names.yaml | 7 +++ 4 files changed, 90 insertions(+), 21 deletions(-) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/search/80_date_math_index_names.yaml diff --git a/core/src/main/java/org/elasticsearch/common/Strings.java b/core/src/main/java/org/elasticsearch/common/Strings.java index dafaa616d57ad..aa560242c5fec 100644 --- a/core/src/main/java/org/elasticsearch/common/Strings.java +++ b/core/src/main/java/org/elasticsearch/common/Strings.java @@ -569,7 +569,6 @@ public static Set splitStringToSet(final String s, final char c) { count++; } } - // TODO (MvG): No push: hppc or jcf? final Set result = new HashSet<>(count); final int len = chars.length; int start = 0; // starting index in chars of the current substring. 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 0cc1d09c997fd..3bf2a9b17ee67 100644 --- a/core/src/main/java/org/elasticsearch/common/path/PathTrie.java +++ b/core/src/main/java/org/elasticsearch/common/path/PathTrie.java @@ -22,6 +22,8 @@ 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; @@ -195,7 +197,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(), decoder.decode(value)); + params.put(node.namedWildcard(), value); } } } @@ -222,7 +224,7 @@ public T retrieve(String path, Map params) { if (path.length() == 0) { return rootValue; } - String[] strings = Strings.splitStringToArray(path, separator); + String[] strings = splitPath(decoder.decode(path)); if (strings.length == 0) { return rootValue; } @@ -233,4 +235,50 @@ 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 1fbf76df367f4..aec4fb248883d 100644 --- a/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java +++ b/core/src/test/java/org/elasticsearch/common/path/PathTrieTests.java @@ -22,9 +22,10 @@ import org.elasticsearch.test.ESTestCase; import org.junit.Test; +import java.util.HashMap; import java.util.Map; -import static com.google.common.collect.Maps.newHashMap; +import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; @@ -33,7 +34,6 @@ */ public class PathTrieTests extends ESTestCase { - @Test public void testPath() { PathTrie trie = new PathTrie<>(); trie.insert("/a/b/c", "walla"); @@ -54,27 +54,25 @@ public void testPath() { assertThat(trie.retrieve("a/b/c/d"), nullValue()); assertThat(trie.retrieve("g/t/x"), equalTo("three")); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("index1/insert/12", params), equalTo("bingo")); assertThat(params.size(), equalTo(2)); assertThat(params.get("index"), equalTo("index1")); assertThat(params.get("docId"), equalTo("12")); } - @Test public void testEmptyPath() { PathTrie trie = new PathTrie<>(); trie.insert("/", "walla"); assertThat(trie.retrieve(""), equalTo("walla")); } - @Test public void testDifferentNamesOnDifferentPath() { PathTrie trie = new PathTrie<>(); trie.insert("/a/{type}", "test1"); trie.insert("/b/{name}", "test2"); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("/a/test", params), equalTo("test1")); assertThat(params.get("type"), equalTo("test")); @@ -83,13 +81,12 @@ public void testDifferentNamesOnDifferentPath() { assertThat(params.get("name"), equalTo("testX")); } - @Test public void testSameNameOnDifferentPath() { PathTrie trie = new PathTrie<>(); trie.insert("/a/c/{name}", "test1"); trie.insert("/b/{name}", "test2"); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("/a/c/test", params), equalTo("test1")); assertThat(params.get("name"), equalTo("test")); @@ -98,7 +95,6 @@ public void testSameNameOnDifferentPath() { assertThat(params.get("name"), equalTo("testX")); } - @Test public void testPreferNonWildcardExecution() { PathTrie trie = new PathTrie<>(); trie.insert("{test}", "test1"); @@ -108,20 +104,19 @@ public void testPreferNonWildcardExecution() { trie.insert("{test}/{testB}", "test5"); trie.insert("{test}/x/{testC}", "test6"); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("/b", params), equalTo("test2")); assertThat(trie.retrieve("/b/a", params), equalTo("test4")); assertThat(trie.retrieve("/v/x", params), equalTo("test5")); assertThat(trie.retrieve("/v/x/c", params), equalTo("test6")); } - @Test public void testSamePathConcreteResolution() { PathTrie trie = new PathTrie<>(); trie.insert("{x}/{y}/{z}", "test1"); trie.insert("{x}/_y/{k}", "test2"); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("/a/b/c", params), equalTo("test1")); assertThat(params.get("x"), equalTo("a")); assertThat(params.get("y"), equalTo("b")); @@ -132,7 +127,6 @@ public void testSamePathConcreteResolution() { assertThat(params.get("k"), equalTo("c")); } - @Test public void testNamedWildcardAndLookupWithWildcard() { PathTrie trie = new PathTrie<>(); trie.insert("x/{test}", "test1"); @@ -141,24 +135,45 @@ public void testNamedWildcardAndLookupWithWildcard() { trie.insert("/{test}/_endpoint", "test4"); trie.insert("/*/{test}/_endpoint", "test5"); - Map params = newHashMap(); + Map params = new HashMap<>(); assertThat(trie.retrieve("/x/*", params), equalTo("test1")); assertThat(params.get("test"), equalTo("*")); - params = newHashMap(); + params = new HashMap<>(); assertThat(trie.retrieve("/b/a", params), equalTo("test2")); assertThat(params.get("test"), equalTo("b")); - params = newHashMap(); + params = new HashMap<>(); assertThat(trie.retrieve("/*", params), equalTo("test3")); assertThat(params.get("test"), equalTo("*")); - params = newHashMap(); + params = new HashMap<>(); assertThat(trie.retrieve("/*/_endpoint", params), equalTo("test4")); assertThat(params.get("test"), equalTo("*")); - params = newHashMap(); + params = new HashMap<>(); assertThat(trie.retrieve("a/*/_endpoint", params), equalTo("test5")); 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")); + } + } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/80_date_math_index_names.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/80_date_math_index_names.yaml new file mode 100644 index 0000000000000..233b41c6cf7e3 --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/80_date_math_index_names.yaml @@ -0,0 +1,7 @@ +--- +"Missing index with catch": + + - do: + catch: /index=logstash-\d{4}\.\d{2}\.\d{2}/ + search: + index: