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

JarUrlConnection.getPermission() can throw NullPointerException if jarFileConnection is null #39856

Closed
wants to merge 1 commit into from

Conversation

yokotaso
Copy link
Contributor

@yokotaso yokotaso commented Mar 8, 2024

Thank you for developing great springframework and spring-boot ecosystem, I appreciate development.

if given resource path is not found and JavaSecurityManager is enabled, NullPointerException is thrown internally.
Not only it is simply meaningless exception,
but also it might be cause of performance regression.

StackTrace is like bellow.

java.lang.NullPointerException: Cannot invoke "java.net.URLConnection.getPermission()" because "this.jarFileConnection" is null
        at org.springframework.boot.loader.net.protocol.jar.JarUrlConnection.getPermission(JarUrlConnection.java:175)
        at java.base/jdk.internal.loader.URLClassPath.check(URLClassPath.java:553)
        at java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:612)
        at java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:296)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:629)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:627)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:626)
        at org.springframework.boot.loader.net.protocol.jar.JarUrlClassLoader.findResource(JarUrlClassLoader.java:70)
        at java.base/java.lang.ClassLoader.getResource(ClassLoader.java:1403)
        at java.base/java.net.URLClassLoader.getResourceAsStream(URLClassLoader.java:290)
        at java.base/java.lang.Class.getResourceAsStream(Class.java:2850)

I got above stacktrace with using debugger around catch statement in jdk.internal.loader.URLClassPath.Loader#findResource and invoking Exception#printStackTrace()

if given resource path is not found and JavaSecurityManager is enabled,
NullPointerException is thrown internaly.
Not only it is simply meaningless exception,
but also it might be cause of performance regression.

StackTrace is like bellow:

java.lang.NullPointerException: Cannot invoke "java.net.URLConnection.getPermission()" because "this.jarFileConnection" is null
        at org.springframework.boot.loader.net.protocol.jar.JarUrlConnection.getPermission(JarUrlConnection.java:175)
        at java.base/jdk.internal.loader.URLClassPath.check(URLClassPath.java:553)
        at java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:612)
        at java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:296)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:629)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:627)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:626)
        at org.springframework.boot.loader.net.protocol.jar.JarUrlClassLoader.findResource(JarUrlClassLoader.java:70)
        at java.base/java.lang.ClassLoader.getResource(ClassLoader.java:1403)
        at java.base/java.net.URLClassLoader.getResourceAsStream(URLClassLoader.java:290)
        at java.base/java.lang.Class.getResourceAsStream(Class.java:2850)
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 8, 2024
@wilkinsona
Copy link
Member

Thanks for the proposal but, following its deprecation in the JDK, we don't support running with the security manager enabled. Support was removed in #28213.

@wilkinsona wilkinsona closed this Mar 8, 2024
@wilkinsona wilkinsona added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 8, 2024
@philwebb philwebb reopened this Apr 19, 2024
@philwebb philwebb added type: bug A general bug and removed status: declined A suggestion or change that we don't feel we should currently apply labels Apr 19, 2024
@philwebb philwebb added this to the 3.2.x milestone Apr 19, 2024
@philwebb
Copy link
Member

Looking again at this one I think we probably should merge it. Although we don't support running with the security manager the getPermission() method hasn't been deprecated and we do protect other call that delegate to this.jarFileConnection to make sure it's not null.

The only thing I'm not sure about is if we can return null. The class loader implementation used to throw an exception. We could do that or return SecurityConstants.ALL_PERMISSION;.

@philwebb philwebb changed the title Fix NullPointerException when JavaSecurityManager enabled JarUrlConnection.getPermission() can throw NullPointerException if jarFileConnection is null Apr 19, 2024
@philwebb
Copy link
Member

Throwing an exception looks like the best option.

@philwebb philwebb added the for: merge-with-amendments Needs some changes when we merge label Apr 19, 2024
@yokotaso
Copy link
Contributor Author

yokotaso commented May 1, 2024

@philwebb Thank you for seeing my suggestion. Sorry for the delay in noticing.

according to javadoc in java.net.URLConnection#getPermission,
it seems ok that java.net.URLConnection#getPermission returns null

Returns a permission object representing the permission
necessary to make the connection represented by this
object. This method returns null if no permission is
required to make the connection.

please comment me if I can be of any help to you.

@philwebb philwebb self-assigned this May 2, 2024
philwebb pushed a commit that referenced this pull request May 2, 2024
Fix regression in `JarUrlConnection` where a NullPointerException could
be thrown internally causing performance issues.

When the SecurityManager is present, the following stack trace is
thrown:

java.lang.NullPointerException: Cannot invoke "java.net.URLConnection.getPermission()" because "this.jarFileConnection" is null
        at org.springframework.boot.loader.net.protocol.jar.JarUrlConnection.getPermission(JarUrlConnection.java:175)
        at java.base/jdk.internal.loader.URLClassPath.check(URLClassPath.java:553)
        at java.base/jdk.internal.loader.URLClassPath$Loader.findResource(URLClassPath.java:612)
        at java.base/jdk.internal.loader.URLClassPath.findResource(URLClassPath.java:296)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:629)
        at java.base/java.net.URLClassLoader$2.run(URLClassLoader.java:627)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:399)
        at java.base/java.net.URLClassLoader.findResource(URLClassLoader.java:626)
        at org.springframework.boot.loader.net.protocol.jar.JarUrlClassLoader.findResource(JarUrlClassLoader.java:70)
        at java.base/java.lang.ClassLoader.getResource(ClassLoader.java:1403)
        at java.base/java.net.URLClassLoader.getResourceAsStream(URLClassLoader.java:290)
        at java.base/java.lang.Class.getResourceAsStream(Class.java:2850)

See gh-39856
@philwebb philwebb closed this in bd16fc4 May 2, 2024
@philwebb philwebb removed the for: merge-with-amendments Needs some changes when we merge label May 2, 2024
@philwebb philwebb modified the milestones: 3.2.x, 3.2.6 May 2, 2024
@philwebb
Copy link
Member

philwebb commented May 2, 2024

Thanks again @yokotaso. This has now been merged.

@yokotaso yokotaso deleted the fix-npe branch May 2, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants