-
Notifications
You must be signed in to change notification settings - Fork 93
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
Comments
@glassfishrobot Commented |
@glassfishrobot Commented |
@glassfishrobot Commented |
@stuartwdouglas Commented
|
@marat-gainullin Commented
Paths “” should be treated as context root and ‘/‘ should not. |
|
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
My reading of the above is that the path passed into this method is appended to 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:
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. |
Undertow does not require a leading slash, if it is omitted it basically just does:
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). |
Jetty also will prepend a leading '/' if there is not one.
I would not mind text that encouraged the argument to have a leading slash
as it is just generating garbage to have to create another string. So
let's say something like "the path should start with a '/' and is
interpreted relative to the ServletContext".
…On Thu, 26 Nov 2020 at 23:54, Stuart Douglas ***@***.***> wrote:
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).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#105 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAARJLLIAU2OHCQ7X54XUOLSR3MDNANCNFSM4UDR2PGQ>
.
--
Greg Wilkins <gregw@webtide.com> CTO http://webtide.com
|
Thanks for the feedback. I'll put together a PR that says something along the lines of:
and update Tomcat to be less strict (Tomcat currently requires a "/"). |
…ng path) Signed-off-by: Mark Thomas <markt@apache.org>
…ng path) Signed-off-by: Mark Thomas <markt@apache.org>
Changes due to jakartaee/servlet#105
Changes due to jakartaee/servlet#105
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 "/".
The text was updated successfully, but these errors were encountered: