Skip to content

Commit 610230f

Browse files
author
Hendrik Muhs
authored
[ML-DataFrame] validate group name to not contain invalid characters (elastic#42292)
disallows of creating groupBy field with '[', ']', '>' in the name to be consistent with aggregations
1 parent 9a152ee commit 610230f

File tree

2 files changed

+40
-0
lines changed
  • x-pack/plugin/core/src

2 files changed

+40
-0
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/GroupConfig.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import org.elasticsearch.common.xcontent.XContentFactory;
2121
import org.elasticsearch.common.xcontent.XContentParser;
2222
import org.elasticsearch.common.xcontent.XContentType;
23+
import org.elasticsearch.search.aggregations.AggregatorFactories;
2324
import org.elasticsearch.xpack.core.dataframe.DataFrameField;
2425
import org.elasticsearch.xpack.core.dataframe.DataFrameMessages;
2526
import org.elasticsearch.xpack.core.dataframe.utils.ExceptionsHelper;
@@ -29,6 +30,7 @@
2930
import java.util.Locale;
3031
import java.util.Map;
3132
import java.util.Objects;
33+
import java.util.regex.Matcher;
3234

3335
import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
3436

@@ -135,6 +137,7 @@ public static GroupConfig fromXContent(final XContentParser parser, boolean leni
135137

136138
private static Map<String, SingleGroupSource> parseGroupConfig(final XContentParser parser,
137139
boolean lenient) throws IOException {
140+
Matcher validAggMatcher = AggregatorFactories.VALID_AGG_NAME.matcher("");
138141
LinkedHashMap<String, SingleGroupSource> groups = new LinkedHashMap<>();
139142

140143
// be parsing friendly, whether the token needs to be advanced or not (similar to what ObjectParser does)
@@ -150,6 +153,11 @@ private static Map<String, SingleGroupSource> parseGroupConfig(final XContentPar
150153

151154
ensureExpectedToken(XContentParser.Token.FIELD_NAME, token, parser::getTokenLocation);
152155
String destinationFieldName = parser.currentName();
156+
if (validAggMatcher.reset(destinationFieldName).matches() == false) {
157+
throw new ParsingException(parser.getTokenLocation(), "Invalid group name [" + destinationFieldName
158+
+ "]. Group names can contain any character except '[', ']', and '>'");
159+
}
160+
153161
token = parser.nextToken();
154162
ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser::getTokenLocation);
155163
token = parser.nextToken();

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/dataframe/transforms/pivot/GroupConfigTests.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package org.elasticsearch.xpack.core.dataframe.transforms.pivot;
88

9+
import org.elasticsearch.common.ParsingException;
910
import org.elasticsearch.common.bytes.BytesReference;
1011
import org.elasticsearch.common.io.stream.Writeable.Reader;
1112
import org.elasticsearch.common.xcontent.ToXContent;
@@ -27,6 +28,9 @@
2728

2829
public class GroupConfigTests extends AbstractSerializingTestCase<GroupConfig> {
2930

31+
// array of illegal characters, see {@link AggregatorFactories#VALID_AGG_NAME}
32+
private static final char[] ILLEGAL_FIELD_NAME_CHARACTERS = {'[', ']', '>'};
33+
3034
public static GroupConfig randomGroupConfig() {
3135
Map<String, Object> source = new LinkedHashMap<>();
3236
Map<String, SingleGroupSource> groups = new LinkedHashMap<>();
@@ -88,6 +92,34 @@ public void testEmptyGroupBy() throws IOException {
8892
}
8993
}
9094

95+
public void testInvalidGroupByNames() throws IOException {
96+
97+
String invalidName = randomAlphaOfLengthBetween(0, 5)
98+
+ ILLEGAL_FIELD_NAME_CHARACTERS[randomIntBetween(0, ILLEGAL_FIELD_NAME_CHARACTERS.length - 1)]
99+
+ randomAlphaOfLengthBetween(0, 5);
100+
101+
XContentBuilder source = JsonXContent.contentBuilder()
102+
.startObject()
103+
.startObject(invalidName)
104+
.startObject("terms")
105+
.field("field", "user")
106+
.endObject()
107+
.endObject()
108+
.endObject();
109+
110+
// lenient, passes but reports invalid
111+
try (XContentParser parser = createParser(source)) {
112+
GroupConfig groupConfig = GroupConfig.fromXContent(parser, true);
113+
assertFalse(groupConfig.isValid());
114+
}
115+
116+
// strict throws
117+
try (XContentParser parser = createParser(source)) {
118+
Exception e = expectThrows(ParsingException.class, () -> GroupConfig.fromXContent(parser, false));
119+
assertTrue(e.getMessage().startsWith("Invalid group name"));
120+
}
121+
}
122+
91123
private static Map<String, Object> getSource(SingleGroupSource groupSource) {
92124
try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
93125
XContentBuilder content = groupSource.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS);

0 commit comments

Comments
 (0)