Don't assume that all scripts come from GroovyScript

[Ecdcaeb]

Don't assume that all scripts come from GroovyScript. This mixin breaks other mods' use of groovy, and scripts not from the groovyscript path do not work as expected.

The following example does not work in a GroovyScript environment.

GroovyClassLoader groovyClassLoader = new GroovyClassLoader(Launch.classLoader);
Class<?> test = groovyClassLoader.parseClass("println('test load class groovy successfully!')", "demo.groovy");

[22:30:18] [CLIENT/ERROR] [placeholdername]: An exception occurred while running scripts. Look at latest.log for a full stacktrace:
    java.lang.IllegalArgumentException: The path 'demo.groovy' does not contain the root path 'F:\other Game\Cleanroom\.minecraft\versions\GroovyScript\groovy'
        at com.cleanroommc.groovyscript.sandbox.FileUtil.relativize(FileUtil.java:25)
        at org.codehaus.groovy.ast.ModuleNode.handler$zzp000$init(ModuleNode.java:1139)
        at org.codehaus.groovy.ast.ModuleNode.<init>(ModuleNode.java:82)
        at org.apache.groovy.parser.antlr4.AstBuilder.<init>(AstBuilder.java:173)
        at org.apache.groovy.parser.antlr4.Antlr4ParserPlugin.buildAST(Antlr4ParserPlugin.java:56)
        at org.codehaus.groovy.control.SourceUnit.buildAST(SourceUnit.java:256)
        at java.util.Iterator.forEachRemaining(Iterator.java:116)
        at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
        at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:580)
        at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:663)
        at groovy.lang.GroovyClassLoader.doParseClass(GroovyClassLoader.java:373)
        at groovy.lang.GroovyClassLoader.lambda$parseClass$2(GroovyClassLoader.java:316)
        at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:314)
        at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:298)
        at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:258)
        at rml.layer.compat.groovyscripts.RMLGroovySandBox.load(RMLGroovySandBox.java:91)
        at com.cleanroommc.groovyscript.sandbox.GroovySandbox.load(GroovySandbox.java)
        at com.cleanroommc.groovyscript.sandbox.GroovySandbox.load(GroovySandbox.java:108)
        at com.cleanroommc.groovyscript.sandbox.GroovyScriptSandbox.run(GroovyScriptSandbox.java:161)
        at com.cleanroommc.groovyscript.GroovyScript.runGroovyScriptsInLoader(GroovyScript.java:188)
        at com.cleanroommc.groovyscript.GroovyScript.initializeGroovyPreInit(GroovyScript.java:174)
        at net.minecraftforge.fml.common.LoadController.handler$zzg000$preInit(LoadController.java:1018)
        at net.minecraftforge.fml.common.LoadController.distributeStateMessage(LoadController.java)
        at net.minecraftforge.fml.common.Loader.preinitializeMods(Loader.java:629)
        at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:252)
        at net.minecraft.client.Minecraft.func_71384_a(Minecraft.java:467)
        at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:378)
        at net.minecraft.client.main.Main.main(SourceFile:123)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
        at net.minecraft.launchwrapper.Launch.main(Launch.java:28)

[brachy84]

WTF

[brachy84]

What are you trying to do here? Are you trying to execute scripts from a script or from an addon? DONT DO THIS! It's dangerous and will lead to issues!

[Ecdcaeb]

What are you trying to do here? Are you trying to execute scripts from a script or from an addon? DONT DO THIS! It's dangerous and will lead to issues!

trying to execute scripts from a script or from an addon

[Ecdcaeb]

I still stick to my point of view. This mixin of GroovyScript destroys the groovy library, and this PR gives other mods the right to use groovy. In addition, it also makes the following if (this.packageNode == null || this.context == null) return; meaningful. Of course, the above is based on "banning heterologous scripts is not a deliberate design." If this is a deliberate design rather than an oversight, please close this pr.

[brachy84]

I will not support direct usage of GroovyClassLoader by any means. If you absolutly need it, i can add a method to GroovyScriptSandbox

[Ecdcaeb]

I will not support direct usage of GroovyClassLoader by any means. If you absolutly need it, i can add a method to GroovyScriptSandbox

I didn't make my point clear at all. I meant to say that other mods written in Java can't use groovy, not in scripts.

The effect of this mixin is global, not just GroovyScript scripts.

[brachy84]

That doesnt change anything. The direct use of GroovyClassLoader can be dangerous. Especially if you barely have any idea what you are doing.

[Ecdcaeb]

Introduction

I want to repeat my thoughts in a short session. GroovyScript should not assume that the user of groovy has and only has GroovyScript.

// status quo:
public static Class<?> compile(GroovyClassLoader loader, String name, String text){
    if(Loader.isLoaded("groovyscript")) return loader.paresClass(text, GroovyScript.getScriptPath() + '/' + name);
    else return loader.paresClass(text, name);
}

This pr can cancel the above method so that GroovyScript does not need to be considered.

misunderstanding

1) This is not executing the script in the script, and this pr does not introduce this operation. 2) This is not designed for GroovyScript, but removes the damage that GroovyScript does to groovy. Adding methods to GroovyScriptSandbox will not help. Mods that use groovy must consider GroovyScript in this case anyway. 3) This PR does not undermine security, because it is not currently secure. The above code can also solve the problem, the only problem is that when using groovy, you must consider GroovyScript.

Result

In the current situation, I completely think this is a design flaw.

To prove it, I might try making a mod (in other uses) that uses Groovy but doesn't depend on GroovyScript.

[brachy84]

Now I understand. I will reconsider this pr

the damage that GroovyScript does to groovy

I just never considered someone else using the groovy lib

[brachy84]

Implemented in #235 because reasons