Skip to content

Commit 1b9320b

Browse files
s1monws1monw
committed
Restrict Document list access in ParseContext (#29463)
Today we expose a mutable list of documents in ParseContext via ParseContext#docs(). This, on the one hand places knowledge how to access nested documnts in multiple places and on the other allows for potential illegal access to nested only docs after the docs are reversed. This change restricts the access and streamlines nested / non-root doc access.
1 parent 00a8939 commit 1b9320b

File tree

11 files changed

+64
-66
lines changed

11 files changed

+64
-66
lines changed

‎server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ ParsedDocument parseDocument(SourceToParse source) throws MapperParsingException
7676
throw new IllegalStateException("found leftover path elements: " + remainingPath);
7777
}
7878

79-
reverseOrder(context);
79+
context.postParse();
8080

8181
return parsedDocument(source, context, createDynamicUpdate(mapping, docMapper, context.getDynamicMappers()));
8282
}
@@ -141,12 +141,6 @@ private static boolean isEmptyDoc(Mapping mapping, XContentParser parser) throws
141141
return false;
142142
}
143143

144-
private static void reverseOrder(ParseContext.InternalParseContext context) {
145-
// reverse the order of docs for nested docs support, parent should be last
146-
if (context.docs().size() > 1) {
147-
Collections.reverse(context.docs());
148-
}
149-
}
150144

151145
private static ParsedDocument parsedDocument(SourceToParse source, ParseContext.InternalParseContext context, Mapping update) {
152146
return new ParsedDocument(

‎server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
273273
if (fieldType().isEnabled() == false) {
274274
return;
275275
}
276-
for (ParseContext.Document document : context.docs()) {
276+
for (ParseContext.Document document : context) {
277277
final List<String> paths = new ArrayList<>(document.getFields().size());
278278
String previousPath = ""; // used as a sentinel - field names can't be empty
279279
for (IndexableField field : document.getFields()) {

‎server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,9 +288,6 @@ public void preParse(ParseContext context) throws IOException {
288288
super.parse(context);
289289
}
290290

291-
@Override
292-
public void postParse(ParseContext context) throws IOException {}
293-
294291
@Override
295292
protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {
296293
if (fieldType.indexOptions() != IndexOptions.NONE || fieldType.stored()) {

‎server/src/main/java/org/elasticsearch/index/mapper/IndexFieldMapper.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,6 @@ private IndexFieldMapper(MappedFieldType fieldType, Settings indexSettings) {
176176
@Override
177177
public void preParse(ParseContext context) throws IOException {}
178178

179-
@Override
180-
public void postParse(ParseContext context) throws IOException {}
181-
182179
@Override
183180
protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {}
184181

‎server/src/main/java/org/elasticsearch/index/mapper/MetadataFieldMapper.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,9 @@ protected MetadataFieldMapper(String simpleName, MappedFieldType fieldType, Mapp
6464
/**
6565
* Called after {@link FieldMapper#parse(ParseContext)} on the {@link RootObjectMapper}.
6666
*/
67-
public abstract void postParse(ParseContext context) throws IOException;
67+
public void postParse(ParseContext context) throws IOException {
68+
// do nothing
69+
}
6870

6971
@Override
7072
public MetadataFieldMapper merge(Mapper mergeWith, boolean updateAllTypes) {

‎server/src/main/java/org/elasticsearch/index/mapper/ParseContext.java‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@
3131
import org.elasticsearch.common.xcontent.XContentParser;
3232

3333
import java.util.ArrayList;
34+
import java.util.Collections;
3435
import java.util.Iterator;
3536
import java.util.List;
3637

37-
public abstract class ParseContext {
38+
public abstract class ParseContext implements Iterable<ParseContext.Document>{
3839

3940
/** Fork of {@link org.apache.lucene.document.Document} with additional functionality. */
4041
public static class Document implements Iterable<IndexableField> {
@@ -173,6 +174,11 @@ private FilterParseContext(ParseContext in) {
173174
this.in = in;
174175
}
175176

177+
@Override
178+
public Iterable<Document> nonRootDocuments() {
179+
return in.nonRootDocuments();
180+
}
181+
176182
@Override
177183
public DocumentMapperParser docMapperParser() {
178184
return in.docMapperParser();
@@ -213,11 +219,6 @@ public Document rootDoc() {
213219
return in.rootDoc();
214220
}
215221

216-
@Override
217-
public List<Document> docs() {
218-
return in.docs();
219-
}
220-
221222
@Override
222223
public Document doc() {
223224
return in.doc();
@@ -287,6 +288,11 @@ public void addDynamicMapper(Mapper update) {
287288
public List<Mapper> getDynamicMappers() {
288289
return in.getDynamicMappers();
289290
}
291+
292+
@Override
293+
public Iterator<Document> iterator() {
294+
return in.iterator();
295+
}
290296
}
291297

292298
public static class InternalParseContext extends ParseContext {
@@ -316,6 +322,8 @@ public static class InternalParseContext extends ParseContext {
316322

317323
private final List<Mapper> dynamicMappers;
318324

325+
private boolean docsReversed = false;
326+
319327
public InternalParseContext(@Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper,
320328
SourceToParse source, XContentParser parser) {
321329
this.indexSettings = indexSettings;
@@ -363,8 +371,7 @@ public Document rootDoc() {
363371
return documents.get(0);
364372
}
365373

366-
@Override
367-
public List<Document> docs() {
374+
List<Document> docs() {
368375
return this.documents;
369376
}
370377

@@ -427,8 +434,35 @@ public void addDynamicMapper(Mapper mapper) {
427434
public List<Mapper> getDynamicMappers() {
428435
return dynamicMappers;
429436
}
437+
438+
@Override
439+
public Iterable<Document> nonRootDocuments() {
440+
if (docsReversed) {
441+
throw new IllegalStateException("documents are already reversed");
442+
}
443+
return documents.subList(1, documents.size());
444+
}
445+
446+
void postParse() {
447+
// reverse the order of docs for nested docs support, parent should be last
448+
if (documents.size() > 1) {
449+
docsReversed = true;
450+
Collections.reverse(documents);
451+
}
452+
}
453+
454+
@Override
455+
public Iterator<Document> iterator() {
456+
return documents.iterator();
457+
}
430458
}
431459

460+
/**
461+
* Returns an Iterable over all non-root documents. If there are no non-root documents
462+
* the iterable will return an empty iterator.
463+
*/
464+
public abstract Iterable<Document> nonRootDocuments();
465+
432466
public abstract DocumentMapperParser docMapperParser();
433467

434468
/** Return a view of this {@link ParseContext} that changes the return
@@ -523,8 +557,6 @@ public boolean isWithinMultiFields() {
523557

524558
public abstract Document rootDoc();
525559

526-
public abstract List<Document> docs();
527-
528560
public abstract Document doc();
529561

530562
protected abstract void addDoc(Document doc);

‎server/src/main/java/org/elasticsearch/index/mapper/RoutingFieldMapper.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,6 @@ public void preParse(ParseContext context) throws IOException {
156156
super.parse(context);
157157
}
158158

159-
@Override
160-
public void postParse(ParseContext context) throws IOException {
161-
}
162-
163159
@Override
164160
public Mapper parse(ParseContext context) throws IOException {
165161
// no need ot parse here, we either get the routing in the sourceToParse

‎server/src/main/java/org/elasticsearch/index/mapper/SeqNoFieldMapper.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,9 @@ public void postParse(ParseContext context) throws IOException {
253253
// we share the parent docs fields to ensure good compression
254254
SequenceIDFields seqID = context.seqID();
255255
assert seqID != null;
256-
int numDocs = context.docs().size();
257256
final Version versionCreated = context.mapperService().getIndexSettings().getIndexVersionCreated();
258257
final boolean includePrimaryTerm = versionCreated.before(Version.V_6_1_0);
259-
for (int i = 1; i < numDocs; i++) {
260-
final Document doc = context.docs().get(i);
258+
for (Document doc : context.nonRootDocuments()) {
261259
doc.add(seqID.seqNo);
262260
doc.add(seqID.seqNoDocValue);
263261
if (includePrimaryTerm) {

‎server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java‎

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,6 @@ public void preParse(ParseContext context) throws IOException {
220220
super.parse(context);
221221
}
222222

223-
@Override
224-
public void postParse(ParseContext context) throws IOException {
225-
}
226-
227223
@Override
228224
public Mapper parse(ParseContext context) throws IOException {
229225
// nothing to do here, we will call it in pre parse
@@ -232,32 +228,23 @@ public Mapper parse(ParseContext context) throws IOException {
232228

233229
@Override
234230
protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {
235-
if (!enabled) {
236-
return;
237-
}
238-
if (!fieldType().stored()) {
239-
return;
240-
}
241231
BytesReference source = context.sourceToParse().source();
242-
// Percolate and tv APIs may not set the source and that is ok, because these APIs will not index any data
243-
if (source == null) {
244-
return;
245-
}
246-
247-
if (filter != null) {
248-
// we don't update the context source if we filter, we want to keep it as is...
249-
Tuple<XContentType, Map<String, Object>> mapTuple =
250-
XContentHelper.convertToMap(source, true, context.sourceToParse().getXContentType());
251-
Map<String, Object> filteredSource = filter.apply(mapTuple.v2());
252-
BytesStreamOutput bStream = new BytesStreamOutput();
253-
XContentType contentType = mapTuple.v1();
254-
XContentBuilder builder = XContentFactory.contentBuilder(contentType, bStream).map(filteredSource);
255-
builder.close();
256-
257-
source = bStream.bytes();
232+
if (enabled && fieldType().stored() && source != null) {
233+
// Percolate and tv APIs may not set the source and that is ok, because these APIs will not index any data
234+
if (filter != null) {
235+
// we don't update the context source if we filter, we want to keep it as is...
236+
Tuple<XContentType, Map<String, Object>> mapTuple =
237+
XContentHelper.convertToMap(source, true, context.sourceToParse().getXContentType());
238+
Map<String, Object> filteredSource = filter.apply(mapTuple.v2());
239+
BytesStreamOutput bStream = new BytesStreamOutput();
240+
XContentType contentType = mapTuple.v1();
241+
XContentBuilder builder = XContentFactory.contentBuilder(contentType, bStream).map(filteredSource);
242+
builder.close();
243+
source = bStream.bytes();
244+
}
245+
BytesRef ref = source.toBytesRef();
246+
fields.add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
258247
}
259-
BytesRef ref = source.toBytesRef();
260-
fields.add(new StoredField(fieldType().name(), ref.bytes, ref.offset, ref.length));
261248
}
262249

263250
@Override

‎server/src/main/java/org/elasticsearch/index/mapper/TypeFieldMapper.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,10 +284,6 @@ public void preParse(ParseContext context) throws IOException {
284284
super.parse(context);
285285
}
286286

287-
@Override
288-
public void postParse(ParseContext context) throws IOException {
289-
}
290-
291287
@Override
292288
public Mapper parse(ParseContext context) throws IOException {
293289
// we parse in pre parse

0 commit comments

Comments
 (0)