Let JarFileLocation work with custom ClassLoader URIs
#1131
Conversation
odrotbohm
force-pushed
the
fix-jar-class-lookup
branch
from
2bee9d1
to
2a8d3f7
Compare
|
@codecholeric @hankem – This was originally reported against Spring Modulith in spring-projects/spring-modulith#221. The fix would be crucial to get our runtime support working. I'd appreciate any estimations on a release including that fix (potentially rather a 1.0.2 than a 1.1?) as we find ourselves blocked from releasing Spring Modulith 1.0 RC1. |
odrotbohm
changed the title
odrotbohm
changed the title
JarFileLocation work with custom ClassLoader URIs
It includes the fix submitted for TNG/ArchUnit#1131. To be removed once we can upgrade to a released version of the fix.
codecholeric
force-pushed
the
fix-jar-class-lookup
branch
from
2a8d3f7
to
34009ca
Compare
|
Hey @odrotbohm, again, sorry that it took me so long right now 🙈 My problem was that I couldn't really overview the whole impact, because I never anticipated multiple separators Anyway, there is one case that currently doesn't work though (I've documented this in a test). If the directory entries within the JAR file are missing, then the respective classes within a certain package won't be found correctly. I had this problem in the past (e.g. the old Anyway, I tried to make some improvements to the test infrastructure on the way and then added some more tests, I'd be happy if you can take a look if this makes sense to you. The last commit I would squash onto your's if that's alright, since it adds extra tests for the concrete fix. In any case, I think it would make sense to add to your commit message also a note, that using as a root JAR URL (where originally only a real root URL of a JAR file was supported, e.g. |
|
PS: Aaaand I see that my tests of course fail on some platforms 🤦♂️ URL handling is so much fun to do cross-platform and across all JDK versions 😬 Gonna look into that tomorrow! I think you can still review it if it generally makes sense from your point of view... |
codecholeric
force-pushed
the
fix-jar-class-lookup
branch
2 times, most recently
from
730066b
to
912f1f1
Compare
|
I think by looking at the test fails I actually found a way to fix the "classes in packages can't be found if there are no directory zip entries" problem 🙂 Because, there was a "legacy way" that actually looked through the context for |
codecholeric
force-pushed
the
fix-jar-class-lookup
branch
from
912f1f1
to
d7714c4
Compare
|
I know too little about the way Boot sets up the class loading in the fat JARs they create. I've asked @wilkinsona to comment, but am not sure when he's going to find time. Personally, I don't have any objectives except getting ArchUnit to work with classes in a running Spring Boot application. Given that this particular context should be a rather rare case for now (and only affects projects using Spring Modulith's runtime support), I agree that the focus should be on making sure that the currently already working scenarios still continue to work. In other words: if we do not catch all corner cases of the “run ArchUnit within a Boot app” is just fine. |
|
The changes here look good to me. I can't see anything that's Spring Boot specific in the main code and that's the way it should be. Regarding jars without directory entries, that can happen with Spring Boot. It shouldn't happen with the "outer" jar as it's created by our build plugins and they will create directory entries. It may happen with jars nested in The code in Spring Framework's |
|
Thanks for the feedback, then I'm gonna get this ready and create a bugfix release! |
According to the ZIP spec (https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT -> 4.4.17.1) file entries in a ZIP file must not start with a leading slash '/'. For tests that purely want to iterate the entries, this problem did not manifest in any way (there is no exception, etc., if an entry starts with '/'). But if such an illegal JAR is used within an `URLClassLoader` it doesn't work correctly and the classes can't be discovered. Signed-off-by: Peter Gafert <[email protected]>
One way how we tackle scanning class files from the classpath is asking the context `ClassLoader` for `getResources(..)`. We now add an explicit test for this. We also document by a test that `getResources(..)` doesn't do what we want if the directory entries are missing from a JAR and we use some `ClassLoader` derived from `URLClassLoader`. When creating JARs we can choose if we want to add ZIP entries for the folders as well or skip them and only add entries for the actual class files. But in case we're not adding those directory entries, any `URLClassLoader.getResources(..)` will return an empty result when asked for this directory. This unfortunately makes the behavior quite inconsistent. We have some mitigation in place to also analyze the classpath and scan through the JARs on the classpath with a prefix logic that ignores if the entries for the directory are present. But in case we really only have a customized `ClassLoader` without any directory entries in a JAR there is not much the `ClassLoader` API allows to do. Signed-off-by: Peter Gafert <[email protected]>
It is possible to quite heavily customize URL handling by extension points like the system property `java.protocol.handler.pkgs`. Thus, it's safer to try to limit manual URI manipulations and file creations as much as possible and derive it from objects that participate in this customization. E.g. use the (possibly customized) `JarURLConnection` obtained from the URL to retrieve the `JarFile` instead of creating a new `File` from the URL and converting this to a `JarFile` again. Signed-off-by: Peter Gafert <[email protected]>
Back when JDK 9 support was implemented I assumed that the `URLClassLoader` would lose some of its omnipresent occurrence and thus dropped retrieving URLs from all `URLClassLoader`s in context (I also assumed that the content of the system property `java.class.path` would coincide with those URLs anyway in all cases where ArchUnit is used). However, for once `URLClassLoader` is still widely used today and secondly retrieving the URLs from such `ClassLoader`s makes a real difference as soon as they are customized like Spring Boot's `ClassLoader`. Because in those cases the URLs from the classpath system property can differ quite a bit from the ones returned by the custom `URLClassLoader`. In Spring Boot's case it can e.g. return nested archive URLs from fat JAR URLs, which are then also resolved correctly when opening the stream by custom URL stream handling. In any case, it makes sense to restore the legacy behavior for JDK < 9 also for all JDKs >= 9 and take the URLs from `URLClassLoader`s into account, so we can support such customized cases better. Signed-off-by: Peter Gafert <[email protected]>
We use different approaches in different places to obtain base, path or both from a JAR URI (i.e. the part up to the separator denoting where the JAR resides versus the part after the separator denoting the path within the JAR file). We now unify this to make the connection clearer in code and use simple character splitting instead of Regex. Signed-off-by: Peter Gafert <[email protected]>
Some ClassLoaders that work with repackaged JAR files return custom resource URIs to indicate custom class loading locations. For example, the ClassLoader in a packaged Spring Boot's returns the following URI for source package named example: jar:file:/Path/to/my.jar!/BOOT-INF/classes!/example/. Note the second "!/" to indicate a classpath root. Prior to this commit, JarFileLocation was splitting paths to a resource at the first "!/" assuming the remainder of the string would depict the actual resource path. That remainder potentially containing a further "!/" would prevent the JAR entry matching in FromJar.classFilesBeneath(…) as the entries themselves do not contain the exclamation mark. This commit changes the treatment of the URI in JarFileLocation to rather use the *last* "!/" as splitting point so that the remainder is a proper path within the ClassLoader and the matching in FromJar.classFilesBeneath(…) works properly. Note that in composition with custom `ClassLoader`s frameworks like Spring Boot can also install custom URL handling. In this case ArchUnit can read a class file from a nested archive URL like `jar:file:/some/file.jar!/BOOT-INF/classes!/...` using the standard `URL#openStream()` method in a completely transparent way (compare setup in `SpringLocationsTest`). Signed-off-by: Oliver Drotbohm <[email protected]>
codecholeric
force-pushed
the
fix-jar-class-lookup
branch
from
d7714c4
to
69e3afc
Compare
|
Okay, as a last test I've now added a JAR without directory entries to the |
|
Just FYI: I decided to just release 1.1.0 with this fix. Because some stuff has accumulated meanwhile and I don't think it makes sense to separate this for a bug fix release (the other changes are either also bug fixes or "low danger" improvements) |
|
Confirming 1.1.0 solves the original problem! Thanks, Pete! 🥳🙇 |
IMPORTANT: I did not add any new test cases to verify the scenario described in the following, as I was unable to get the project imported into my IDE. The existing tests still succeed so that I feel confident I didn't break anything. Furthermore, a downstream sample project producing the described scenario works with the fix applied and previously failed.
Some ClassLoaders that work with repackaged JAR files return custom resource URIs to indicate custom class loading locations. For example, the ClassLoader in a packaged Spring Boot's returns the following URI for source package named example: jar:file:/Path/to/my.jar!/BOOT-INF/classes!/example/. Note the second "!/" to indicate a classpath root.
Prior to this commit, JarFileLocation was splitting paths to a resource at the first "!/" assuming the remainder of the string would depict the actual resource path. That remainder potentially containing a further "!/" would prevent the JAR entry matching in FromJar.classFilesBeneath(…) as the entries themselves do not contain the exclamation mark.
This commit changes the treatment of the URI in JarFileLocation to rather use the last "!/" as splitting point so that the remainder is a proper path within the ClassLoader and the matching in FromJar.classFilesBeneath(…) works properly.