Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows, Java launcher: relativize paths in the classpath jar #4408

Closed
laszlocsomor opened this issue Jan 9, 2018 · 8 comments
Closed

Windows, Java launcher: relativize paths in the classpath jar #4408

laszlocsomor opened this issue Jan 9, 2018 · 8 comments
Assignees

Comments

@laszlocsomor
Copy link
Contributor

Description of the problem / feature request:

On Windows, the java_{binary,test} native launcher should create classpath jars with relative paths in the manifest, not with absolute file URLs.

Feature requests: what underlying problem are you trying to solve with this feature?

Motivation: JUnit testing Java annotation processors.

Such tests run javac within a JUnit test, and the JUnit test's runtime classpath (which is a classpath jar file) is javac's compile-time classpath jar.

The JAR file spec states:

Currently, the URLs must be relative to the code base of the JAR file for security reasons.

(Source: https://docs.oracle.com/javase/9/docs/specs/jar/jar.html#class-path-attribute)

The Java launcher currently violates the JAR spec because the launcher writes absolute "file:/" URLs in the classpath jar's manifest.

What operating system are you running Bazel on?

Windows 10.

What's the output of bazel info release?

0.9.0

@laszlocsomor
Copy link
Contributor Author

Huge thanks to @cushon and his colleagues for their help with identifying this problem.

@laszlocsomor
Copy link
Contributor Author

Repro

1. Source tree:

I pretend to have two source trees, c:\tmp\cpjar\tree{1,2}.
This way javac won't be able to find the sources of the missing class in step 4.

$ pwd
/c/tmp/cpjar

$ cat classpath-jar.manifest
Manifest-Version: 1.0
Class-Path: file:/C:/tmp/cpjar/my-class.jar

$ cat tree1/foo/bar/MyClass.java
package foo.bar;

public class MyClass {
  public String hello() {
    return "hello";
  }
}

$ cat tree2/foo/MySrc.java
package foo;

public class MySrc {
  public static void main(String[] args) {
    System.out.println("Message of MyClass: " + new foo.bar.MyClass().hello());
  }
}

2. Compiling MyClass and creating my-class.jar:

$ javac -d tree1 tree1/foo/bar/MyClass.java

$ jar -cf my-class.jar -C tree1 foo/bar/MyClass.class

$ unzip -l my-class.jar
Archive:  my-class.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2018-01-08 17:55   META-INF/
       69  2018-01-08 17:55   META-INF/MANIFEST.MF
      271  2018-01-08 17:55   foo/bar/MyClass.class
---------                     -------
      340                     3 files

3. Creating the classpath jar:

$ cat classpath-jar.manifest
Manifest-Version: 1.0
Class-Path: file:/C:/tmp/cpjar/my-class.jar

$ jar -cfm classpath.jar classpath-jar.manifest

$ unzip -l classpath.jar
Archive:  classpath.jar
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  2018-01-08 17:56   META-INF/
      114  2018-01-08 17:56   META-INF/MANIFEST.MF
---------                     -------
      114                     2 files

$ unzip classpath.jar META-INF/MANIFEST.MF >&/dev/null && cat META-INF/MANIFEST.MF && rm -rf META-INF
Manifest-Version: 1.0
Class-Path: file:/C:/tmp/cpjar/my-class.jar
Created-By: 1.8.0_144 (Azul Systems, Inc.)

4. Compiling against my-class.jar works, but against classpath.jar it doesn't:

C:\tmp\cpjar>javac -d tree2 -cp my-class.jar tree2\foo\MySrc.java

C:\tmp\cpjar>javac -d tree2 -cp classpath.jar tree2\foo\MySrc.java
tree2\foo\MySrc.java:5: error: package foo.bar does not exist
    System.out.println("Message of MyClass: " + new foo.bar.MyClass().hello());
                                                           ^
1 error

5. Running against either of the jars works, and the jars are needed:

C:\tmp\cpjar>java -cp tree2;my-class.jar foo.MySrc
Message of MyClass: hello

C:\tmp\cpjar>java -cp tree2;classpath.jar foo.MySrc
Message of MyClass: hello

C:\tmp\cpjar>java -cp tree2 foo.MySrc
Exception in thread "main" java.lang.NoClassDefFoundError: foo/bar/MyClass
        at foo.MySrc.main(MySrc.java:5)
Caused by: java.lang.ClassNotFoundException: foo.bar.MyClass
        at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
        at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:335)
        at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
        ... 1 more

6. Using relative paths works both with javac and java:

6.a. References to a subdirectory:

C:\tmp>type cpjar\classpath-jar.manifest
Manifest-Version: 1.0
Class-Path: cpjar/my-class.jar

C:\tmp>jar -cfm classpath.jar cpjar\classpath-jar.manifest

C:\tmp>javac -d cpjar\tree2 -cp classpath.jar cpjar\tree2\foo\MySrc.java

C:\tmp>java -cp cpjar/tree2;classpath.jar foo.MySrc
Message of MyClass: hello

6.b. Uplevel references are fine too:

C:\tmp\cpjar\tree3>type ..\classpath-jar.manifest
Manifest-Version: 1.0
Class-Path: ../my-class.jar

C:\tmp\cpjar\tree3>jar -cfm classpath.jar ..\classpath-jar.manifest

C:\tmp\cpjar\tree3>javac -d ..\tree2 -cp classpath.jar ..\tree2\foo\MySrc.java

C:\tmp\cpjar\tree3>java -cp ../tree2;classpath.jar foo.MySrc
Message of MyClass: hello

@meteorcloudy
Copy link
Member

Thank you all for figuring out this, I'm looking into it!

@laszlocsomor
Copy link
Contributor Author

Thank you!
I think all we need to do is change the classpath jar creation so it rewrites classpath entries relative to the classpath jar itself.

@meteorcloudy
Copy link
Member

Will do it!

@meteorcloudy
Copy link
Member

Is there a minimal repo with Bazel that I can test against?

@laszlocsomor
Copy link
Contributor Author

Not yet.

@c-parsons is working on CL 180818052, which adds a new annotation processor and associated tests, which is how we found the problem.

  1. Once that CL is submitted, it will add the new file src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor/SkylarkConfigurationFieldProcessorTest.java.
  2. You'll need to remove the assumeTrue(OS.getCurrent() != OS.WINDOWS); line from testGoldenConfigurationField(). You can comment out the other tests, you don't need those for a minimal repro.
  3. You'll need to bazel test //src/test/java/com/google/devtools/build/lib/analysis/skylark/annotations/processor:SkylarkConfigurationFieldProcessorTest

@meteorcloudy
Copy link
Member

Very helpful, thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants