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

Improve resource handling for empty files contained in jars #28850

Conversation

larsgrefer
Copy link
Contributor

We noticed a strange behavior in WebMVC when working with empty files:

We had some empty files in a classpath location configured in org.springframework.web.servlet.config.annotation.WebMvcConfigurer#addResourceHandlers

When running the application locally using the IDE, the files could be "successfully" accessed by the browser (it got a HTTP 200 response with content-length 0)

After packaging the application, the browser suddenly got 404 responses for the empty files.

The difference seems to be in org.springframework.core.io.AbstractFileResolvingResource#checkReadable.
As long as the empty files are actual files on the file system (ResourceUtils.isFileURL(url) == true), the corresponding ClasspathResource is considered both existent and readable.
After packaging into a jar file, the ClasspathResource is still existent, but no longer considered readable.

I know that serving empty files as static resources is not a typical use case, but I think it should still be (a) possible and (b) consistent, regardless of whether the ClasspathResource in question is an actual File or a JarFileEntry

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jul 21, 2022
@larsgrefer larsgrefer changed the base branch from main to 5.3.x July 21, 2022 19:55
@JanStureNielsen
Copy link
Contributor

JanStureNielsen commented Jul 22, 2022

It seems like checkReadable should be consistently handling empty-content, or not at all, regardless of packaging, and this inconsistency feels like a bug; so, what motivated the comment "empty file or directory -> not considered readable..."?

If empty-content is okay for ResourceUtils, HttpURLConnection, and JarURLConnection branches, why not okay for the rest?

@bclozel bclozel self-assigned this Aug 4, 2022
@bclozel
Copy link
Member

bclozel commented Aug 4, 2022

Indeed, serving an empty resource over HTTP should not result in a 404 response.

The current behavior can be tracked to #21372, where we tried to avoid serving empty files for directory entries. In the course of fixing that problem (in 69f14a2 and 616a40a), it appeared that a long-standing JVM bug prevents us from considering folders and empty resources separately. @JanStureNielsen this should answer your comment.

This behavior has been in place since Spring Framework 5.1 and we've been reluctant to change things more in this area, because of risks of regression or performance issues. In the meantime, this problem has been reported multiple times in Spring Boot and Spring Framework (see #28107 for the latest instance).

I'm inclined to consider this change, which should solve the JAR case and improve consistency with the file system case. Now we're quite late in the 5.3.x cycle, so maybe pushing this to 6.0.0 would make more sense. What do you think @jhoeller @rstoyanchev ?

@bclozel bclozel closed this in 94e5eb3 Sep 26, 2022
@bclozel bclozel added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement and removed for: team-attention status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 26, 2022
@bclozel bclozel added this to the 6.0.0-RC1 milestone Sep 26, 2022
@bclozel bclozel changed the title MVC returns 404 for empty files in static resources in jars Improve resource handling for empty files contained in jars Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants