[ES|QL] Validate index name in parser (elastic#112081)

* validate index name in parser
matthewabbott · Oct 4, 2024 · 78d4bb0 · 78d4bb0
1 parent 1b3ccd9
commit 78d4bb0
Show file tree

Hide file tree

Showing 5 changed files with 284 additions and 12 deletions.
diff --git a/docs/changelog/112081.yaml b/docs/changelog/112081.yaml
@@ -0,0 +1,5 @@
+pr: 112081
+summary: "[ES|QL] Validate index name in parser"
+area: ES|QL
+type: enhancement
+issues: []
diff --git a/...ck/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/StringUtils.java b/...ck/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/util/StringUtils.java
@@ -39,6 +39,7 @@ private StringUtils() {}
     public static final String NEW_LINE = "\n";
     public static final String SQL_WILDCARD = "%";
     public static final String WILDCARD = "*";
+    public static final String EXCLUSION = "-";
 
     private static final String[] INTEGER_ORDINALS = new String[] { "th", "st", "nd", "rd", "th", "th", "th", "th", "th", "th" };
 

diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/IdentifierBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/IdentifierBuilder.java
@@ -8,14 +8,22 @@
 package org.elasticsearch.xpack.esql.parser;
 
 import org.antlr.v4.runtime.tree.TerminalNode;
+import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
+import org.elasticsearch.cluster.metadata.MetadataCreateIndexService;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.indices.InvalidIndexNameException;
+import org.elasticsearch.xpack.esql.core.util.Holder;
 import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IdentifierContext;
 import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IndexStringContext;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.elasticsearch.transport.RemoteClusterAware.REMOTE_CLUSTER_INDEX_SEPARATOR;
+import static org.elasticsearch.xpack.esql.core.util.StringUtils.EXCLUSION;
+import static org.elasticsearch.xpack.esql.core.util.StringUtils.WILDCARD;
+import static org.elasticsearch.xpack.esql.parser.ParserUtils.source;
 
 abstract class IdentifierBuilder extends AbstractBuilder {
 
@@ -46,12 +54,54 @@ public String visitIndexString(IndexStringContext ctx) {
 
     public String visitIndexPattern(List<EsqlBaseParser.IndexPatternContext> ctx) {
         List<String> patterns = new ArrayList<>(ctx.size());
+        Holder<Boolean> hasSeenStar = new Holder<>(false);
         ctx.forEach(c -> {
             String indexPattern = visitIndexString(c.indexString());
+            hasSeenStar.set(indexPattern.contains(WILDCARD) || hasSeenStar.get());
+            validateIndexPattern(indexPattern, c, hasSeenStar.get());
             patterns.add(
                 c.clusterString() != null ? c.clusterString().getText() + REMOTE_CLUSTER_INDEX_SEPARATOR + indexPattern : indexPattern
             );
         });
         return Strings.collectionToDelimitedString(patterns, ",");
     }
+
+    private static void validateIndexPattern(String indexPattern, EsqlBaseParser.IndexPatternContext ctx, boolean hasSeenStar) {
+        // multiple index names can be in the same double quote, e.g. indexPattern = "idx1, *, -idx2"
+        String[] indices = indexPattern.split(",");
+        boolean hasExclusion = false;
+        for (String index : indices) {
+            hasSeenStar = index.contains(WILDCARD) || hasSeenStar;
+            index = index.replace(WILDCARD, "").strip();
+            if (index.isBlank()) {
+                continue;
+            }
+            hasExclusion = index.startsWith(EXCLUSION);
+            index = removeExclusion(index);
+            String tempName;
+            try {
+                // remove the exclusion outside of <>, from index names with DateMath expression,
+                // e.g. -<-logstash-{now/d}> becomes <-logstash-{now/d}> before calling resolveDateMathExpression
+                tempName = IndexNameExpressionResolver.resolveDateMathExpression(index);
+            } catch (ElasticsearchParseException e) {
+                // throws exception if the DateMath expression is invalid, resolveDateMathExpression does not complain about exclusions
+                throw new ParsingException(e, source(ctx), e.getMessage());
+            }
+            hasExclusion = tempName.startsWith(EXCLUSION) || hasExclusion;
+            index = tempName.equals(index) ? index : removeExclusion(tempName);
+            try {
+                MetadataCreateIndexService.validateIndexOrAliasName(index, InvalidIndexNameException::new);
+            } catch (InvalidIndexNameException e) {
+                // ignore invalid index name if it has exclusions and there is an index with wildcard before it
+                if (hasSeenStar && hasExclusion) {
+                    continue;
+                }
+                throw new ParsingException(e, source(ctx), e.getMessage());
+            }
+        }
+    }
+
+    private static String removeExclusion(String indexPattern) {
+        return indexPattern.charAt(0) == EXCLUSION.charAt(0) ? indexPattern.substring(1) : indexPattern;
+    }
 }
diff --git a/.../esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/.../esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java
@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.esql.parser;
 
+import org.elasticsearch.common.logging.LoggerMessageFormat;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.esql.VerificationException;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
@@ -38,6 +39,10 @@ void assertStatement(String statement, LogicalPlan expected) {
         assertThat(statement, actual, equalTo(expected));
     }
 
+    LogicalPlan statement(String query, String arg) {
+        return statement(LoggerMessageFormat.format(null, query, arg), new QueryParams());
+    }
+
     LogicalPlan statement(String e) {
         return statement(e, new QueryParams());
     }
@@ -124,4 +129,12 @@ void expectError(String query, List<QueryParam> params, String errorMessage) {
         );
         assertThat(e.getMessage(), containsString(errorMessage));
     }
+
+    void expectInvalidIndexNameErrorWithLineNumber(String query, String arg, String lineNumber, String indexString) {
+        expectError(LoggerMessageFormat.format(null, query, arg), lineNumber + "Invalid index name [" + indexString);
+    }
+
+    void expectDateMathErrorWithLineNumber(String query, String arg, String lineNumber, String error) {
+        expectError(LoggerMessageFormat.format(null, query, arg), lineNumber + error);
+    }
 }