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

Make groovy sandbox method blacklist dynamically additive #9470

Merged
merged 1 commit into from Jan 28, 2015

Conversation

Projects
None yet
3 participants
@dakrone
Copy link
Member

commented Jan 28, 2015

Using the script.groovy.sandbox.method_blacklist_patch setting, the
blacklist can be dynamically added to by specifying a comma-separated
list of methods (for example, "toString,size" would add .toString and
.size to the blacklist).

When the script.groovy.sandbox.method_blacklist_patch setting is
changed, the script cache is cleared to force new scripts to be
recompiled. Additionally the on-disk cache is cleared so that scripts in
the config/scripts directory are re-compiled as well.

This also fixes an issue where script engines were injected more than
once, which can cause multiple instances of the script engine per node.

@kimchy

View changes

src/main/java/org/elasticsearch/script/ScriptService.java Outdated
// Because the GroovyScriptEngineService knows nothing about the
// cache, we need to clear it here if the setting changes
if (settings.get(GroovyScriptEngineService.GROOVY_SCRIPT_BLACKLIST_PATCH) != null) {
ScriptService.this.clearCache();

This comment has been minimized.

Copy link
@kimchy

kimchy Jan 28, 2015

Member

this will cause the cache to be cleared on each settings refresh, even unrelated ones (once the path one is set), I think that the place where we compare the current patch vs. the new one and if there is a diff, we should clear the cache.

This comment has been minimized.

Copy link
@kimchy

kimchy Jan 28, 2015

Member

we also need to make sure we clear the cache after the settings have been updated on the engine itself. Maybe the engine won't registerer as a listener for settings on the service, and this class will be responsible to go over and call the refresh settings, and then call clear cache if needed?

This comment has been minimized.

Copy link
@dakrone

dakrone Jan 28, 2015

Author Member

I will move changing the settings into the ScriptService entirely so that I can enforce the order of operations, good idea!

@kimchy

This comment has been minimized.

Copy link
Member

commented Jan 28, 2015

minor comment, other than that, LGTM

@dakrone dakrone force-pushed the dakrone:groovy-updatable-blacklist branch 2 times, most recently to c610524 Jan 28, 2015

Make groovy sandbox method blacklist dynamically additive
Using the `script.groovy.sandbox.method_blacklist_patch` setting, the
blacklist can be dynamically *added* to by specifying a comma-separated
list of methods (for example, "toString,size" would add .toString and
.size to the blacklist).

When the `script.groovy.sandbox.method_blacklist_patch` setting is
changed, the script cache is cleared to force new scripts to be
recompiled. Additionally the on-disk cache is cleared so that scripts in
the `config/scripts` directory are re-compiled as well.

This also fixes an issue where script engines were injected more than
once, which can cause multiple instances of the script engine per node.

@dakrone dakrone merged commit c610524 into elastic:master Jan 28, 2015

1 check passed

CLA Commit author has signed the CLA
Details

@clintongormley clintongormley changed the title Make groovy sandbox method blacklist dynamically additive Scripting: Make groovy sandbox method blacklist dynamically additive Feb 10, 2015

@dakrone dakrone deleted the dakrone:groovy-updatable-blacklist branch Apr 6, 2015

@clintongormley clintongormley changed the title Scripting: Make groovy sandbox method blacklist dynamically additive Make groovy sandbox method blacklist dynamically additive Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.