jOOQ / jOOR

jOOR - Fluent Reflection in Java jOOR is a very simple fluent API that gives access to your Java Class structures in a more intuitive way. The JDK's reflection APIs are hard and verbose to use. Other languages have much simpler constructs to access type meta information at runtime. Let us make Java reflection better.
http://www.jooq.org/products
Apache License 2.0
2.8k stars 377 forks source link

Default classpath entries are URL encoded & don't work if there is a space in the path #82

Closed cbajema closed 5 years ago

cbajema commented 5 years ago

My application is a Spring application run on Windows 10 with Tomcat 9 installed in the default directory.

The problem is that in Compile.compile() the creation of the classpath option adds URL encoded paths which fail to load anything (eg. symbol/package not found compile errors).

Added classpath: C:\Program%20Files\Apache%20Softwar%20Foundation\Tomcat%209.0\webapps\ROOT\WEB-INF\classes

Expected classpath: C:\Program Files\Apache Software Foundation\Tomcat 9.0\webapps\ROOT\WEB-INF\classes

I got around the problem by passing the classpath into the compile options argument but I feel it would work better to update this to URLDecode by default.

Eg. Update the Compile.compile to add classpaths with classpath.append(URLDecoder.decode(new File(url.getFile())), StandardCharsets.UTF_8.name());

Thanks

lukaseder commented 5 years ago

Thank you very much for your message. I've tried the following (on Windows):

try (var in = JDBC.class.getResourceAsStream("/org/joor/test xyz.txt")) {
    in.transferTo(System.out);
}

This correctly reads and prints the contents of the test xyz.txt file, loaded from the classpath. So, I'm not sure what doesn't work here and what you're expecting. Can you please provide a test case that illustrates what you were doing and how this fails in your case?

Notice, I'm not sure if we should URL decode any classpath inputs. After all, %20 might be a valid part of a file name on some OS's and users would expect to be able to load those files without any additional URL decoding. Can you point to some specification that says that the JDK can process and decode URL encoded path elements? If the JDK doesn't do it, we won't do it either.

From how I see things, you ran into an error that isn't strictly related to jOOR and you found a workaround outside of jOOR, so we don't need to do anything here.

cbajema commented 5 years ago

TestPrinter -> The class that is referenced in the compiled code:

package com.other;

public class TestPrinter {
    public static void test() {
        System.out.println("It works!");
    }
}

Compiling of a new class at runtime:

package com.test;

import org.joor.Reflect;

public class TestCompiler {
    public static void main(String[] args) {
        String classCode = "package com.test;\n" +
                "\n" +
                "import com.other.TestPrinter;\n" +
                "\n" +
                "public class HelloWorld {\n" +
                "    public static void test() {\n" +
                "        TestPrinter.test();\n" +
                "    }\n" +
                "}\n";

        Reflect.compile("com.test.HelloWorld", classCode);
    }
}

If this project was placed in the folder "c:\compileTest" it would compile correctly.

If this project was placed in the folder "c:\Compile Test" it would fail to compile as it cannot find package com.other or class com.other.TestPrinter

This is due to line 85 of jOOR/jOOR/src/main/java/org/joor/Compile.java adding a URL as a classpath which has spaces replaced with %20.

Maybe this works fine on some OS's, maybe URLDecoding isn't the best solution. But I consider this a bug as it doesn't work out of the box as expected & think it would make it easier for others if this compiled without needing a work around.

lukaseder commented 5 years ago

OK, I see now. This happens only on JDK 8 where URLClassLoader were a thing. In JDK 11, the relevant logic is not executed.

I couldn't manage to get a failing test to reproduce, but I could see the %20 instances appear when debugging. I'm still unconvinced that simply decoding those URLs again is correct, however passing by url.toURI() seems to do the trick. Can you confirm this?

lukaseder commented 5 years ago

This change seems to do the trick: https://github.com/jOOQ/jOOR/commit/dfd6946ffb2dbdf758fe37e43fd5a1bcb793b360