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

PathEditor returns wrong result for absolute path in servletContext.getRealPath(...) [SPR-14549] #19117

Closed
spring-projects-issues opened this issue Aug 1, 2016 · 2 comments
Assignees
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 1, 2016

Holger Stenzhorn opened SPR-14549 and commented

In relation to #19007, when I now put the following code in my webapp...

  @Value("#{servletContext.getRealPath('/resources/ontologies')}")
  private void setString(String string) {
    System.out.println(string);
  }

  @Value("#{servletContext.getRealPath('/resources/ontologies')}")
  private void setFile(File file) {
    System.out.println(file);
  }

  @Value("#{servletContext.getRealPath('/resources/ontologies')}")
  private void setPath(Path path) {
    System.out.println(path);
  }

...then this results in the following output...

/Users/holger/Developer/ObTiMA/out/artifacts/ObTiMA_Web_exploded/resources/ontologies
/Users/holger/Developer/ObTiMA/out/artifacts/ObTiMA_Web_exploded/resources/ontologies
/Users/holger/Developer/ObTiMA/out/artifacts/ObTiMA_Web_exploded/Users/holger/Developer/ObTiMA/out/artifacts/ObTiMA_Web_exploded/resources/ontologies

...where the first two lines are indeed correct but the last one is not.

Looking at your implementation of PathEditor, it seems that the code below from my original post needs to be added to make things work correctly:

// Check whether we got an absolute file path without "file:" prefix.
// For backwards compatibility, we'll consider those as straight file path.
if (!ResourceUtils.isUrl(text)) {
  Path path = Paths.get(text);
  if (path.isAbsolute()) {
    setValue(path);
    return;
  }
}

Affects: 4.3.2

Issue Links:

  • #19007 Add PathEditor to enable conversion to java.nio.file.Path
  • #19821 Different handling in PathEditor in Servlet Context for existing and non-existing paths

Referenced from: commits d69afaa, 4ada571

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 8, 2016

Juergen Hoeller commented

PathEditor was meant to avoid some of the legacy steps in FileEditor but I guess I restricted it a bit too far there. Anyway, the absoluteness check doesn't seem to be needed: Instead, it should be sufficient to port another part of FileEditor's algorithm, namely to fall back to plain path resolution in case of resource resolution ending up with a non-existent resource (in your scenario, when trying to apply the given real path relative to the web application root).

That said, I recommend against relying on those fallback steps. Explicitly prefixing with "file:" is usually preferable. For your scenario, you can even simply skip explicit SpEL access to the ServletContext and specify plain a @Value("/resources/ontologies") resource location, letting Spring automatically adapt to ServletContext path resolution in a web application context (this will even work in 4.3.2 already; just make a relative path go in instead of an absolute real path).

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 8, 2016

Holger Stenzhorn commented

Great, thanks a lot Jürgen for the helpful explanation!

Actually, the given code snippet to demonstrate my issue is a bit oversimplified: In our actual webapp the annotation currently look like @Value("${ontologyBasePath:#{servletContext.getRealPath('/resources/ontologies')"} }) where ontologyBasePath is an absolute path independent of the webapp. Thus, only for the default case I need the resolution relative to the webapp.

So, following your solution, I would change the annotation to @Value("${ontologyBasePath:/resources/ontologies}") but then I also have to change the (existing) property files, i.e. add file: in front of the ontologyBasePath property value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants