Skip to content

Commit 45e7e24

Browse files
s1monws1monw
authored
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 3a147b4 commit 45e7e24

File tree

11 files changed

+64
-67
lines changed

11 files changed

+64
-67
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
@@ -263,7 +263,7 @@ protected void parseCreateField(ParseContext context, List<IndexableField> field
263263
if (fieldType().isEnabled() == false) {
264264
return;
265265
}
266-
for (ParseContext.Document document : context.docs()) {
266+
for (ParseContext.Document document : context) {
267267
final List<String> paths = new ArrayList<>(document.getFields().size());
268268
String previousPath = ""; // used as a sentinel - field names can't be empty
269269
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
@@ -267,9 +267,6 @@ public void preParse(ParseContext context) throws IOException {
267267
super.parse(context);
268268
}
269269

270-
@Override
271-
public void postParse(ParseContext context) throws IOException {}
272-
273270
@Override
274271
protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {
275272
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
@@ -172,9 +172,6 @@ private IndexFieldMapper(MappedFieldType fieldType, Settings indexSettings) {
172172
@Override
173173
public void preParse(ParseContext context) throws IOException {}
174174

175-
@Override
176-
public void postParse(ParseContext context) throws IOException {}
177-
178175
@Override
179176
protected void parseCreateField(ParseContext context, List<IndexableField> fields) throws IOException {}
180177

‎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) {

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

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

3131
import java.util.ArrayList;
32+
import java.util.Collections;
3233
import java.util.Iterator;
3334
import java.util.List;
3435

35-
public abstract class ParseContext {
36+
public abstract class ParseContext implements Iterable<ParseContext.Document>{
3637

3738
/** Fork of {@link org.apache.lucene.document.Document} with additional functionality. */
3839
public static class Document implements Iterable<IndexableField> {
@@ -171,6 +172,11 @@ private FilterParseContext(ParseContext in) {
171172
this.in = in;
172173
}
173174

175+
@Override
176+
public Iterable<Document> nonRootDocuments() {
177+
return in.nonRootDocuments();
178+
}
179+
174180
@Override
175181
public DocumentMapperParser docMapperParser() {
176182
return in.docMapperParser();
@@ -211,11 +217,6 @@ public Document rootDoc() {
211217
return in.rootDoc();
212218
}
213219

214-
@Override
215-
public List<Document> docs() {
216-
return in.docs();
217-
}
218-
219220
@Override
220221
public Document doc() {
221222
return in.doc();
@@ -280,6 +281,11 @@ public void addDynamicMapper(Mapper update) {
280281
public List<Mapper> getDynamicMappers() {
281282
return in.getDynamicMappers();
282283
}
284+
285+
@Override
286+
public Iterator<Document> iterator() {
287+
return in.iterator();
288+
}
283289
}
284290

285291
public static class InternalParseContext extends ParseContext {
@@ -309,9 +315,10 @@ public static class InternalParseContext extends ParseContext {
309315

310316
private long numNestedDocs;
311317

312-
313318
private final List<Mapper> dynamicMappers;
314319

320+
private boolean docsReversed = false;
321+
315322
public InternalParseContext(@Nullable Settings indexSettings, DocumentMapperParser docMapperParser, DocumentMapper docMapper,
316323
SourceToParse source, XContentParser parser) {
317324
this.indexSettings = indexSettings;
@@ -360,8 +367,7 @@ public Document rootDoc() {
360367
return documents.get(0);
361368
}
362369

363-
@Override
364-
public List<Document> docs() {
370+
List<Document> docs() {
365371
return this.documents;
366372
}
367373

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

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

433466
/**
@@ -506,8 +539,6 @@ public boolean isWithinMultiFields() {
506539

507540
public abstract Document rootDoc();
508541

509-
public abstract List<Document> docs();
510-
511542
public abstract Document doc();
512543

513544
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
@@ -157,10 +157,6 @@ public void preParse(ParseContext context) throws IOException {
157157
super.parse(context);
158158
}
159159

160-
@Override
161-
public void postParse(ParseContext context) throws IOException {
162-
}
163-
164160
@Override
165161
public Mapper parse(ParseContext context) throws IOException {
166162
// 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
@@ -216,10 +216,6 @@ public void preParse(ParseContext context) throws IOException {
216216
super.parse(context);
217217
}
218218

219-
@Override
220-
public void postParse(ParseContext context) throws IOException {
221-
}
222-
223219
@Override
224220
public Mapper parse(ParseContext context) throws IOException {
225221
// nothing to do here, we will call it in pre parse
@@ -228,32 +224,23 @@ public Mapper parse(ParseContext context) throws IOException {
228224

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

259246
@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
@@ -269,10 +269,6 @@ public void preParse(ParseContext context) throws IOException {
269269
super.parse(context);
270270
}
271271

272-
@Override
273-
public void postParse(ParseContext context) throws IOException {
274-
}
275-
276272
@Override
277273
public Mapper parse(ParseContext context) throws IOException {
278274
// we parse in pre parse

0 commit comments

Comments
 (0)