groovy

Allow overriding the default Groovy class loader

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.6.7
  • Fix Version/s: 1.6.8, 1.7.1, 1.8-beta-1
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    2

Description

I am using Groovy with JSR-223 and I need to override the default Groovy class loader so I can provide a custom CompilationUnit with an additional PrimaryClassNodeOperation in the SEMANTIC_ANALYSIS phase.

I don't know too much about Groovy internals, but as GroovyScriptEngineImpl's loader field is private and never gets reassigned, the only way I found for making it work is using reflection:

private void overrideDefaultGroovyClassLoader(final ScriptEngine engine) throws Exception {
  final Field classLoader = engine.getClass().getDeclaredField("loader");
  classLoader.setAccessible(true);
  classLoader.set(engine, new CustomGroovyClassLoader());
}

If there is not a better way for doing this today, maybe GroovyScriptEngineImpl should provide a setter or a post construct-like callback method for allowing overriding such field.

  1. Java Source File
    GroovyScriptEngineImpl.java
    18/Dec/09 8:28 AM
    18 kB
    Tiago Fernandez
  2. Java Source File
    JSR223SecurityTest.java
    18/Dec/09 8:28 AM
    5 kB
    Tiago Fernandez

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Guillaume Laforge added a comment -

We can certainly open up the implementation a bit, if needs be.

