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

URLResource.isDirectory() throws a NullPointerException when created from a jar:file: URL #9984

Closed
wilkinsona opened this issue Jun 28, 2023 · 6 comments · Fixed by #9985
Closed
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@wilkinsona
Copy link
Contributor

Jetty version(s)
12.0.0.Beta2

Java version/vendor (use: java -version)

openjdk version "17.0.7" 2023-04-18 LTS
OpenJDK Runtime Environment (build 17.0.7+7-LTS)
OpenJDK 64-Bit Server VM (build 17.0.7+7-LTS, mixed mode, sharing)

OS type/version
macOS 13.4

Description
URI.getPath() can return null but URLResourceFactory.URLResource.isDirectory() assumes that it will not. This can result in a NullPointerException.

How to reproduce?

new URLResourceFactory().newResource(new URL("jar:file:/example.jar!/")).isDirectory();
@wilkinsona wilkinsona added the Bug For general bugs on Jetty side label Jun 28, 2023
@lorban lorban self-assigned this Jun 28, 2023
@lorban lorban moved this to 🏗 In progress in Jetty 12.0.0.beta3 Jun 28, 2023
lorban added a commit that referenced this issue Jun 28, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

You're right that this NullPointerException should not happen, so I've created a PR to fix this: #9985

But this looks like you're abusing the URLResourceFactory as it wasn't designed to handle file nor jar:file: URIs. Its only intended purpose is to be able to register a ResourceFactory handling HTTP or HTTPS schemes like so:

ResourceFactory.registerResourceFactory("https", new URLResourceFactory());

Using it for any other purpose sounds like abuse and makes me wonder what you're trying to achieve that cannot be done with the official ResourceFactory mechanism.

@lorban lorban moved this from 🏗 In progress to 👀 In review in Jetty 12.0.0.beta3 Jun 28, 2023
lorban added a commit that referenced this issue Jun 28, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@joakime
Copy link
Contributor

joakime commented Jun 28, 2023

@lorban see issue #9973 for origin of this use

@wilkinsona
Copy link
Contributor Author

Thanks, both. Just to provide a summary of the background, as a (possibly intermediate) step I am trying to get things working with Jetty 12 as they did with Resource.newResource(String) in Jetty 11. Until we have FileSystem support in Spring Boot's fat jars, the goal is to use a URL with our custom Handler for jar:file: URLs as Jetty 11 allowed.

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

I don't think URLResourceFactory is advanced enough to usefully handle jar:file URLs, and making it this capable sounds overkill as it would basically require duplicating MountedPathResource and its integration with FileSystemPool.

What kind of magic is your custom URLStreamHandler doing? Maybe that could be achieved another way?

lorban added a commit that referenced this issue Jun 28, 2023
Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@wilkinsona
Copy link
Contributor Author

What kind of magic is your custom URLStreamHandler doing? Maybe that could be achieved another way?

It's a cornerstone of Spring Boot's executable jar support. I don't think it can be achieved in another way as a custom Handler is essential for compatibility with existing code that uses jar: URLs.

@lorban
Copy link
Contributor

lorban commented Jun 28, 2023

I'd like to understand better Spring Boot's executable jar support to try to figure out an elegant solution.

Barring that, URLResourceFactory will have fairly limited support: it is slow, isDirectory makes a guess, and there is no way to list sub-resources. Hopefully, if you have to resort to using that ResourceFactory, it's enough for your needs.

lorban added a commit that referenced this issue Jun 28, 2023
…9985)

* #9984 fix URLResourceFactory isDirectory and newReadableByteChannel

Signed-off-by: Ludovic Orban <lorban@bitronix.be>
@lorban lorban moved this from 👀 In review to ✅ Done in Jetty 12.0.0.beta3 Jun 28, 2023
@lorban lorban closed this as completed Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants