From: Simon Brandhof Date: Tue, 29 Nov 2016 10:48:08 +0000 (+0100) Subject: SONAR-8437 escape selected value in ES facet of WS api/issues X-Git-Tag: 6.2-RC2~19 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=40cc8f14ff522910dc708f077af9b38ed9173917;p=sonarqube.git SONAR-8437 escape selected value in ES facet of WS api/issues --- diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/EsUtils.java b/server/sonar-server/src/main/java/org/sonar/server/es/EsUtils.java index 25770e0d97e..85f65400722 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/es/EsUtils.java +++ b/server/sonar-server/src/main/java/org/sonar/server/es/EsUtils.java @@ -31,6 +31,7 @@ import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Queue; +import java.util.regex.Pattern; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.elasticsearch.action.bulk.BulkRequestBuilder; @@ -47,6 +48,7 @@ import static java.lang.String.format; public class EsUtils { public static final int SCROLL_TIME_IN_MINUTES = 3; + private static final Pattern SPECIAL_REGEX_CHARS = Pattern.compile("[{}()\\[\\].+*?^$\\\\|]"); private EsUtils() { // only static methods @@ -107,6 +109,15 @@ public class EsUtils { return new DocScrollIterator<>(esClient, scrollId, docConverter); } + /** + * Escapes regexp special characters so that text can be forwarded from end-user input + * to Elasticsearch regexp query (for instance attributes "include" and "exclude" of + * term aggregations. + */ + public static String escapeSpecialRegexChars(String str) { + return SPECIAL_REGEX_CHARS.matcher(str).replaceAll("\\\\$0"); + } + private static class DocScrollIterator implements Iterator { private final EsClient esClient; diff --git a/server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java b/server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java index 4c0070d08ac..11519a92055 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java +++ b/server/sonar-server/src/main/java/org/sonar/server/es/StickyFacetBuilder.java @@ -19,8 +19,11 @@ */ package org.sonar.server.es; -import com.google.common.base.Joiner; +import java.util.Arrays; import java.util.Map; +import java.util.Objects; +import java.util.stream.Collector; +import java.util.stream.Collectors; import javax.annotation.Nullable; import org.apache.commons.lang.ArrayUtils; import org.elasticsearch.index.query.BoolQueryBuilder; @@ -40,7 +43,7 @@ public class StickyFacetBuilder { private static final int FACET_DEFAULT_MIN_DOC_COUNT = 1; private static final int FACET_DEFAULT_SIZE = 10; private static final Order FACET_DEFAULT_ORDER = Terms.Order.count(false); - private static final Joiner PIPE_JOINER = Joiner.on('|'); + private static final Collector PIPE_JOINER = Collectors.joining("|"); private final QueryBuilder query; private final Map filters; @@ -107,9 +110,14 @@ public class StickyFacetBuilder { public FilterAggregationBuilder addSelectedItemsToFacet(String fieldName, String facetName, FilterAggregationBuilder facetTopAggregation, Object... selected) { if (selected.length > 0) { + String includes = Arrays.stream(selected) + .filter(Objects::nonNull) + .map(s -> EsUtils.escapeSpecialRegexChars(s.toString())) + .collect(PIPE_JOINER); + TermsBuilder selectedTerms = AggregationBuilders.terms(facetName + "_selected") .field(fieldName) - .include(PIPE_JOINER.join(selected)); + .include(includes); if (subAggregation != null) { selectedTerms = selectedTerms.subAggregation(subAggregation); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java index 9fcc1101827..e42af16478c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java @@ -33,7 +33,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.regex.Pattern; import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.commons.lang.StringUtils; @@ -69,6 +68,7 @@ import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.matchQuery; import static org.elasticsearch.index.query.QueryBuilders.simpleQueryStringQuery; import static org.sonar.server.es.EsUtils.SCROLL_TIME_IN_MINUTES; +import static org.sonar.server.es.EsUtils.escapeSpecialRegexChars; import static org.sonar.server.es.EsUtils.scrollIds; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_ACTIVE_RULE_INHERITANCE; import static org.sonar.server.rule.index.RuleIndexDefinition.FIELD_ACTIVE_RULE_PROFILE_KEY; @@ -106,7 +106,6 @@ public class RuleIndex extends BaseIndex { public static final String FACET_STATUSES = "statuses"; public static final String FACET_TYPES = "types"; public static final String FACET_OLD_DEFAULT = "true"; - private static Pattern SPECIAL_REGEX_CHARS = Pattern.compile("[{}()\\[\\].+*?^$\\\\|]"); public static final List ALL_STATUSES_EXCEPT_REMOVED = ImmutableList.copyOf( Collections2.filter(Collections2.transform(Arrays.asList(RuleStatus.values()), RuleStatusToString.INSTANCE), NotRemoved.INSTANCE)); @@ -496,10 +495,6 @@ public class RuleIndex extends BaseIndex { return terms; } - private static String escapeSpecialRegexChars(String str) { - return SPECIAL_REGEX_CHARS.matcher(str).replaceAll("\\\\$0"); - } - private enum ToRuleKey implements Function { INSTANCE; diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/EsUtilsTest.java b/server/sonar-server/src/test/java/org/sonar/server/es/EsUtilsTest.java index 25c6f83b403..95d81be899c 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/es/EsUtilsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/es/EsUtilsTest.java @@ -33,6 +33,7 @@ import org.sonar.test.TestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.server.es.EsUtils.escapeSpecialRegexChars; public class EsUtilsTest { @@ -74,4 +75,19 @@ public class EsUtilsTest { assertThat(EsUtils.parseDateTime("2017-07-14T04:40:00.000+02:00").getTime()).isEqualTo(1_500_000_000_000L); assertThat(EsUtils.parseDateTime(null)).isNull(); } + + @Test + public void test_escapeSpecialRegexChars() { + assertThat(escapeSpecialRegexChars("")).isEqualTo(""); + assertThat(escapeSpecialRegexChars("foo")).isEqualTo("foo"); + assertThat(escapeSpecialRegexChars("FOO")).isEqualTo("FOO"); + assertThat(escapeSpecialRegexChars("foo++")).isEqualTo("foo\\+\\+"); + assertThat(escapeSpecialRegexChars("foo[]")).isEqualTo("foo\\[\\]"); + assertThat(escapeSpecialRegexChars(".*")).isEqualTo("\\.\\*"); + assertThat(escapeSpecialRegexChars("foo\\d")).isEqualTo("foo\\\\d"); + assertThat(escapeSpecialRegexChars("^")).isEqualTo("\\^"); + assertThat(escapeSpecialRegexChars("$")).isEqualTo("\\$"); + assertThat(escapeSpecialRegexChars("|")).isEqualTo("\\|"); + assertThat(escapeSpecialRegexChars("a bit of | $ .* ^ everything")).isEqualTo("a bit of \\| \\$ \\.\\* \\^ everything"); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java index 4d44e20c9e8..dd4f79d4918 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexTest.java @@ -252,6 +252,17 @@ public class RuleIndexTest { assertThat(index.search(query, new SearchOptions()).getIds()).hasSize(2); } + @Test + public void tags_facet_supports_selected_value_with_regexp_special_characters() { + indexRules(newDoc(RuleKey.of("java", "S001")).setAllTags(singleton("misra++"))); + + RuleQuery query = new RuleQuery().setTags(singletonList("misra[")); + SearchOptions options = new SearchOptions().addFacets(RuleIndex.FACET_TAGS); + + // do not fail + assertThat(index.search(query, options).getTotal()).isEqualTo(0); + } + @Test public void search_by_types() { indexRules(