But you're aware of the fact it'd be a specific feature of the Groovy JSR-223 engine, right?
The purpose of JSR-223 being to be non-specific to any language, but to provide a common way of invoking scripts, etc.
(hence why I often advise to use Groovy's integration mechanisms if anyone wants to do something a bit more advanced that only Groovy can provide)

Show
Guillaume Laforge added a comment - We can certainly open up the implementation a bit, if needs be. But you're aware of the fact it'd be a specific feature of the Groovy JSR-223 engine, right? The purpose of JSR-223 being to be non-specific to any language, but to provide a common way of invoking scripts, etc. (hence why I often advise to use Groovy's integration mechanisms if anyone wants to do something a bit more advanced that only Groovy can provide)
Hide
Permalink
Guillaume Laforge added a comment -

(adding code tags to the description)

Show
Guillaume Laforge added a comment - (adding code tags to the description)
Hide
Permalink
Tiago Fernandez added a comment -

Yes, I am aware this feature would be Groovy's specific. I am using the javax.script API in general and I need this only for enhancing security inside scripts, e.g. implement a blacklist of forbidden APIs. Since as far as I know the JSR-223 does not provide a generic mechanism for security and I have to support Rhino, Groovy, JRuby and Jython, I need to get specific when it comes to security at least.

The solution for Rhino for example is to implement a org.mozilla.javascript.ClassShutter and extend from org.mozilla.javascript.ContextFactory for making my blacklist visible, and the way I have found with Groovy would be do deal with the AST. What Groovy's integration mechanisms you are referring?

Show
Tiago Fernandez added a comment - Yes, I am aware this feature would be Groovy's specific. I am using the javax.script API in general and I need this only for enhancing security inside scripts, e.g. implement a blacklist of forbidden APIs. Since as far as I know the JSR-223 does not provide a generic mechanism for security and I have to support Rhino, Groovy, JRuby and Jython, I need to get specific when it comes to security at least. The solution for Rhino for example is to implement a org.mozilla.javascript.ClassShutter and extend from org.mozilla.javascript.ContextFactory for making my blacklist visible, and the way I have found with Groovy would be do deal with the AST. What Groovy's integration mechanisms you are referring?
Hide
Permalink
Guillaume Laforge added a comment -

Indeed, JSR-223 is too limited for that kind of goals (properly securing an application). I once discussed with the JSR-223 team to see if we could have some additional configuration options (like even just passing a mere map of configuration properties) to further configure the engine, but alas, with no success.

Anyhow, yes, the use case mandates some language / API specific integration.

The integration mechanisms I was referring to were just using Groovy's own GroovyShell, GroovyClassLoader, GroovyScriptEngine, etc. to potentially fully leverage other aspects those native integrations provide.

Now, more on the security aspects... if all you need is to forbid access to certain classes, etc. Wouldn't using a Security Manager be a cross-language solution? Would that be possible with the other engines? Would that be application to your use case?

Show
Guillaume Laforge added a comment - Indeed, JSR-223 is too limited for that kind of goals (properly securing an application). I once discussed with the JSR-223 team to see if we could have some additional configuration options (like even just passing a mere map of configuration properties) to further configure the engine, but alas, with no success. Anyhow, yes, the use case mandates some language / API specific integration. The integration mechanisms I was referring to were just using Groovy's own GroovyShell, GroovyClassLoader, GroovyScriptEngine, etc. to potentially fully leverage other aspects those native integrations provide. Now, more on the security aspects... if all you need is to forbid access to certain classes, etc. Wouldn't using a Security Manager be a cross-language solution? Would that be possible with the other engines? Would that be application to your use case?
Hide
Permalink
Tiago Fernandez added a comment -

The Security Manager option was actually the first one investigated by my team, and it was dropped because it would impact performance (around extra 10-15% in WebLogic 10). I am not saying this analysis can be systematically applied to all possible cases and application servers, so I think the decision of using a Security Manager or not should be up to JSR-223's users.

Hence, I believe it stills valid to provide a specific mechanism for making this easier in Groovy, like in Rhino.

Show
Tiago Fernandez added a comment - The Security Manager option was actually the first one investigated by my team, and it was dropped because it would impact performance (around extra 10-15% in WebLogic 10). I am not saying this analysis can be systematically applied to all possible cases and application servers, so I think the decision of using a Security Manager or not should be up to JSR-223's users. Hence, I believe it stills valid to provide a specific mechanism for making this easier in Groovy, like in Rhino.
Hide
Permalink
Guillaume Laforge added a comment -

You can provide a little patch for opening up the implementation (ie. providing getter/setter for the loader), if you wish.
Ideally, even a sample test case showing how you're doing this class shuttering may be interesting, and would cover that part of the code and show people how they can realize that as well.

Show
Guillaume Laforge added a comment - You can provide a little patch for opening up the implementation (ie. providing getter/setter for the loader), if you wish. Ideally, even a sample test case showing how you're doing this class shuttering may be interesting, and would cover that part of the code and show people how they can realize that as well.
Hide
Permalink
Tiago Fernandez added a comment -

Thanks for your support Guillaume. I am attaching the JUnit test case using both reflection and injection for overriding the default class loader. The patch is pretty simple, I have just added getter/setter for the GroovyClassLoader member variable. Cheers.

Show
Tiago Fernandez added a comment - Thanks for your support Guillaume. I am attaching the JUnit test case using both reflection and injection for overriding the default class loader. The patch is pretty simple, I have just added getter/setter for the GroovyClassLoader member variable. Cheers.
Hide
Permalink
Guillaume Laforge added a comment -

A proper "patch" file would be better the next time
Do you mind if instead I provide a getter only, but an additional constructor that would let you specify the classloader?
The reasoning behind that would be to avoid people from change the classloader in the middle of the usage of the engine, that could lead to weird behaviour.
So if one sets the load at construction time, you've got the same ability to customize it, but at least, we prefernt any weird behaviour in case someone inadvertantly change the classloader at a later stage.
What do you think?

Show
Guillaume Laforge added a comment - A proper "patch" file would be better the next time Do you mind if instead I provide a getter only, but an additional constructor that would let you specify the classloader? The reasoning behind that would be to avoid people from change the classloader in the middle of the usage of the engine, that could lead to weird behaviour. So if one sets the load at construction time, you've got the same ability to customize it, but at least, we prefernt any weird behaviour in case someone inadvertantly change the classloader at a later stage. What do you think?
Hide
Permalink
Tiago Fernandez added a comment - - edited

Good point, passing the class loader in the engine's construction would avoid the weird behavior you mentioned. The only thing is, by the time I get the concrete implementation from the ScriptEngineManager the GroovyScriptEngineImpl class is already instantiated. In this case what would be the solution, how would I tell to the GroovyScriptEngineFactory it should instantiate the GroovyScriptEngineImpl with my custom GroovyClassLoader?

PS: Sorry about the proper patch, I never submitted anything to open-source projects rather than logging bugs. Let me know what you expect next time (tiago182 at gmail dot com).

Show
Tiago Fernandez added a comment - - edited Good point, passing the class loader in the engine's construction would avoid the weird behavior you mentioned. The only thing is, by the time I get the concrete implementation from the ScriptEngineManager the GroovyScriptEngineImpl class is already instantiated. In this case what would be the solution, how would I tell to the GroovyScriptEngineFactory it should instantiate the GroovyScriptEngineImpl with my custom GroovyClassLoader? PS: Sorry about the proper patch, I never submitted anything to open-source projects rather than logging bugs. Let me know what you expect next time (tiago182 at gmail dot com).
Hide
Permalink
Guillaume Laforge added a comment -

Ah true, I had forgotten that that's the GSEFactory does instanciate the engine, bad luck.
On the other hand, as you're doing already something specific with the engine, you could decide to bypass the factory
But well, we'll go with the getter/setter, even if it's not ideal.
Anyway, someone using those methods means that he's using the concrete implementation directly (ie. through a cast), and that means he should be aware of the risks!
So I'll go with getter/setter.

As for contributing code, you can use diff / patch to provide patches, or also IDEs these days provide these facilities.
The change being trivial (getter/setter) + a new test case (that I'll have to adapt since we're not using JUnit annotations) are not complicated to apply anyway, so it's not critical.
I think we've got some guidelines on the wiki, I'll have to see if they properly explain how users can contribute.

Thanks again for your help on this!

Show
Guillaume Laforge added a comment - Ah true, I had forgotten that that's the GSEFactory does instanciate the engine, bad luck. On the other hand, as you're doing already something specific with the engine, you could decide to bypass the factory But well, we'll go with the getter/setter, even if it's not ideal. Anyway, someone using those methods means that he's using the concrete implementation directly (ie. through a cast), and that means he should be aware of the risks! So I'll go with getter/setter. As for contributing code, you can use diff / patch to provide patches, or also IDEs these days provide these facilities. The change being trivial (getter/setter) + a new test case (that I'll have to adapt since we're not using JUnit annotations) are not complicated to apply anyway, so it's not critical. I think we've got some guidelines on the wiki, I'll have to see if they properly explain how users can contribute. Thanks again for your help on this!

People

  • Assignee:
    Guillaume Laforge
    Reporter:
    Tiago Fernandez
Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: