LaunchedURLClassLoader can return unrelated JAR content

[mederly]

Hello.

We use Spring Boot 1.5.8 as part of midPoint, open source identity management software. Recently we found out that during startup, midPoint is unable to correctly resolve URIs like the following one (note: schema-3.7.jar is one of our components):

jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd

Instead of expected XML content of types-3.xsd, the calling method gets - without any error indication - entirely unrelated binary data! It turned out that the data is the content of this file:

C:\tmp\mp\lib\midpoint.war!/WEB-INF/lib/hibernate-commons-annotations-4.0.5.Final.jar

The schema-3.7.jar and hibernate-commons-annotations-4.0.5.Final.jar files have nothing in common, besides the fact they are both on classpath (hibernate jar first, schema second - this is important).

We were able to create a test case for this condition (inspired by similar tests in LaunchedURLClassLoaderTests):

@Test
public void resolveFromNestedNestedJarAbsolutePath() throws Exception {
    File file = this.temporaryFolder.newFile();
    TestJarCreator.createTestJar(file, false, true);   // creates a structure described below
    JarFile jarFile = new JarFile(file);
    JarFile nestedJarFile = jarFile.getNestedJarFile(jarFile.getEntry("nesting-nested.jar"));
    JarFile nJarFile = jarFile.getNestedJarFile(jarFile.getEntry("n123456789012345678901234567890.jar"));
    LaunchedURLClassLoader loader = new LaunchedURLClassLoader(new URL[] { nJarFile.getUrl(), nestedJarFile.getUrl() }, null);
    String absolutePath = nestedJarFile.getUrl() + "nested.jar!/3.dat";
    URL resource = loader.getResource(absolutePath);
    System.out.println("Looked for: " + absolutePath);
    System.out.println("Found resource: " + resource);
    assertThat(resource.toString()).isEqualTo(absolutePath);
    assertThat(resource.openConnection().getInputStream().read()).isEqualTo(3);
}

The test JAR file is a simulation of our midpoint.war and contains the following entries (among others):

• nesting-nested.jar (this is analogous to schema-3.7.jar in midPoint)
  • nested.jar (this is not present in midPoint)
    • 3.dat (contains a value of 3) (analogous to types-3.xsd)
• n123456789012345678901234567890.jar (analogous to hibernate-commons-annotations-4.0.5.Final.jar)
  • (contains no entries besides manifest)

If the classpath is:

1. jar:file:/C:/Users/.../junit4507521104116618629.tmp!/n123456789012345678901234567890.jar!/
2. jar:file:/C:/Users/.../junit4507521104116618629.tmp!/nesting-nested.jar!/

then the method call:

loader.getResource("jar:file:/C:/Users/.../junit4507521104116618629.tmp!/nesting-nested.jar!/nested.jar!/3.dat")

returns the content of n123456789012345678901234567890.jar instead of 3.dat, without any warning.

Just for the completeness, the sysout is

Looked for: jar:file:/C:/Users/.../junit4507521104116618629.tmp!/nesting-nested.jar!/nested.jar!/3.dat
Found resource: jar:file:/C:/Users/.../junit4507521104116618629.tmp!/nesting-nested.jar!/nested.jar!/3.dat

but then the exception is

org.junit.ComparisonFailure: 
Expected: 3
Actual: 80

(note that 80 is 'P' character, the first one of the n1234...jar file)

The problem is in JarURLConnection.get(URL, JarFile) method in connection with extractFullSpec(String, String) method in the same class. The implementation ignores the fact that the URL to be resolved might point to a JAR file different from the JarFile provided by the caller. In such cases, usually nothing wrong happens; but there is a chance of file name length collision leading to extractFullSpec returning an empty string which is later interpreted in the get method as "we have found a match". I can provide more details if needed.

Please find a couple of test cases (both in LaunchedURLClassLoaderTests and JarURLConnectionTests) and a proposed fix in a forthcoming commit.

---

While reading the code of JarURLConnection.get/extractFullSpec we have found two more bugs, for which we have written separate test cases. The first one is related to resolution of more than one segment in the path returned by extractFullSpec (there should be "=" instead of "+=" on this line). The second one is related to spaces and other non-URI-compliant characters in jar entry names. Again, please see the test cases created and fixes proposed.

---

Could you please have a look at the problems and the proposed fixes? If they are correct, we can submit a PR. If they are not, please could you provide us a hint what should we do to resolve them in a proper way?

Thanks.

[mederly]

Just a note on why in our situation the loader gets an absolute URI to resolve (because maybe this is wrong in the first place - this is something I am really not sure about).

In our case the loader is called by Apache CXF resolver (TransportURIResolver.resolve method). It gets arguments of

• curUri = jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd
• baseUri = jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/xml/ns/public/common/common-3.xsd

I think such parameters are legitimate.

It then tries to resolve URL of

jar:file:/C:/tmp/mp/lib/midpoint.war!jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd

by calling URL(...).openStream. The URL is obviously wrong, because there are two midpoint-war.jar segments in sequence there. After receiving an IOException it asks the classloader to do the following:

getResource("jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd")

and here Spring Boot comes into play.

So actually I am not sure if the parameter that goes to getResource is legitimate, i.e. if the primary problem is in the CXF or Spring Boot. But - for sure - if LaunchedURLClassLoader is unable to resolve the value of

