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 25770e0d97e53b7ba5210a8652647d378e14ce1f..85f65400722962d3ba97f8f0c3b79b5e1366fc80 100644 (file)
--- 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<D extends BaseDoc> implements Iterator<D> {
 
     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 4c0070d08ac39081b69306c332f61637a6d7ea8a..11519a92055612e5613aebe7be9233c38ed502b2 100644 (file)
--- 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<CharSequence, ?, String> PIPE_JOINER = Collectors.joining("|");
 
   private final QueryBuilder query;
   private final Map<String, QueryBuilder> 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 9fcc1101827ebae89adaf9c78660ddc71bdb3c15..e42af16478c0f6ce89119f4ccd1f80ade5e54ded 100644 (file)
--- 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<String> 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<String, RuleKey> {
     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 25c6f83b40347a4fc6745331f817037d5d334d8e..95d81be899ca76f42229cdbaf3d442c44243974c 100644 (file)
--- 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 4d44e20c9e8588f792090b95cf568c4c33fdf4e6..dd4f79d4918685624c42728db899cea99c759549 100644 (file)
--- 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(