Skip to content

Commit a50d342

Browse files
committed
Ensure cluster string could be quoted (#120355)
Currently we accept "remote:index", remote:"index" but not "remote":"index" as a valid index pattern. This change fixes this.
1 parent 24b7b69 commit a50d342

File tree

7 files changed

+51
-8
lines changed

7 files changed

+51
-8
lines changed

docs/changelog/120355.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 120355
2+
summary: Ensure cluster string could be quoted
3+
area: ES|QL
4+
type: enhancement
5+
issues: []

x-pack/plugin/esql/src/main/antlr/EsqlBaseParser.g4

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ indexPattern
143143

144144
clusterString
145145
: UNQUOTED_SOURCE
146+
| QUOTED_STRING
146147
;
147148

148149
indexString

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.interp

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/EsqlBaseParser.java

Lines changed: 13 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/IdentifierBuilder.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.cluster.metadata.MetadataCreateIndexService;
1414
import org.elasticsearch.common.Strings;
1515
import org.elasticsearch.indices.InvalidIndexNameException;
16+
import org.elasticsearch.transport.RemoteClusterService;
1617
import org.elasticsearch.xpack.esql.core.util.Holder;
1718
import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IdentifierContext;
1819
import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IndexStringContext;
@@ -51,29 +52,51 @@ protected static String quoteIdString(String unquotedString) {
5152
return "`" + unquotedString.replace("`", "``") + "`";
5253
}
5354

55+
@Override
56+
public String visitClusterString(EsqlBaseParser.ClusterStringContext ctx) {
57+
if (ctx == null) {
58+
return null;
59+
} else if (ctx.UNQUOTED_SOURCE() != null) {
60+
return ctx.UNQUOTED_SOURCE().getText();
61+
} else {
62+
return unquote(ctx.QUOTED_STRING().getText());
63+
}
64+
}
65+
5466
@Override
5567
public String visitIndexString(IndexStringContext ctx) {
56-
TerminalNode n = ctx.UNQUOTED_SOURCE();
57-
return n != null ? n.getText() : unquote(ctx.QUOTED_STRING().getText());
68+
if (ctx.UNQUOTED_SOURCE() != null) {
69+
return ctx.UNQUOTED_SOURCE().getText();
70+
} else {
71+
return unquote(ctx.QUOTED_STRING().getText());
72+
}
5873
}
5974

6075
public String visitIndexPattern(List<EsqlBaseParser.IndexPatternContext> ctx) {
6176
List<String> patterns = new ArrayList<>(ctx.size());
6277
Holder<Boolean> hasSeenStar = new Holder<>(false);
6378
ctx.forEach(c -> {
6479
String indexPattern = visitIndexString(c.indexString());
65-
String clusterString = c.clusterString() != null ? c.clusterString().getText() : null;
80+
String clusterString = visitClusterString(c.clusterString());
6681
// skip validating index on remote cluster, because the behavior of remote cluster is not consistent with local cluster
6782
// For example, invalid#index is an invalid index name, however FROM *:invalid#index does not return an error
6883
if (clusterString == null) {
6984
hasSeenStar.set(indexPattern.contains(WILDCARD) || hasSeenStar.get());
7085
validateIndexPattern(indexPattern, c, hasSeenStar.get());
86+
} else {
87+
validateClusterString(clusterString, c);
7188
}
7289
patterns.add(clusterString != null ? clusterString + REMOTE_CLUSTER_INDEX_SEPARATOR + indexPattern : indexPattern);
7390
});
7491
return Strings.collectionToDelimitedString(patterns, ",");
7592
}
7693

94+
protected static void validateClusterString(String clusterString, EsqlBaseParser.IndexPatternContext ctx) {
95+
if (clusterString.indexOf(RemoteClusterService.REMOTE_CLUSTER_INDEX_SEPARATOR) != -1) {
96+
throw new ParsingException(source(ctx), "cluster string [{}] must not contain ':'", clusterString);
97+
}
98+
}
99+
77100
private static void validateIndexPattern(String indexPattern, EsqlBaseParser.IndexPatternContext ctx, boolean hasSeenStar) {
78101
// multiple index names can be in the same double quote, e.g. indexPattern = "idx1, *, -idx2"
79102
String[] indices = indexPattern.split(",");

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/IdentifierGenerator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ public static String randomIndexPattern(Feature... features) {
8080

8181
var pattern = maybeQuote(index.toString());
8282
if (canAdd(Features.CROSS_CLUSTER, features)) {
83-
var cluster = randomIdentifier();
83+
var cluster = maybeQuote(randomIdentifier());
8484
pattern = maybeQuote(cluster + ":" + pattern);
8585
}
8686

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2267,6 +2267,10 @@ public void testInvalidAlias() {
22672267
expectError("from test | eval A = coalesce(\"Å\", Å)", "line 1:36: token recognition error at: 'Å'");
22682268
}
22692269

2270+
public void testInvalidRemoteClusterPattern() {
2271+
expectError("from \"rem:ote\":index", "cluster string [rem:ote] must not contain ':'");
2272+
}
2273+
22702274
private LogicalPlan unresolvedRelation(String index) {
22712275
return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, index), false, List.of(), IndexMode.STANDARD, null, "FROM");
22722276
}

0 commit comments

Comments
 (0)