jar:file:/C:/tmp/mp/lib/midpoint.war!/WEB-INF/lib/schema-3.7.jar!/prism/xml/ns/public/types-3.xsd

it should return null, not some unrelated content.

[philwebb]

Possibly related to discussions in #10268 and #11057. I'm afraid this might need to wait until after the holiday break so we can get some of @wilkinsona's input.

[mederly]

@philwebb Thank you. I am looking for your opinions. In the meanwhile I have filed a CXF issue as well.

[wilkinsona]

Thanks for the detailed analysis, @mederly.

I'm not sure that I can see the connection with #10268 and #11057. Those issues are both specific to DevTools which, generally speaking, isn't involved when LaunchedURLClassLoader is involved.

The jar launcher is a pretty tricky and performance sensitive piece of code. For example, we've learned, and been reminded again recently, that anything that creates extra garbage can have a fairly big impact on performance. This means that calls to substring are generally to be avoided if possible.

Before we jump to a solution, I'd like to make sure I fully understand exactly what's happening. To that end, could you please open a PR that adds one or more failing tests that illustrates the problems you've found as minimally as possible?

[mederly]

@wilkinsona Thank you for consideration. I have created the PR with the tests (11497; not sure what to do with the CLA at this moment).

Just a few comments: There are two (out of originally reported three) issues outstanding; one was fixed in the meanwhile. These are:

1. returning wrong JAR content (this is a major one),
2. incorrectly handling non-URI-compliant chars, e.g. spaces, in entry names (this is a minor one).

Tests were added at two levels:

1. lower level: JarURLConnectionTests: connectionToEntryUsingWrongAbsoluteUrlForEntryFromNestedJarFile() is for issue 1, connectionToEntryUsingRelativeUrlForDoublyNestedEntryWithSpace() and connectionToEntryUsingRelativeUrlForDoublyNestedEntryWithTwoSpaces() are for issue 2.
2. upper level: LaunchedURLClassLoaderTests: resolveFromDoublyNestedJarUsingUriAsResourceName...() are for issue 1 (and partially for issue 2), resolveFromDoublyNestedJarHavingSpace() is for issue 2.

Hope it's ok.

[wilkinsona]

Thank you, @mederly. Could you please sign the CLA? We can't do much with the tests in your PR until you have done so.

[mederly]

@wilkinsona I am trying to resolve this with my employer. There are some technical issues to be sorted out. I will sign as soon as it will be possible,

[mederly]

@wilkinsona I am lost. The CLA is signed (I have got a message stating that "Thank you for signing the Agreement! The contributor license agreement checks on /pull/11497 should now pass.") but I was not able to manually synchronize PR #11497 (got "Internal Server Error" from Pivotal page), so I closed it. Nor I am able to create a new PR - #11600 is telling me that "Please sign the Contributor License Agreement!".

I do not want to create too much trash in your system by trying again and again, so, please, have a look at this. :) Tell me to do if there's anything I can do.

[mederly]

@wilkinsona Switching the email address didn't help. The current PR #11601 is again blocked by the CLA check.

[wilkinsona]

Ok. Thanks for trying. Let's wait for @rwinch's CLA expertise.

[rwinch]

@mederly Since this is a corporate signature please refer to the FAQ. Specifically:

If your company associated the signature to a GitHub organization, then you must ensure the following is true.

• Your GitHub account is a member of the selected organization
• You have made your membership public

At the moment, github status is giving me unicorns all over the place which could explain why the sync is not happening.

[mederly]

@rwinch Thank you. But I have met the two requirements before I opened second PR (#11600). See here. Is there anything (more) I could do now - when github status is ok? Should I create fourth PR? Or is there a way how to "synchronize" currently open PR #11601? Thank you in advance.

[rwinch]

@mederly Thanks for the additional information.

I found out that a GitHub permission was changed which was preventing the CLA tooling from updating the status (which is why the error page was happening). The PR is now marked as signed and the issue should be resolved.

Thank you for your patience, using Spring, and of course your contribution back to the community!

[philwebb]

I think this has now been fixed by a combination of #12483 and #12765. @mederly Can you try again with the latest snapshot.

[mederly]

@philwebb Checked on the master just now. Higher-level tests in LaunchedURLClassLoaderTests pass. Lower-level tests in JarURLConnectionTests almost pass: all except connectionToEntryUsingWrongAbsoluteUrlForEntryFromNestedJarFile().

	@Test(expected = FileNotFoundException.class)
	public void connectionToEntryUsingWrongAbsoluteUrlForEntryFromNestedJarFile()
			throws Exception {
		// coincidental match (extractFullSpec would return "")
		URL url = new URL("jar:file:" + getAbsolutePath() + "!/w.jar!/3.dat");
		JarFile nested = this.jarFile
				.getNestedJarFile(this.jarFile.getEntry("nested.jar"));
		JarURLConnection.get(url, nested).getInputStream();
	}

Maybe we could live with that; but I would feel better if it would be fixed as well. :)

---

Anyway, I do have a question: tests on 1.5.x branch do not pass. What is missing is this change:
JarURLConnection.java:257/266 is currently

index += separator + SEPARATOR.length();

but it obviously should be

index = separator + SEPARATOR.length();

(as it is on master - you have changed it on December 19th 2017 there). Without this change, many of my tests on 1.5.x fail.

Could you please implement this change on 1.5.x as well?

Thanks.

[philwebb]

@mederly Thanks for checking. We'll try to fix both those remaining issues in 1.5.x.