Groovy
  1. Groovy
  2. GROOVY-3946

Allow overriding the default Groovy class loader

    Details

    • Type: Improvement Improvement
    • Status: 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

      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. JSR223SecurityTest.java
        5 kB
        Tiago Fernandez
      2. GroovyScriptEngineImpl.java
        18 kB
        Tiago Fernandez

        Activity

        Hide
        Guillaume Delcroix 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 Delcroix 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
        Guillaume Delcroix added a comment -

        (adding code tags to the description)

        Show
        Guillaume Delcroix added a comment - (adding code tags to the description)
        Hide
        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
        Guillaume Delcroix 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 Delcroix 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
        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
        Guillaume Delcroix 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 Delcroix 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
        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
        Guillaume Delcroix 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 Delcroix 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
        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
        Guillaume Delcroix 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 Delcroix 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 Delcroix
            Reporter:
            Tiago Fernandez
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development