diff --git a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java index 50d579492d17c..8556c7733804d 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java +++ b/src/main/java/org/elasticsearch/search/aggregations/AggregatorParsers.java @@ -35,7 +35,7 @@ */ public class AggregatorParsers { - public static final Pattern VALID_AGG_NAME = Pattern.compile("[a-zA-Z0-9\\-_]+"); + public static final Pattern VALID_AGG_NAME = Pattern.compile("[^\\[\\]>]+"); private final ImmutableMap parsers; diff --git a/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java b/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java index eea4482df8401..29a2abfe8d7cb 100644 --- a/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java +++ b/src/main/java/org/elasticsearch/search/aggregations/support/OrderPath.java @@ -71,27 +71,43 @@ public static OrderPath parse(String path) { String[] tuple = new String[2]; for (int i = 0; i < elements.length; i++) { String element = elements[i]; - int index = element.lastIndexOf('.'); - if (index >= 0) { + if (i == elements.length - 1) { + int index = element.lastIndexOf('['); + if (index >= 0) { + if (index == 0 || index > element.length() - 3) { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + if (element.charAt(element.length() - 1) != ']') { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + tokens[i] = new Token(element, element.substring(0, index), element.substring(index + 1, element.length() - 1)); + continue; + } + index = element.lastIndexOf('.'); + if (index < 0) { + tokens[i] = new Token(element, element, null); + continue; + } if (index == 0 || index > element.length() - 2) { throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); } tuple = split(element, index, tuple); tokens[i] = new Token(element, tuple[0], tuple[1]); - continue; - } - index = element.lastIndexOf('['); - if (index < 0) { + + } else { + int index = element.lastIndexOf('['); + if (index >= 0) { + if (index == 0 || index > element.length() - 3) { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + if (element.charAt(element.length() - 1) != ']') { + throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); + } + tokens[i] = new Token(element, element.substring(0, index), element.substring(index + 1, element.length() - 1)); + continue; + } tokens[i] = new Token(element, element, null); - continue; - } - if (index == 0 || index > element.length() - 3) { - throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); - } - if (element.charAt(element.length() - 1) != ']') { - throw new AggregationExecutionException("Invalid path element [" + element + "] in path [" + path + "]"); } - tokens[i] = new Token(element, element.substring(0, index), element.substring(index + 1, element.length() - 1)); } return new OrderPath(tokens); } diff --git a/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java index 1d3044700a407..9007611aa978c 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/ParsingTests.java @@ -83,7 +83,7 @@ public void testTwoAggs() throws Exception { @Test(expected=SearchPhaseExecutionException.class) public void testInvalidAggregationName() throws Exception { - Matcher matcher = Pattern.compile("[a-zA-Z0-9\\-_]+").matcher(""); + Matcher matcher = Pattern.compile("[^\\[\\]>]+").matcher(""); String name; SecureRandom rand = new SecureRandom(); int len = randomIntBetween(1, 5); diff --git a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java index d4cda692ff35e..6c03dc9a1a3f4 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/bucket/StringTermsTests.java @@ -984,6 +984,130 @@ public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels() assertThat(stats.getMax(), equalTo(asc ? 4.0 : 2.0)); } + @Test + public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels_specialChars() throws Exception { + StringBuilder filter2NameBuilder = new StringBuilder("filt.er2"); + filter2NameBuilder.append(randomAsciiOfLengthBetween(3, 10).replace("[", "").replace("]", "").replace(">", "")); + String filter2Name = filter2NameBuilder.toString(); + StringBuilder statsNameBuilder = new StringBuilder("st.ats"); + statsNameBuilder.append(randomAsciiOfLengthBetween(3, 10).replace("[", "").replace("]", "").replace(">", "")); + String statsName = statsNameBuilder.toString(); + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .executionHint(randomExecutionHint()) + .field("tag") + .collectMode(randomFrom(SubAggCollectionMode.values())) + .order(Terms.Order.aggregation("filter1>" + filter2Name + ">" + statsName + ".max", asc)) + .subAggregation(filter("filter1").filter(FilterBuilders.matchAllFilter()) + .subAggregation(filter(filter2Name).filter(FilterBuilders.matchAllFilter()) + .subAggregation(stats(statsName).field("i")))) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + // the max for "more" is 2 + // the max for "less" is 4 + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "more" : "less")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter2 = filter1.getAggregations().get(filter2Name); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 3l : 2l)); + Stats stats = filter2.getAggregations().get(statsName); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 2.0 : 4.0)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "less" : "more")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 2l : 3l)); + filter2 = filter1.getAggregations().get(filter2Name); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 2l : 3l)); + stats = filter2.getAggregations().get(statsName); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 4.0 : 2.0)); + } + + @Test + public void singleValuedField_OrderedBySubAggregationAsc_MultiHierarchyLevels_specialCharsNoDotNotation() throws Exception { + StringBuilder filter2NameBuilder = new StringBuilder("filt.er2"); + filter2NameBuilder.append(randomAsciiOfLengthBetween(3, 10).replace("[", "").replace("]", "").replace(">", "")); + String filter2Name = filter2NameBuilder.toString(); + StringBuilder statsNameBuilder = new StringBuilder("st.ats"); + statsNameBuilder.append(randomAsciiOfLengthBetween(3, 10).replace("[", "").replace("]", "").replace(">", "")); + String statsName = statsNameBuilder.toString(); + boolean asc = randomBoolean(); + SearchResponse response = client().prepareSearch("idx").setTypes("type") + .addAggregation(terms("tags") + .executionHint(randomExecutionHint()) + .field("tag") + .collectMode(randomFrom(SubAggCollectionMode.values())) + .order(Terms.Order.aggregation("filter1>" + filter2Name + ">" + statsName + "[max]", asc)) + .subAggregation(filter("filter1").filter(FilterBuilders.matchAllFilter()) + .subAggregation(filter(filter2Name).filter(FilterBuilders.matchAllFilter()) + .subAggregation(stats(statsName).field("i")))) + ).execute().actionGet(); + + + assertSearchResponse(response); + + Terms tags = response.getAggregations().get("tags"); + assertThat(tags, notNullValue()); + assertThat(tags.getName(), equalTo("tags")); + assertThat(tags.getBuckets().size(), equalTo(2)); + + Iterator iters = tags.getBuckets().iterator(); + + // the max for "more" is 2 + // the max for "less" is 4 + + Terms.Bucket tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "more" : "less")); + assertThat(tag.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 3l : 2l)); + Filter filter2 = filter1.getAggregations().get(filter2Name); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 3l : 2l)); + Stats stats = filter2.getAggregations().get(statsName); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 2.0 : 4.0)); + + tag = iters.next(); + assertThat(tag, notNullValue()); + assertThat(key(tag), equalTo(asc ? "less" : "more")); + assertThat(tag.getDocCount(), equalTo(asc ? 2l : 3l)); + filter1 = tag.getAggregations().get("filter1"); + assertThat(filter1, notNullValue()); + assertThat(filter1.getDocCount(), equalTo(asc ? 2l : 3l)); + filter2 = filter1.getAggregations().get(filter2Name); + assertThat(filter2, notNullValue()); + assertThat(filter2.getDocCount(), equalTo(asc ? 2l : 3l)); + stats = filter2.getAggregations().get(statsName); + assertThat(stats, notNullValue()); + assertThat(stats.getMax(), equalTo(asc ? 4.0 : 2.0)); + } + @Test public void singleValuedField_OrderedByMissingSubAggregation() throws Exception { diff --git a/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java b/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java index 26b485bc1f734..cc41e04c3b20f 100644 --- a/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java +++ b/src/test/java/org/elasticsearch/search/aggregations/support/PathTests.java @@ -27,8 +27,6 @@ import java.util.List; import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.fail; /** * @@ -54,8 +52,8 @@ public void testValidPaths() throws Exception { assertValidPath("foo[bar]>baz", tokens().add("foo", "bar").add("baz")); assertValidPath("foo[bar]>baz[qux]", tokens().add("foo", "bar").add("baz", "qux")); assertValidPath("foo[bar]>baz.qux", tokens().add("foo", "bar").add("baz", "qux")); - assertValidPath("foo.bar>baz.qux", tokens().add("foo", "bar").add("baz", "qux")); - assertValidPath("foo.bar>baz[qux]", tokens().add("foo", "bar").add("baz", "qux")); + assertValidPath("foo.bar>baz.qux", tokens().add("foo.bar").add("baz", "qux")); + assertValidPath("foo.bar>baz[qux]", tokens().add("foo.bar").add("baz", "qux")); } private void assertInvalidPath(String path, String reason) {