Make groovy sandbox method blacklist dynamically additive #9470
Conversation
| // 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(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I will move changing the settings into the ScriptService entirely so that I can enforce the order of operations, good idea!
|
minor comment, other than that, LGTM |
dakrone
force-pushed
the
groovy-updatable-blacklist
branch
2 times, most recently
from
5626394
to
c610524
Compare
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.
clintongormley
added
the
:Core/Infra/Scripting
clintongormley
changed the title
clintongormley
changed the title
Using the
script.groovy.sandbox.method_blacklist_patchsetting, theblacklist 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_patchsetting ischanged, 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/scriptsdirectory 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.