Skip to content

Commit 075aa8a

Browse files
lktscbuescher
lkts
authored and
cbuescher
committed
Prevent synthetic field loaders accessing stored fields from using stale data (elastic#112173)
1 parent cffbfd7 commit 075aa8a

File tree

22 files changed

+517
-227
lines changed

22 files changed

+517
-227
lines changed

‎docs/changelog/112173.yaml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 112173
2+
summary: Prevent synthetic field loaders accessing stored fields from using stale
3+
data
4+
area: Mapping
5+
type: bug
6+
issues:
7+
- 112156

‎modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
447447
"field [" + fullPath() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
448448
);
449449
}
450-
return new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), leafName(), null) {
450+
return new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), leafName()) {
451451
@Override
452452
protected void write(XContentBuilder b, Object value) throws IOException {
453453
b.value((String) value);

‎plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
584584
);
585585
}
586586
if (fieldType.stored()) {
587-
return new StringStoredFieldFieldLoader(fullPath(), leafName(), null) {
587+
return new StringStoredFieldFieldLoader(fullPath(), leafName()) {
588588
@Override
589589
protected void write(XContentBuilder b, Object value) throws IOException {
590590
b.value((String) value);

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/indices.create/20_synthetic_source.yml‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1250,8 +1250,6 @@ empty nested object sorted as a first document:
12501250
- match: { hits.hits.1._source.name: B }
12511251
- match: { hits.hits.1._source.nested.a: "b" }
12521252

1253-
1254-
12551253
---
12561254
subobjects auto:
12571255
- requires:
@@ -1339,3 +1337,54 @@ subobjects auto:
13391337
- match: { hits.hits.3._source.id: 4 }
13401338
- match: { hits.hits.3._source.auto_obj.foo: 40 }
13411339
- match: { hits.hits.3._source.auto_obj.foo\.bar: 400 }
1340+
1341+
---
1342+
# 112156
1343+
stored field under object with store_array_source:
1344+
- requires:
1345+
cluster_features: ["mapper.track_ignored_source"]
1346+
reason: requires tracking ignored source
1347+
1348+
- do:
1349+
indices.create:
1350+
index: test
1351+
body:
1352+
settings:
1353+
index:
1354+
sort.field: "name"
1355+
sort.order: "asc"
1356+
mappings:
1357+
_source:
1358+
mode: synthetic
1359+
properties:
1360+
name:
1361+
type: keyword
1362+
obj:
1363+
store_array_source: true
1364+
properties:
1365+
foo:
1366+
type: keyword
1367+
store: true
1368+
1369+
- do:
1370+
bulk:
1371+
index: test
1372+
refresh: true
1373+
body:
1374+
- '{ "create": { } }'
1375+
- '{ "name": "B", "obj": null }'
1376+
- '{ "create": { } }'
1377+
- '{ "name": "A", "obj": [ { "foo": "hello_from_the_other_side" } ] }'
1378+
1379+
- match: { errors: false }
1380+
1381+
- do:
1382+
search:
1383+
index: test
1384+
sort: name
1385+
1386+
- match: { hits.total.value: 2 }
1387+
- match: { hits.hits.0._source.name: A }
1388+
- match: { hits.hits.0._source.obj: [ { "foo": "hello_from_the_other_side" } ] }
1389+
- match: { hits.hits.1._source.name: B }
1390+
- match: { hits.hits.1._source.obj: null }

‎server/src/internalClusterTest/java/org/elasticsearch/search/fieldcaps/FieldCapabilitiesIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,7 +859,7 @@ protected String contentType() {
859859

860860
@Override
861861
public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
862-
return new StringStoredFieldFieldLoader(fullPath(), leafName(), null) {
862+
return new StringStoredFieldFieldLoader(fullPath(), leafName()) {
863863
@Override
864864
protected void write(XContentBuilder b, Object value) throws IOException {
865865
BytesRef ref = (BytesRef) value;

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

Lines changed: 73 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.io.IOException;
1616
import java.util.ArrayList;
1717
import java.util.Arrays;
18+
import java.util.Collection;
1819
import java.util.List;
1920
import java.util.Map;
2021
import java.util.stream.Stream;
@@ -28,29 +29,44 @@
2829
* stored in a different field in case of ignore_malformed being enabled.
2930
*/
3031
public class CompositeSyntheticFieldLoader implements SourceLoader.SyntheticFieldLoader {
31-
private final String fieldName;
32+
private final String leafFieldName;
3233
private final String fullFieldName;
33-
private final SyntheticFieldLoaderLayer[] parts;
34-
private boolean hasValue;
34+
private final Collection<Layer> parts;
35+
private boolean storedFieldLoadersHaveValues;
36+
private boolean docValuesLoadersHaveValues;
3537

36-
public CompositeSyntheticFieldLoader(String fieldName, String fullFieldName, SyntheticFieldLoaderLayer... parts) {
37-
this.fieldName = fieldName;
38+
public CompositeSyntheticFieldLoader(String leafFieldName, String fullFieldName, Layer... parts) {
39+
this(leafFieldName, fullFieldName, Arrays.asList(parts));
40+
}
41+
42+
public CompositeSyntheticFieldLoader(String leafFieldName, String fullFieldName, Collection<Layer> parts) {
43+
this.leafFieldName = leafFieldName;
3844
this.fullFieldName = fullFieldName;
3945
this.parts = parts;
40-
this.hasValue = false;
46+
this.storedFieldLoadersHaveValues = false;
47+
this.docValuesLoadersHaveValues = false;
4148
}
4249

4350
@Override
4451
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
45-
return Arrays.stream(parts).flatMap(SyntheticFieldLoaderLayer::storedFieldLoaders).map(e -> Map.entry(e.getKey(), values -> {
46-
hasValue = true;
47-
e.getValue().load(values);
52+
return parts.stream().flatMap(Layer::storedFieldLoaders).map(e -> Map.entry(e.getKey(), new StoredFieldLoader() {
53+
@Override
54+
public void advanceToDoc(int docId) {
55+
storedFieldLoadersHaveValues = false;
56+
e.getValue().advanceToDoc(docId);
57+
}
58+
59+
@Override
60+
public void load(List<Object> newValues) {
61+
storedFieldLoadersHaveValues = true;
62+
e.getValue().load(newValues);
63+
}
4864
}));
4965
}
5066

5167
@Override
5268
public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException {
53-
var loaders = new ArrayList<DocValuesLoader>(parts.length);
69+
var loaders = new ArrayList<DocValuesLoader>(parts.size());
5470
for (var part : parts) {
5571
var partLoader = part.docValuesLoader(leafReader, docIdsInLeaf);
5672
if (partLoader != null) {
@@ -63,38 +79,40 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
6379
}
6480

6581
return docId -> {
82+
this.docValuesLoadersHaveValues = false;
83+
6684
boolean hasDocs = false;
6785
for (var loader : loaders) {
6886
hasDocs |= loader.advanceToDoc(docId);
6987
}
7088

71-
this.hasValue |= hasDocs;
89+
this.docValuesLoadersHaveValues = hasDocs;
7290
return hasDocs;
7391
};
7492
}
7593

7694
@Override
7795
public boolean hasValue() {
78-
return hasValue;
96+
return storedFieldLoadersHaveValues || docValuesLoadersHaveValues;
7997
}
8098

8199
@Override
82100
public void write(XContentBuilder b) throws IOException {
83-
var totalCount = Arrays.stream(parts).mapToLong(SyntheticFieldLoaderLayer::valueCount).sum();
101+
var totalCount = parts.stream().mapToLong(Layer::valueCount).sum();
84102

85103
if (totalCount == 0) {
86104
return;
87105
}
88106

89107
if (totalCount == 1) {
90-
b.field(fieldName);
108+
b.field(leafFieldName);
91109
for (var part : parts) {
92110
part.write(b);
93111
}
94112
return;
95113
}
96114

97-
b.startArray(fieldName);
115+
b.startArray(leafFieldName);
98116
for (var part : parts) {
99117
part.write(b);
100118
}
@@ -113,9 +131,9 @@ public String fieldName() {
113131
* Note that the contract of {@link SourceLoader.SyntheticFieldLoader#write(XContentBuilder)}
114132
* is slightly different here since it only needs to write field values without encompassing object or array.
115133
*/
116-
public interface SyntheticFieldLoaderLayer extends SourceLoader.SyntheticFieldLoader {
134+
public interface Layer extends SourceLoader.SyntheticFieldLoader {
117135
/**
118-
* Number of values that this loader will write.
136+
* Number of values that this loader will write for a given document.
119137
* @return
120138
*/
121139
long valueCount();
@@ -125,12 +143,30 @@ public interface SyntheticFieldLoaderLayer extends SourceLoader.SyntheticFieldLo
125143
* Layer that loads malformed values stored in a dedicated field with a conventional name.
126144
* @see IgnoreMalformedStoredValues
127145
*/
128-
public static class MalformedValuesLayer implements SyntheticFieldLoaderLayer {
146+
public static class MalformedValuesLayer extends StoredFieldLayer {
147+
public MalformedValuesLayer(String fieldName) {
148+
super(IgnoreMalformedStoredValues.name(fieldName));
149+
}
150+
151+
@Override
152+
protected void writeValue(Object value, XContentBuilder b) throws IOException {
153+
if (value instanceof BytesRef r) {
154+
XContentDataHelper.decodeAndWrite(b, r);
155+
} else {
156+
b.value(value);
157+
}
158+
}
159+
}
160+
161+
/**
162+
* Layer that loads field values from a provided stored field.
163+
*/
164+
public abstract static class StoredFieldLayer implements Layer {
129165
private final String fieldName;
130166
private List<Object> values;
131167

132-
public MalformedValuesLayer(String fieldName) {
133-
this.fieldName = IgnoreMalformedStoredValues.name(fieldName);
168+
public StoredFieldLayer(String fieldName) {
169+
this.fieldName = fieldName;
134170
this.values = emptyList();
135171
}
136172

@@ -141,7 +177,17 @@ public long valueCount() {
141177

142178
@Override
143179
public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
144-
return Stream.of(Map.entry(fieldName, values -> this.values = values));
180+
return Stream.of(Map.entry(fieldName, new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
181+
@Override
182+
public void advanceToDoc(int docId) {
183+
values = emptyList();
184+
}
185+
186+
@Override
187+
public void load(List<Object> newValues) {
188+
values = newValues;
189+
}
190+
}));
145191
}
146192

147193
@Override
@@ -157,15 +203,15 @@ public boolean hasValue() {
157203
@Override
158204
public void write(XContentBuilder b) throws IOException {
159205
for (Object v : values) {
160-
if (v instanceof BytesRef r) {
161-
XContentDataHelper.decodeAndWrite(b, r);
162-
} else {
163-
b.value(v);
164-
}
206+
writeValue(v, b);
165207
}
166-
values = emptyList();
167208
}
168209

210+
/**
211+
* Write a value read from stored field using appropriate format.
212+
*/
213+
protected abstract void writeValue(Object value, XContentBuilder b) throws IOException;
214+
169215
@Override
170216
public String fieldName() {
171217
return fieldName;

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,17 @@ private static class Stored extends IgnoreMalformedStoredValues {
107107

108108
@Override
109109
public Stream<Map.Entry<String, SourceLoader.SyntheticFieldLoader.StoredFieldLoader>> storedFieldLoaders() {
110-
return Stream.of(Map.entry(name(fieldName), values -> this.values = values));
110+
return Stream.of(Map.entry(name(fieldName), new SourceLoader.SyntheticFieldLoader.StoredFieldLoader() {
111+
@Override
112+
public void advanceToDoc(int docId) {
113+
values = emptyList();
114+
}
115+
116+
@Override
117+
public void load(List<Object> newValues) {
118+
values = newValues;
119+
}
120+
}));
111121
}
112122

113123
@Override

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import java.io.IOException;
4848
import java.net.InetAddress;
4949
import java.time.ZoneId;
50+
import java.util.ArrayList;
5051
import java.util.Arrays;
5152
import java.util.Collection;
5253
import java.util.Collections;
@@ -625,7 +626,9 @@ public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
625626
"field [" + fullPath() + "] of type [" + typeName() + "] doesn't support synthetic source because it declares copy_to"
626627
);
627628
}
628-
return new SortedSetDocValuesSyntheticFieldLoader(fullPath(), leafName(), null, ignoreMalformed) {
629+
630+
var layers = new ArrayList<CompositeSyntheticFieldLoader.Layer>();
631+
layers.add(new SortedSetDocValuesSyntheticFieldLoaderLayer(fullPath()) {
629632
@Override
630633
protected BytesRef convert(BytesRef value) {
631634
byte[] bytes = Arrays.copyOfRange(value.bytes, value.offset, value.offset + value.length);
@@ -637,6 +640,12 @@ protected BytesRef preserve(BytesRef value) {
637640
// No need to copy because convert has made a deep copy
638641
return value;
639642
}
640-
};
643+
});
644+
645+
if (ignoreMalformed) {
646+
layers.add(new CompositeSyntheticFieldLoader.MalformedValuesLayer(fullPath()));
647+
}
648+
649+
return new CompositeSyntheticFieldLoader(leafName(), fullPath(), layers);
641650
}
642651
}

0 commit comments

Comments
 (0)