Skip to content

Commit

Permalink
Cleanup Duplication in PainlessScriptEngine (#31991)
Browse files Browse the repository at this point in the history 
* Cleanup Duplication in `PainlessScriptEngine`
* Extract duplicate building of compiler settings to method
* Remove dead method params + dead constant in `ScriptProcessor`
  • Loading branch information
@original-brownbear
original-brownbear authored Jul 14, 2018
1 parent ccf6126 commit b65c586
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 62 deletions.
3 changes: 0 additions & 3 deletions modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/ScriptProcessor.java
View file
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
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

0 comments on commit b65c586

Please sign in to comment.