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

Un-deprecate PathResource (for java.nio.file.Path resolution in createRelative) #24211

Closed
userquin opened this issue Dec 14, 2019 · 4 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Milestone

Comments

@userquin
Copy link

When creating a FileSystemResource with a Path containing .. in the path, then createRelative resolves to a wrong path.

The problem resides on StringUtils::applyRelativePath method: I suggest change the method on FileSystemResource checking first if the location is a Path and then use Path::resolve instead calling StringUtils::applyRelativePath.

println(FileSystemResource(Paths.get("../xxxx/dist/")).createRelative("index.html"))

print ../index.html instead ../xxx/dist/index.html.

Attached image shows the variables inside FileSystemResource::createRelative method.

bug

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 14, 2019
@userquin
Copy link
Author

I'm using spring.resource.static-locations on a webflux project and works with default ones (classpath resources), but externalizing resources doesn't work due to the reported error when using .. in the path. It work when using paths without ...

@jhoeller jhoeller self-assigned this Dec 16, 2019
@jhoeller
Copy link
Contributor

jhoeller commented Dec 16, 2019

As far as I can reproduce this, it is unfortunately by design: Due to the handling of trailing slashes in the java.io.File and java.nio.file.Path APIs, the cleaned paths always point to the directory itself, not to a base path for subdirectories. Resource.createRelative therefore applies at the same level as the directory, consistent for java.io.File and java.nio.file.Path sources.

Quoting the FileSystemResource(File) javadoc: When building relative resources via createRelative, the relative path will apply at the same directory level: e.g. new File("C:/dir1"), relative path "dir2" -> "C:/dir2"! If you prefer to have relative paths built underneath the given root directory, use the FileSystemResource(String) constructor with a file path to append a trailing slash to the root path: "C:/dir1/", which indicates this directory as root for all relative paths.

I suppose we should add the same comment to the FileSystemResource(Path) constructor. As for usage scenarios in Boot, is that particular constructor being used? This could be replaced by FileSystemResource(String) usage where the trailing slash is going to be an actual differentiator for a base path.

@jhoeller
Copy link
Contributor

jhoeller commented Dec 16, 2019

It seems that our now deprecated PathResource implements the behavior that you're suggesting. Maybe we should un-deprecate it for pure java.nio.file.Path handling, independent from FileSystemResource semantics, if such behavior is still desirable for some scenarios. Since it only got deprecated in 5.1.1, we could still easily un-deprecate it in 5.1.13+.

@rstoyanchev rstoyanchev added the in: core Issues in core modules (aop, beans, core, context, expression) label Dec 16, 2019
@userquin
Copy link
Author

I think it will be necessary to do it (un-deprecate), I just create my own to handle only Path resources: why to handle the same way if java.nio resolves almost all problems to resolve path problems, including resolving .. uri path requests?.

@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 16, 2019
@jhoeller jhoeller added this to the 5.2.3 milestone Dec 16, 2019
@jhoeller jhoeller changed the title Spring core 5.2.2: FileSystemResource::createRelative is wrong Un-deprecate PathResource (for java.nio.file.Path resolution in createRelative) Dec 16, 2019
@jhoeller jhoeller added the for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x label Dec 16, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Dec 16, 2019
jhoeller added a commit that referenced this issue Dec 16, 2019
Includes aligned createRelative signature and dedicated java.io.File test.

Closes gh-24211
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) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants