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.

Sign up for GitHub

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

Jump to bottom

Cleanup Duplication in PainlessScriptEngine #31991

Merged
merged 4 commits into from
Jul 14, 2018
Merged

Cleanup Duplication in PainlessScriptEngine #31991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2a95c5
Cleanup Duplication in `PainlessScriptEngine`
original-brownbear Jul 12, 2018
5d20e9e
Merge remote-tracking branch 'elastic/master' into dry-up-painless-sc…
original-brownbear Jul 13, 2018
b80bb97
cleanup separate declaration from CR
original-brownbear Jul 13, 2018
0b4e01c
Merge remote-tracking branch 'elastic/master' into dry-up-painless-sc…
original-brownbear Jul 14, 2018
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
3 changes: 0 additions & 3 deletions modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package org.elasticsearch.ingest.common;

import com.fasterxml.jackson.core.JsonFactory;

import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
Expand Down Expand Up @@ -48,7 +46,6 @@
public final class ScriptProcessor extends AbstractProcessor {

public static final String TYPE = "script";
private static final JsonFactory JSON_FACTORY = new JsonFactory();

private final Script script;
private final ScriptService scriptService;
Expand Down
85 changes: 26 additions & 59 deletions modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,44 +366,7 @@ private void writeNeedsMethods(Class<?> clazz, ClassWriter writer, MainMethodRes
}

Object compile(Compiler compiler, String scriptName, String source, Map<String, String> params, Object... args) {
final CompilerSettings compilerSettings;

if (params.isEmpty()) {
// Use the default settings.
compilerSettings = defaultCompilerSettings;
} else {
// Use custom settings specified by params.
compilerSettings = new CompilerSettings();

// Except regexes enabled - this is a node level setting and can't be changed in the request.
compilerSettings.setRegexesEnabled(defaultCompilerSettings.areRegexesEnabled());

Map<String, String> copy = new HashMap<>(params);

String value = copy.remove(CompilerSettings.MAX_LOOP_COUNTER);
if (value != null) {
compilerSettings.setMaxLoopCounter(Integer.parseInt(value));
}

value = copy.remove(CompilerSettings.PICKY);
if (value != null) {
compilerSettings.setPicky(Boolean.parseBoolean(value));
}

value = copy.remove(CompilerSettings.INITIAL_CALL_SITE_DEPTH);
if (value != null) {
compilerSettings.setInitialCallSiteDepth(Integer.parseInt(value));
}

value = copy.remove(CompilerSettings.REGEX_ENABLED.getKey());
if (value != null) {
throw new IllegalArgumentException("[painless.regex.enabled] can only be set on node startup.");
}

if (!copy.isEmpty()) {
throw new IllegalArgumentException("Unrecognized compile-time parameter(s): " + copy);
}
}
final CompilerSettings compilerSettings = buildCompilerSettings(params);

// Check we ourselves are not being called by unprivileged code.
SpecialPermission.check();
Expand Down Expand Up @@ -434,14 +397,33 @@ public Object run() {
}, COMPILATION_CONTEXT);
// Note that it is safe to catch any of the following errors since Painless is stateless.
} catch (OutOfMemoryError | StackOverflowError | VerifyError | Exception e) {
throw convertToScriptException(scriptName == null ? source : scriptName, source, e);
throw convertToScriptException(source, e);
}
}

void compile(Compiler compiler, Loader loader, MainMethodReserved reserved,
String scriptName, String source, Map<String, String> params) {
final CompilerSettings compilerSettings;
final CompilerSettings compilerSettings = buildCompilerSettings(params);

try {
// Drop all permissions to actually compile the code itself.
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
String name = scriptName == null ? source : scriptName;
compiler.compile(loader, reserved, name, source, compilerSettings);

return null;
}
}, COMPILATION_CONTEXT);
// Note that it is safe to catch any of the following errors since Painless is stateless.
} catch (OutOfMemoryError | StackOverflowError | VerifyError | Exception e) {
throw convertToScriptException(source, e);
}
}

private CompilerSettings buildCompilerSettings(Map<String, String> params) {
CompilerSettings compilerSettings;
if (params.isEmpty()) {
// Use the default settings.
compilerSettings = defaultCompilerSettings;
Expand Down Expand Up @@ -478,25 +460,10 @@ void compile(Compiler compiler, Loader loader, MainMethodReserved reserved,
throw new IllegalArgumentException("Unrecognized compile-time parameter(s): " + copy);
}
}

try {
// Drop all permissions to actually compile the code itself.
AccessController.doPrivileged(new PrivilegedAction<Void>() {
@Override
public Void run() {
String name = scriptName == null ? source : scriptName;
compiler.compile(loader, reserved, name, source, compilerSettings);

return null;
}
}, COMPILATION_CONTEXT);
// Note that it is safe to catch any of the following errors since Painless is stateless.
} catch (OutOfMemoryError | StackOverflowError | VerifyError | Exception e) {
throw convertToScriptException(scriptName == null ? source : scriptName, source, e);
}
return compilerSettings;
}

private ScriptException convertToScriptException(String scriptName, String scriptSource, Throwable t) {
private ScriptException convertToScriptException(String scriptSource, Throwable t) {
// create a script stack: this is just the script portion
List<String> scriptStack = new ArrayList<>();
for (StackTraceElement element : t.getStackTrace()) {
Expand All @@ -507,7 +474,7 @@ private ScriptException convertToScriptException(String scriptName, String scrip
scriptStack.add("<<< unknown portion of script >>>");
} else {
offset--; // offset is 1 based, line numbers must be!
int startOffset = getPreviousStatement(scriptSource, offset);
int startOffset = getPreviousStatement(offset);
int endOffset = getNextStatement(scriptSource, offset);
StringBuilder snippet = new StringBuilder();
if (startOffset > 0) {
Expand Down Expand Up @@ -535,7 +502,7 @@ private ScriptException convertToScriptException(String scriptName, String scrip
}

// very simple heuristic: +/- 25 chars. can be improved later.
private int getPreviousStatement(String scriptSource, int offset) {
private int getPreviousStatement(int offset) {
return Math.max(0, offset - 25);
}

Expand Down