Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop runtime script from emitting too many values (backport of #61938) #62186

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ public abstract class AbstractLongScriptFieldScript extends AbstractScriptFieldS
private long[] values = new long[1];
private int count;

public AbstractLongScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public AbstractLongScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

/**
Expand Down Expand Up @@ -50,6 +50,7 @@ public final int count() {
}

protected final void emitValue(long v) {
checkMaxSize(count);
if (values.length < count + 1) {
values = ArrayUtil.grow(values, count + 1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.elasticsearch.search.lookup.SourceLookup;

import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;

Expand All @@ -27,6 +28,11 @@
* {@link AggregationScript} but hopefully with less historical baggage.
*/
public abstract class AbstractScriptFieldScript {
/**
* The maximum number of values a script should be allowed to emit.
*/
static final int MAX_VALUES = 100;

public static <F> ScriptContext<F> newContext(String name, Class<F> factoryClass) {
return new ScriptContext<F>(
name + "_script_field",
Expand Down Expand Up @@ -54,10 +60,12 @@ public static <F> ScriptContext<F> newContext(String name, Class<F> factoryClass
value -> ((SourceLookup) value).loadSourceIfNeeded()
);

protected final String fieldName;
private final Map<String, Object> params;
private final LeafSearchLookup leafSearchLookup;

public AbstractScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
public AbstractScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
this.fieldName = fieldName;
this.leafSearchLookup = searchLookup.getLeafSearchLookup(ctx);
params = new HashMap<>(params);
params.put("_source", leafSearchLookup.source());
Expand Down Expand Up @@ -94,5 +102,23 @@ public final Map<String, ScriptDocValues<?>> getDoc() {
return leafSearchLookup.doc();
}

/**
* Check if the we can add another value to the list of values.
* @param currentSize the current size of the list
*/
protected final void checkMaxSize(int currentSize) {
if (currentSize >= MAX_VALUES) {
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
"Runtime field [%s] is emitting [%s] values while the maximum number of values allowed is [%s]",
fieldName,
currentSize + 1,
MAX_VALUES
)
);
}
}

public abstract void execute();
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup);
}

public interface LeafFactory {
Expand All @@ -41,8 +41,8 @@ public interface LeafFactory {
private int trues;
private int falses;

public BooleanScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public BooleanScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup, DateFormatter formatter);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup, DateFormatter formatter);
}

