Skip to content

Commit b9f7e45

Browse files
authored
Document that DotExpandingXContentParser outputs duplicate keys (#85063)
When we are expanding dots in fields names as part of parsing incoming documents, we may end up exposing duplicate keys, as multiple fields with dots in their names may share parts of their path, and the resulting objects are not merged with one another. This is ok as long as what's being parsed is not loaded into a map. This commit documents this behaviour and modifies the corresponding parser to throw exception when one of the methods that parse into a map is called. Also, a new test is added to DotExpandingXContentParserTests, although DocumentParserTests already has specific test for this scenario.
1 parent e27f64e commit b9f7e45

File tree

2 files changed

+48
-0
lines changed

2 files changed

+48
-0
lines changed

libs/x-content/src/main/java/org/elasticsearch/xcontent/DotExpandingXContentParser.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,23 @@
88

99
package org.elasticsearch.xcontent;
1010

11+
import org.elasticsearch.core.CheckedFunction;
12+
1113
import java.io.IOException;
1214
import java.util.ArrayDeque;
1315
import java.util.Deque;
16+
import java.util.List;
17+
import java.util.Map;
18+
import java.util.function.Supplier;
1419

1520
/**
1621
* An XContentParser that reinterprets field names containing dots as an object structure.
1722
*
1823
* A field name named {@code "foo.bar.baz":...} will be parsed instead as {@code 'foo':{'bar':{'baz':...}}}.
1924
* The token location is preserved so that error messages refer to the original content being parsed.
25+
* This parser can output duplicate keys, but that is fine given that it's used for document parsing. The mapping
26+
* lookups will return the same mapper/field type, and we never load incoming documents in a map where duplicate
27+
* keys would end up overriding each other.
2028
*/
2129
public class DotExpandingXContentParser extends FilterXContentParserWrapper {
2230

@@ -75,6 +83,37 @@ private void expandDots() throws IOException {
7583
protected XContentParser delegate() {
7684
return parsers.peek();
7785
}
86+
87+
@Override
88+
public Map<String, Object> map() throws IOException {
89+
throw new UnsupportedOperationException();
90+
}
91+
92+
@Override
93+
public Map<String, Object> mapOrdered() throws IOException {
94+
throw new UnsupportedOperationException();
95+
}
96+
97+
@Override
98+
public Map<String, String> mapStrings() throws IOException {
99+
throw new UnsupportedOperationException();
100+
}
101+
102+
@Override
103+
public <T> Map<String, T> map(Supplier<Map<String, T>> mapFactory, CheckedFunction<XContentParser, T, IOException> mapValueParser)
104+
throws IOException {
105+
throw new UnsupportedOperationException();
106+
}
107+
108+
@Override
109+
public List<Object> list() throws IOException {
110+
throw new UnsupportedOperationException();
111+
}
112+
113+
@Override
114+
public List<Object> listOrderedMap() throws IOException {
115+
throw new UnsupportedOperationException();
116+
}
78117
}
79118

80119
private static String[] splitAndValidatePath(String fullFieldPath) {

libs/x-content/src/test/java/org/elasticsearch/xcontent/DotExpandingXContentParserTests.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@ public class DotExpandingXContentParserTests extends ESTestCase {
1919
private void assertXContentMatches(String dotsExpanded, String withDots) throws IOException {
2020
XContentParser inputParser = createParser(JsonXContent.jsonXContent, withDots);
2121
XContentParser expandedParser = DotExpandingXContentParser.expandDots(inputParser);
22+
expandedParser.allowDuplicateKeys(true);
2223

2324
XContentBuilder actualOutput = XContentBuilder.builder(JsonXContent.jsonXContent).copyCurrentStructure(expandedParser);
2425
assertEquals(dotsExpanded, Strings.toString(actualOutput));
2526

2627
XContentParser expectedParser = createParser(JsonXContent.jsonXContent, dotsExpanded);
28+
expectedParser.allowDuplicateKeys(true);
2729
XContentParser actualParser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, withDots));
2830
XContentParser.Token currentToken;
2931
while ((currentToken = actualParser.nextToken()) != null) {
@@ -101,6 +103,13 @@ public void testTrailingDotsAreStripped() throws IOException {
101103

102104
}
103105

106+
public void testDuplicateKeys() throws IOException {
107+
assertXContentMatches("""
108+
{"test":{"with":{"dots1":"value1"}},"test":{"with":{"dots2":"value2"}}}""", """
109+
{ "test.with.dots1" : "value1",
110+
"test.with.dots2" : "value2"}""");
111+
}
112+
104113
public void testSkipChildren() throws IOException {
105114
XContentParser parser = DotExpandingXContentParser.expandDots(createParser(JsonXContent.jsonXContent, """
106115
{ "test.with.dots" : "value", "nodots" : "value2" }"""));

0 commit comments

Comments
 (0)