net.sf.cglib.core.DefaultNamingPolicy
, which is also used by the SpringNamingPolicy
of the Spring Framework checks explicitly for a java
prefix.
A problem that might occur here is that someone tries to build a javabeat.net
Spring example. This would cause a crash during any CGLIB define class due to the prepended dollar sign ($
).
Wouldn't it therefore make sense to check for java.
as a prefix instead of java
?
Comment From: sbrannen
A problem that might occur here is that e.g. someone tries to build a "javabeat.net" spring example.
Wouldn't the package name be net.javabeat
, which starts with net
instead of java
?
This would cause a crash during any cglib define class due to the prepended dollar sign...
Do you have a concrete use case that results in such a crash?
Also, what version of the JDK/JRE are you using?
Comment From: MuellerMP
I mean the example was just meant to be demonstrative... But I actually ran across the issue while trying to run a spring-boot v2.5.6 (spring framework 5.3.12) sample app under orcale jdk 17. Due to the fact that the fallback method of using ClassLoader::defineClass not being available without a "--add-opens java.base/java.lang=ALL-UNNAMED" it just results in a InaccessibleObjectException.
Full version info for my java binary: Java HotSpot(TM) 64-Bit Server VM (build 17+35-LTS-2724, mixed mode, sharing)
It does work under java 11.. but yea i find it kind of odd that cglib implies these restrictions and doesn't make it clear that it does in fact not support these usecases. Oracles naming convention itself does not impose such restrictions: https://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html
Example.jar and sources: https://drive.google.com/file/d/1Y9ejDOixpaPPP9yMUsIftg3xA0bkcNJv/view?usp=sharing
Comment From: sbrannen
Thanks for the feedback.
It sounds like you're saying it's only a result of enforcement of strong encapsulation since JDK 16/17 -- right?
And in that case, there is in fact a workaround as you've mentioned with --add-opens ...
.
As for the reason why CGLIB prepends $
to any fully qualified class name starting with java
instead of java.
or javax.
, perhaps @vlsi can shed some light on that.
Comment From: vlsi
@MuellerMP ,
1) I guess you are right, and it would be better to use java.
and javax.
prefix tests (see https://github.com/cglib/cglib/blame/975e481faf39c91b8ac5b9b3d62822b7c52c5f47/cglib/src/main/java/net/sf/cglib/core/DefaultNamingPolicy.java#L41-L42 )
2) However, I guess, it does not matter much, as $
should be a valid character in the class name even at the very beginning.
Do you have a stack trace and/or reproducer at hand?
Comment From: MuellerMP
@sbrannen i think the fallback method using a reflective call to ClassLoader::defineClass is a separate design issue of the CGLIB... Current state in spring and the problem in my case is the naming issue. It could also confuse users to see a InaccessibleObjectException just because they named there package in an apparently wrong way. So atleast a helpful exception would be nice trat this usecase is in fact unsupported.
@vlsi Hey! Can you access the google drive link I posted? It should contain a jar and sources to the sample that reproduces the exception under java 16+.
Comment From: vlsi
@MuellerMP , I've never worked with maven-based spring boot project, and it would take me ages to start. Could you please share the stack trace related to $
-based class name?
Comment From: MuellerMP
this is the complete log of the program runthrough (including exception in the reflectutils). example-run.txt
Original Exception is caught here: https://github.com/spring-projects/spring-framework/blob/v5.3.12/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java#L566 Fallthrough throw here: https://github.com/spring-projects/spring-framework/blob/v5.3.12/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java#L589
The exception with the naming issue isnt thrown though and occurs here: https://github.com/spring-projects/spring-framework/blob/v5.3.12/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java#L507
because the package name starts with a $ sign so the ContextClass is not in the same package and it results in a IllegalArgumentException. For IllegalArgumentException and LinkageErrors there exists a fallback in the ReflectUtils using the ClassLoader::defineClass via reflection which is no longer allowed due to the strong encapsulation.
the package name is determined in this stacktrace: AbstractClassGenerator::generate(ClassLoaderData data) https://github.com/spring-projects/spring-framework/blob/v5.3.12/spring-core/src/main/java/org/springframework/cglib/core/AbstractClassGenerator.java#L345 AbstractClassGenerator::generateClassName(Predicate nameTestPrediacte) https://github.com/spring-projects/spring-framework/blob/v5.3.12/spring-core/src/main/java/org/springframework/cglib/core/AbstractClassGenerator.java#L177 SpringNamingPolicy via inheritance from DefaultNamingPolicy::getClassName(String prefix, String source, Object key, Predicate names): https://github.com/cglib/cglib/blob/master/cglib/src/main/java/net/sf/cglib/core/DefaultNamingPolicy.java#L42
Comment From: MuellerMP
I have to admit debugging this sort of error handling is non-trivial and using a reassignable temporary variable that is thrown somewhere else instead of using the Throwable(Throwable cause) constructor first and throwing after is very evil... =( 1) You dont know where the exception actually occured 2) You dont know if there occured an exception beforehand because it is simply ignored and overriden by the latter occuring one
IMO calling Throwable::addSurpressed would be a better alternative if you really dont want to throw...
Comment From: sbrannen
Related Issues
-
28138