public interface LeafFactory {
Expand All @@ -40,8 +40,14 @@ public interface LeafFactory {

private final DateFormatter formatter;

public DateScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, DateFormatter formatter, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public DateScriptFieldScript(
String fieldName,
Map<String, Object> params,
SearchLookup searchLookup,
DateFormatter formatter,
LeafReaderContext ctx
) {
super(fieldName, params, searchLookup, ctx);
this.formatter = formatter;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup);
}

public interface LeafFactory {
Expand All @@ -41,8 +41,8 @@ public interface LeafFactory {
private double[] values = new double[1];
private int count;

public DoubleScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public DoubleScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

/**
Expand Down Expand Up @@ -72,6 +72,7 @@ public final int count() {
}

protected final void emitValue(double v) {
checkMaxSize(count);
if (values.length < count + 1) {
values = ArrayUtil.grow(values, count + 1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup);
}

public interface LeafFactory {
Expand All @@ -60,8 +60,8 @@ public interface LeafFactory {
private BytesRef[] values = new BytesRef[1];
private int count;

public IpScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public IpScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

/**
Expand Down Expand Up @@ -94,6 +94,7 @@ public final int count() {
}

protected final void emitValue(String v) {
checkMaxSize(count);
if (values.length < count + 1) {
values = ArrayUtil.grow(values, count + 1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup);
}

public interface LeafFactory {
LongScriptFieldScript newInstance(LeafReaderContext ctx) throws IOException;
}

public LongScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public LongScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

public static class EmitValue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;

public abstract class StringScriptFieldScript extends AbstractScriptFieldScript {
/**
* The maximum number of chars a script should be allowed to emit.
*/
public static final long MAX_CHARS = 1024 * 1024;

public static final ScriptContext<Factory> CONTEXT = newContext("string_script_field", Factory.class);

static List<Whitelist> whitelist() {
Expand All @@ -31,17 +37,18 @@ static List<Whitelist> whitelist() {
public static final String[] PARAMETERS = {};

public interface Factory extends ScriptFactory {
LeafFactory newFactory(Map<String, Object> params, SearchLookup searchLookup);
LeafFactory newFactory(String fieldName, Map<String, Object> params, SearchLookup searchLookup);
}

public interface LeafFactory {
StringScriptFieldScript newInstance(LeafReaderContext ctx) throws IOException;
}

private final List<String> results = new ArrayList<>();
private long chars;

public StringScriptFieldScript(Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(params, searchLookup, ctx);
public StringScriptFieldScript(String fieldName, Map<String, Object> params, SearchLookup searchLookup, LeafReaderContext ctx) {
super(fieldName, params, searchLookup, ctx);
}

/**
Expand All @@ -52,12 +59,26 @@ public StringScriptFieldScript(Map<String, Object> params, SearchLookup searchLo
*/
public final List<String> resultsForDoc(int docId) {
results.clear();
chars = 0;
setDocument(docId);
execute();
return results;
}

protected final void emitValue(String v) {
checkMaxSize(results.size());
chars += v.length();
if (chars > MAX_CHARS) {
throw new IllegalArgumentException(
String.format(
Locale.ROOT,
"Runtime field [%s] is emitting [%s] characters while the maximum number of values allowed is [%s]",
fieldName,
chars,
MAX_CHARS
)
);
}
results.add(v);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public ScriptBooleanFieldData.Builder fielddataBuilder(String fullyQualifiedInde
}

private BooleanScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
return scriptFactory.newFactory(script.getParams(), searchLookup);
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public ScriptDateFieldData.Builder fielddataBuilder(String fullyQualifiedIndexNa
}

private DateScriptFieldScript.LeafFactory leafFactory(SearchLookup lookup) {
return scriptFactory.newFactory(script.getParams(), lookup, dateTimeFormatter);
return scriptFactory.newFactory(name(), script.getParams(), lookup, dateTimeFormatter);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public ScriptDoubleFieldData.Builder fielddataBuilder(String fullyQualifiedIndex
}

private DoubleScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
return scriptFactory.newFactory(script.getParams(), searchLookup);
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public ScriptIpFieldData.Builder fielddataBuilder(String fullyQualifiedIndexName
}

private IpScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
return scriptFactory.newFactory(script.getParams(), searchLookup);
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public ScriptStringFieldData.Builder fielddataBuilder(String fullyQualifiedIndex
}

private StringScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
return scriptFactory.newFactory(script.getParams(), searchLookup);
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public ScriptLongFieldData.Builder fielddataBuilder(String fullyQualifiedIndexNa
}

private LongScriptFieldScript.LeafFactory leafFactory(SearchLookup searchLookup) {
return scriptFactory.newFactory(script.getParams(), searchLookup);
return scriptFactory.newFactory(name(), script.getParams(), searchLookup);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,22 @@

package org.elasticsearch.xpack.runtimefields;

import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.RandomIndexWriter;
import org.apache.lucene.store.Directory;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.lookup.SearchLookup;

import java.io.IOException;

import static org.mockito.Mockito.mock;

public class BooleanScriptFieldScriptTests extends ScriptFieldScriptTestCase<BooleanScriptFieldScript.Factory> {
public static final BooleanScriptFieldScript.Factory DUMMY = (params, lookup) -> ctx -> new BooleanScriptFieldScript(
public static final BooleanScriptFieldScript.Factory DUMMY = (fieldName, params, lookup) -> ctx -> new BooleanScriptFieldScript(
fieldName,
params,
lookup,
ctx
Expand All @@ -29,4 +41,27 @@ protected ScriptContext<BooleanScriptFieldScript.Factory> context() {
protected BooleanScriptFieldScript.Factory dummyScript() {
return DUMMY;
}

public void testTooManyValues() throws IOException {
try (Directory directory = newDirectory(); RandomIndexWriter iw = new RandomIndexWriter(random(), directory)) {
iw.addDocument(org.elasticsearch.common.collect.List.of(new StoredField("_source", new BytesRef("{}"))));
try (DirectoryReader reader = iw.getReader()) {
BooleanScriptFieldScript script = new BooleanScriptFieldScript(
"test",
org.elasticsearch.common.collect.Map.of(),
new SearchLookup(mock(MapperService.class), (ft, lookup) -> null, null),
reader.leaves().get(0)
) {
@Override
public void execute() {
for (int i = 0; i <= AbstractScriptFieldScript.MAX_VALUES * 1000; i++) {
emitValue(i % 2 == 0);
}
}
};
// There isn't a limit to the number of values so this won't throw
script.execute();
}
}
}
}
Loading