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

ServletContext.getRealPath(String) needs clarification #105

Closed
glassfishrobot opened this issue Sep 1, 2014 · 10 comments
Closed

ServletContext.getRealPath(String) needs clarification #105

glassfishrobot opened this issue Sep 1, 2014 · 10 comments
Assignees

Comments

@glassfishrobot
Copy link

The Javadoc for getRealPath() uses the term "virtual path" but doesn't define it anywhere. Clearly, the virtual path is relative to the context root but the following are not clear:

1. Must the path start with '/'?
2. If the path must start with '/', what happens if it does not?
3. If the path doesn't have to start with '/' how do you get from the provided path to a path relative to the content root?

My personal view is:
1. Yes.
2. Throw an IllegalArgumentException
3. N/A

One caveat to the above is that we might allow paths of "" which are treated as equivalent to "/".

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
Reported by markt_asf

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
markt_asf said:
Ping. Does anyone else on the EG have a view on this?

@glassfishrobot
Copy link
Author

@glassfishrobot Commented
This issue was imported from java.net JIRA SERVLET_SPEC-105

@glassfishrobot
Copy link
Author

@stuartwdouglas Commented

  1. I don't think we can add a requirement that paths need to start with a '/', as it could break many existing applications.
  2. N/A
  3. You treat /index.html and index.html the same

@glassfishrobot
Copy link
Author

@marat-gainullin Commented

  1. No.
  2. If provided path starts with '/' then it means that an application refers to file system root and path is not relative. If it does not, provided path is treated as relative to context root similarly with Path.resolve()
  3. N/A.

Paths “” should be treated as context root and ‘/‘ should not.

@glassfishrobot
Copy link
Author

@markt-asf
Copy link
Contributor

I'd like to see if we can clear this one up. The Javadoc (which is essentially unchanged all the way back to Servlet 2.4) is

For example, if path is equal to /index.html, this method will return the absolute file path on the server's filesystem to which a request of the form http://<host>:<port>/<contextPath>/index.html would be mapped, where <contextPath> corresponds to the context path of this ServletContext.

My reading of the above is that the path passed into this method is appended to http://<host>:<port>/<contextPath> to identify the resource in question. For that to work, the path has to start with "/". This is consistent with other methods in the API that accept paths relative to the context root such as getResource().

Given the above, I'd like to make the Javadoc explicit that the path must start with "/".

That then raises the question what to do if the path doesn't start with "/". Other methods throw a MalformedURLException. However, the Javadoc for this method states:

This method returns null if the servlet container is unable to translate the given virtual path to a real path.

Given this, I'd like to make it explicit that the method returns null if a path is passed that does not start with "/".

This approach may create backwards compatibility concerns. If that is the case my alternative proposal is that if the method is called with a path that does not start with "/", a "/" is pre-pended to the provided path and then processing continues as above.

@stuartwdouglas
Copy link
Contributor

Undertow does not require a leading slash, if it is omitted it basically just does:

            if(!canonicalPath.startsWith("/")) {
                canonicalPath = "/" + canonicalPath;
            }

From our POV requiring a / would be a backwards compatibility concern. I also don't think the 'starts with a slash' requirement is particularly helpful or user friendly. I would much prefer that we just say that all paths are effectively absolute, so a leading slash is implied (or something like that, TBH I think the current text already implies that).

@gregw
Copy link
Contributor

gregw commented Nov 27, 2020 via email

@markt-asf
Copy link
Contributor

Thanks for the feedback. I'll put together a PR that says something along the lines of:

  • path is relative to the context root
  • path should start with a "/"
  • if path doesn't start with a "/" the container must behave as if the method was called with "/" + path

and update Tomcat to be less strict (Tomcat currently requires a "/").

markt-asf added a commit to markt-asf/servlet-api that referenced this issue Nov 27, 2020
stuartwdouglas pushed a commit to markt-asf/servlet-api that referenced this issue Nov 30, 2020
markt-asf added a commit to markt-asf/jakartaee-tck that referenced this issue Dec 3, 2021
markt-asf added a commit to markt-asf/jakartaee-tck that referenced this issue Jan 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants