Skip to content

Commit 12c3c15

Browse files
nik9000nik9000
authored
Check for runtime field loops in queries (#61927)
We were checking for loops in queries before, but we had an "off by one" error where we wouldn't notice the "top level" runtime field when detecting a loop. So the error message would be wrong. I also caught a few bugs with query generation caused by missing `@Override` annotations and fixed a few of them. There is a bug with `regexp` queries with match options that I'm not fixing in this PR but will get to later. Relates to #59332
1 parent e743ed6 commit 12c3c15

18 files changed

+417
-291
lines changed

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/AbstractScriptMappedFieldType.java

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@
1212
import org.apache.lucene.search.spans.SpanMultiTermQueryWrapper;
1313
import org.apache.lucene.search.spans.SpanQuery;
1414
import org.elasticsearch.ElasticsearchException;
15+
import org.elasticsearch.common.TriFunction;
1516
import org.elasticsearch.common.geo.ShapeRelation;
1617
import org.elasticsearch.common.time.DateMathParser;
1718
import org.elasticsearch.common.unit.Fuzziness;
1819
import org.elasticsearch.index.mapper.MappedFieldType;
1920
import org.elasticsearch.index.mapper.TextSearchInfo;
2021
import org.elasticsearch.index.query.QueryShardContext;
2122
import org.elasticsearch.script.Script;
23+
import org.elasticsearch.search.lookup.SearchLookup;
2224

2325
import java.io.IOException;
2426
import java.time.ZoneId;
@@ -31,12 +33,19 @@
3133
/**
3234
* Abstract base {@linkplain MappedFieldType} for scripted fields.
3335
*/
34-
abstract class AbstractScriptMappedFieldType extends MappedFieldType {
36+
abstract class AbstractScriptMappedFieldType<LeafFactory> extends MappedFieldType {
3537
protected final Script script;
38+
private final TriFunction<String, Map<String, Object>, SearchLookup, LeafFactory> factory;
3639

37-
AbstractScriptMappedFieldType(String name, Script script, Map<String, String> meta) {
40+
AbstractScriptMappedFieldType(
41+
String name,
42+
Script script,
43+
TriFunction<String, Map<String, Object>, SearchLookup, LeafFactory> factory,
44+
Map<String, String> meta
45+
) {
3846
super(name, false, false, TextSearchInfo.SIMPLE_MATCH_ONLY, meta);
3947
this.script = script;
48+
this.factory = factory;
4049
}
4150

4251
protected abstract String runtimeType();
@@ -61,6 +70,26 @@ public final boolean isAggregatable() {
6170
return true;
6271
}
6372

73+
/**
74+
* Create a script leaf factory.
75+
*/
76+
protected final LeafFactory leafFactory(SearchLookup searchLookup) {
77+
return factory.apply(name(), script.getParams(), searchLookup);
78+
}
79+
80+
/**
81+
* Create a script leaf factory for queries.
82+
*/
83+
protected final LeafFactory leafFactory(QueryShardContext context) {
84+
/*
85+
* Forking here causes us to count this field in the field data loop
86+
* detection code as though we were resolving field data for this field.
87+
* We're not, but running the query is close enough.
88+
*/
89+
return leafFactory(context.lookup().forkAndTrackFieldReferences(name()));
90+
}
91+
92+
@Override
6493
public abstract Query termsQuery(List<?> values, QueryShardContext context);
6594

6695
@Override
@@ -149,8 +178,15 @@ public SpanQuery spanPrefixQuery(String value, SpanMultiTermQueryWrapper.SpanRew
149178
}
150179

151180
private String unsupported(String query, String supported) {
152-
String thisField = "[" + name() + "] which is of type [script] with runtime_type [" + runtimeType() + "]";
153-
return "Can only use " + query + " queries on " + supported + " fields - not on " + thisField;
181+
return String.format(
182+
Locale.ROOT,
183+
"Can only use %s queries on %s fields - not on [%s] which is of type [%s] with runtime_type [%s]",
184+
query,
185+
supported,
186+
name(),
187+
RuntimeFieldMapper.CONTENT_TYPE,
188+
runtimeType()
189+
);
154190
}
155191

156192
protected final void checkAllowExpensiveQueries(QueryShardContext context) {

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/RuntimeFieldMapper.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryTyp
5353

5454
protected RuntimeFieldMapper(
5555
String simpleName,
56-
AbstractScriptMappedFieldType mappedFieldType,
56+
AbstractScriptMappedFieldType<?> mappedFieldType,
5757
MultiFields multiFields,
5858
CopyTo copyTo,
5959
String runtimeType,
@@ -88,7 +88,7 @@ protected String contentType() {
8888

8989
public static class Builder extends ParametrizedFieldMapper.Builder {
9090

91-
static final Map<String, BiFunction<Builder, BuilderContext, AbstractScriptMappedFieldType>> FIELD_TYPE_RESOLVER = Map.of(
91+
static final Map<String, BiFunction<Builder, BuilderContext, AbstractScriptMappedFieldType<?>>> FIELD_TYPE_RESOLVER = Map.of(
9292
BooleanFieldMapper.CONTENT_TYPE,
9393
(builder, context) -> {
9494
builder.formatAndLocaleNotSupported();
@@ -213,7 +213,7 @@ private static RuntimeFieldMapper toType(FieldMapper in) {
213213
private final Parameter<String> format = Parameter.stringParam(
214214
"format",
215215
true,
216-
mapper -> ((AbstractScriptMappedFieldType) mapper.fieldType()).format(),
216+
mapper -> ((AbstractScriptMappedFieldType<?>) mapper.fieldType()).format(),
217217
null
218218
).setSerializer((b, n, v) -> {
219219
if (v != null && false == v.equals(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.pattern())) {
@@ -225,7 +225,7 @@ private static RuntimeFieldMapper toType(FieldMapper in) {
225225
true,
226226
() -> null,
227227
(n, c, o) -> o == null ? null : LocaleUtils.parse(o.toString()),
228-
mapper -> ((AbstractScriptMappedFieldType) mapper.fieldType()).formatLocale()
228+
mapper -> ((AbstractScriptMappedFieldType<?>) mapper.fieldType()).formatLocale()
229229
).setSerializer((b, n, v) -> {
230230
if (v != null && false == v.equals(Locale.ROOT)) {
231231
b.field(n, v.toString());
@@ -246,7 +246,7 @@ protected List<Parameter<?>> getParameters() {
246246

247247
@Override
248248
public RuntimeFieldMapper build(BuilderContext context) {
249-
BiFunction<Builder, BuilderContext, AbstractScriptMappedFieldType> fieldTypeResolver = Builder.FIELD_TYPE_RESOLVER.get(
249+
BiFunction<Builder, BuilderContext, AbstractScriptMappedFieldType<?>> fieldTypeResolver = Builder.FIELD_TYPE_RESOLVER.get(
250250
runtimeType.getValue()
251251
);
252252
if (fieldTypeResolver == null) {

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptBooleanMappedFieldType.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,9 @@
2727
import java.util.Map;
2828
import java.util.function.Supplier;
2929

30-
public class ScriptBooleanMappedFieldType extends AbstractScriptMappedFieldType {
31-
private final BooleanScriptFieldScript.Factory scriptFactory;
32-
30+
public class ScriptBooleanMappedFieldType extends AbstractScriptMappedFieldType<BooleanScriptFieldScript.LeafFactory> {
3331
ScriptBooleanMappedFieldType(String name, Script script, BooleanScriptFieldScript.Factory scriptFactory, Map<String, String> meta) {
34-
super(name, script, meta);
35-
this.scriptFactory = scriptFactory;
32+
super(name, script, scriptFactory::newFactory, meta);
3633
}
3734

3835
@Override
@@ -71,14 +68,10 @@ public ScriptBooleanFieldData.Builder fielddataBuilder(String fullyQualifiedInde
7168
return new ScriptBooleanFieldData.Builder(name(), leafFactory(searchLookup.get()));
7269
}
7370

74-
private BooleanScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
75-
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
76-
}
77-
7871
@Override
7972
public Query existsQuery(QueryShardContext context) {
8073
checkAllowExpensiveQueries(context);
81-
return new BooleanScriptFieldExistsQuery(script, leafFactory(context.lookup()), name());
74+
return new BooleanScriptFieldExistsQuery(script, leafFactory(context), name());
8275
}
8376

8477
@Override
@@ -149,7 +142,7 @@ public Query rangeQuery(
149142
@Override
150143
public Query termQuery(Object value, QueryShardContext context) {
151144
checkAllowExpensiveQueries(context);
152-
return new BooleanScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), toBoolean(value));
145+
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), toBoolean(value));
153146
}
154147

155148
@Override
@@ -176,11 +169,11 @@ private Query termsQuery(boolean trueAllowed, boolean falseAllowed, QueryShardCo
176169
return existsQuery(context);
177170
}
178171
checkAllowExpensiveQueries(context);
179-
return new BooleanScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), true);
172+
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), true);
180173
}
181174
if (falseAllowed) {
182175
checkAllowExpensiveQueries(context);
183-
return new BooleanScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), false);
176+
return new BooleanScriptFieldTermQuery(script, leafFactory(context), name(), false);
184177
}
185178
return new MatchNoDocsQuery("neither true nor false allowed");
186179
}

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptDateMappedFieldType.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,7 @@
3737
import java.util.Map;
3838
import java.util.function.Supplier;
3939

40-
public class ScriptDateMappedFieldType extends AbstractScriptMappedFieldType {
41-
private final DateScriptFieldScript.Factory scriptFactory;
40+
public class ScriptDateMappedFieldType extends AbstractScriptMappedFieldType<DateScriptFieldScript.LeafFactory> {
4241
private final DateFormatter dateTimeFormatter;
4342

4443
ScriptDateMappedFieldType(
@@ -48,8 +47,7 @@ public class ScriptDateMappedFieldType extends AbstractScriptMappedFieldType {
4847
DateFormatter dateTimeFormatter,
4948
Map<String, String> meta
5049
) {
51-
super(name, script, meta);
52-
this.scriptFactory = scriptFactory;
50+
super(name, script, (n, params, ctx) -> scriptFactory.newFactory(n, params, ctx, dateTimeFormatter), meta);
5351
this.dateTimeFormatter = dateTimeFormatter;
5452
}
5553

@@ -84,10 +82,6 @@ public ScriptDateFieldData.Builder fielddataBuilder(String fullyQualifiedIndexNa
8482
return new ScriptDateFieldData.Builder(name(), leafFactory(lookup.get()));
8583
}
8684

87-
private DateScriptFieldScript.LeafFactory leafFactory(SearchLookup lookup) {
88-
return scriptFactory.newFactory(name(), script.getParams(), lookup, dateTimeFormatter);
89-
}
90-
9185
@Override
9286
public Query distanceFeatureQuery(Object origin, String pivot, float boost, QueryShardContext context) {
9387
checkAllowExpensiveQueries(context);
@@ -103,7 +97,7 @@ public Query distanceFeatureQuery(Object origin, String pivot, float boost, Quer
10397
TimeValue pivotTime = TimeValue.parseTimeValue(pivot, "distance_feature.pivot");
10498
return new LongScriptFieldDistanceFeatureQuery(
10599
script,
106-
leafFactory(context.lookup())::newInstance,
100+
leafFactory(context)::newInstance,
107101
name(),
108102
originLong,
109103
pivotTime.getMillis(),
@@ -115,7 +109,7 @@ public Query distanceFeatureQuery(Object origin, String pivot, float boost, Quer
115109
@Override
116110
public Query existsQuery(QueryShardContext context) {
117111
checkAllowExpensiveQueries(context);
118-
return new LongScriptFieldExistsQuery(script, leafFactory(context.lookup())::newInstance, name());
112+
return new LongScriptFieldExistsQuery(script, leafFactory(context)::newInstance, name());
119113
}
120114

121115
@Override
@@ -139,7 +133,7 @@ public Query rangeQuery(
139133
parser,
140134
context,
141135
DateFieldMapper.Resolution.MILLISECONDS,
142-
(l, u) -> new LongScriptFieldRangeQuery(script, leafFactory(context.lookup())::newInstance, name(), l, u)
136+
(l, u) -> new LongScriptFieldRangeQuery(script, leafFactory(context)::newInstance, name(), l, u)
143137
);
144138
}
145139

@@ -155,7 +149,7 @@ public Query termQuery(Object value, QueryShardContext context) {
155149
DateFieldMapper.Resolution.MILLISECONDS
156150
);
157151
checkAllowExpensiveQueries(context);
158-
return new LongScriptFieldTermQuery(script, leafFactory(context.lookup())::newInstance, name(), l);
152+
return new LongScriptFieldTermQuery(script, leafFactory(context)::newInstance, name(), l);
159153
});
160154
}
161155

@@ -179,7 +173,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
179173
);
180174
}
181175
checkAllowExpensiveQueries(context);
182-
return new LongScriptFieldTermsQuery(script, leafFactory(context.lookup())::newInstance, name(), terms);
176+
return new LongScriptFieldTermsQuery(script, leafFactory(context)::newInstance, name(), terms);
183177
});
184178
}
185179

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptDoubleMappedFieldType.java

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,9 @@
2929
import java.util.Map;
3030
import java.util.function.Supplier;
3131

32-
public class ScriptDoubleMappedFieldType extends AbstractScriptMappedFieldType {
33-
private final DoubleScriptFieldScript.Factory scriptFactory;
34-
32+
public class ScriptDoubleMappedFieldType extends AbstractScriptMappedFieldType<DoubleScriptFieldScript.LeafFactory> {
3533
ScriptDoubleMappedFieldType(String name, Script script, DoubleScriptFieldScript.Factory scriptFactory, Map<String, String> meta) {
36-
super(name, script, meta);
37-
this.scriptFactory = scriptFactory;
34+
super(name, script, scriptFactory::newFactory, meta);
3835
}
3936

4037
@Override
@@ -63,14 +60,10 @@ public ScriptDoubleFieldData.Builder fielddataBuilder(String fullyQualifiedIndex
6360
return new ScriptDoubleFieldData.Builder(name(), leafFactory(searchLookup.get()));
6461
}
6562

66-
private DoubleScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
67-
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
68-
}
69-
7063
@Override
7164
public Query existsQuery(QueryShardContext context) {
7265
checkAllowExpensiveQueries(context);
73-
return new DoubleScriptFieldExistsQuery(script, leafFactory(context.lookup()), name());
66+
return new DoubleScriptFieldExistsQuery(script, leafFactory(context), name());
7467
}
7568

7669
@Override
@@ -89,14 +82,14 @@ public Query rangeQuery(
8982
upperTerm,
9083
includeLower,
9184
includeUpper,
92-
(l, u) -> new DoubleScriptFieldRangeQuery(script, leafFactory(context.lookup()), name(), l, u)
85+
(l, u) -> new DoubleScriptFieldRangeQuery(script, leafFactory(context), name(), l, u)
9386
);
9487
}
9588

9689
@Override
9790
public Query termQuery(Object value, QueryShardContext context) {
9891
checkAllowExpensiveQueries(context);
99-
return new DoubleScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), NumberType.objectToDouble(value));
92+
return new DoubleScriptFieldTermQuery(script, leafFactory(context), name(), NumberType.objectToDouble(value));
10093
}
10194

10295
@Override
@@ -109,6 +102,6 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
109102
terms.add(Double.doubleToLongBits(NumberType.objectToDouble(value)));
110103
}
111104
checkAllowExpensiveQueries(context);
112-
return new DoubleScriptFieldTermsQuery(script, leafFactory(context.lookup()), name(), terms);
105+
return new DoubleScriptFieldTermsQuery(script, leafFactory(context), name(), terms);
113106
}
114107
}

‎x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/ScriptIpMappedFieldType.java

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,9 @@
3737
import java.util.Map;
3838
import java.util.function.Supplier;
3939

40-
public final class ScriptIpMappedFieldType extends AbstractScriptMappedFieldType {
41-
42-
private final Script script;
43-
private final IpScriptFieldScript.Factory scriptFactory;
44-
40+
public final class ScriptIpMappedFieldType extends AbstractScriptMappedFieldType<IpScriptFieldScript.LeafFactory> {
4541
ScriptIpMappedFieldType(String name, Script script, IpScriptFieldScript.Factory scriptFactory, Map<String, String> meta) {
46-
super(name, script, meta);
47-
this.script = script;
48-
this.scriptFactory = scriptFactory;
42+
super(name, script, scriptFactory::newFactory, meta);
4943
}
5044

5145
@Override
@@ -79,14 +73,10 @@ public ScriptIpFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName
7973
return new ScriptIpFieldData.Builder(name(), leafFactory(searchLookup.get()));
8074
}
8175

82-
private IpScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
83-
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
84-
}
85-
8676
@Override
8777
public Query existsQuery(QueryShardContext context) {
8878
checkAllowExpensiveQueries(context);
89-
return new IpScriptFieldExistsQuery(script, leafFactory(context.lookup()), name());
79+
return new IpScriptFieldExistsQuery(script, leafFactory(context), name());
9080
}
9181

9282
@Override
@@ -107,7 +97,7 @@ public Query rangeQuery(
10797
includeUpper,
10898
(lower, upper) -> new IpScriptFieldRangeQuery(
10999
script,
110-
leafFactory(context.lookup()),
100+
leafFactory(context),
111101
name(),
112102
new BytesRef(InetAddressPoint.encode(lower)),
113103
new BytesRef(InetAddressPoint.encode(upper))
@@ -119,14 +109,18 @@ public Query rangeQuery(
119109
public Query termQuery(Object value, QueryShardContext context) {
120110
checkAllowExpensiveQueries(context);
121111
if (value instanceof InetAddress) {
122-
return InetAddressPoint.newExactQuery(name(), (InetAddress) value);
112+
return inetAddressQuery((InetAddress) value, context);
123113
}
124114
String term = BytesRefs.toString(value);
125115
if (term.contains("/")) {
126116
return cidrQuery(term, context);
127117
}
128118
InetAddress address = InetAddresses.forString(term);
129-
return new IpScriptFieldTermQuery(script, leafFactory(context.lookup()), name(), new BytesRef(InetAddressPoint.encode(address)));
119+
return inetAddressQuery(address, context);
120+
}
121+
122+
private Query inetAddressQuery(InetAddress address, QueryShardContext context) {
123+
return new IpScriptFieldTermQuery(script, leafFactory(context), name(), new BytesRef(InetAddressPoint.encode(address)));
130124
}
131125

132126
@Override
@@ -149,7 +143,7 @@ public Query termsQuery(List<?> values, QueryShardContext context) {
149143
}
150144
cidrQueries.add(cidrQuery(term, context));
151145
}
152-
Query termsQuery = new IpScriptFieldTermsQuery(script, leafFactory(context.lookup()), name(), terms);
146+
Query termsQuery = new IpScriptFieldTermsQuery(script, leafFactory(context), name(), terms);
153147
if (cidrQueries == null) {
154148
return termsQuery;
155149
}
@@ -176,6 +170,6 @@ private Query cidrQuery(String term, QueryShardContext context) {
176170
// Force the terms into IPv6
177171
BytesRef lowerBytes = new BytesRef(InetAddressPoint.encode(InetAddressPoint.decode(lower)));
178172
BytesRef upperBytes = new BytesRef(InetAddressPoint.encode(InetAddressPoint.decode(upper)));
179-
return new IpScriptFieldRangeQuery(script, leafFactory(context.lookup()), name(), lowerBytes, upperBytes);
173+
return new IpScriptFieldRangeQuery(script, leafFactory(context), name(), lowerBytes, upperBytes);
180174
}
181175
}

0 commit comments

Comments
 (0)