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

HttpServletResponse.encodeURL not working for URLs starting with ../ #7615

Closed
anca-jalbum opened this issue Feb 17, 2022 · 5 comments · Fixed by #7763 or #7765
Closed

HttpServletResponse.encodeURL not working for URLs starting with ../ #7615

anca-jalbum opened this issue Feb 17, 2022 · 5 comments · Fixed by #7763 or #7765
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@anca-jalbum
Copy link

Jetty version(s)
9.4.37 - 9.4.45

Java version/vendor
Java 1.8.0

OS type/version
Mac

Description
When doing HttpServletResponse.encodeURL("../foo/bar.jsp") when client has no session cookie so the URL should be rewritten as "../foo/bar.jsp;jsessionid=[sessionID]" the HttpURI.parse method responds with "Bad URI".

That is not a "Bad URI". Relative URLs with any number of ../ must be allowed.

How to reproduce?
Create a foo.jsp file in a foo-folder that just contains:
<%= response.encodeURL("../foo/bar.jsp") %>

Access the page http://[server]/foo/foo.jsp with a client that doesn't send cookies, or have no JSESSIONID cookie, like
curl http://[server]/foo/foo.jsp

This worked upto and including Jetty 9.4.36, but don't work with Jetty 9.4.37 and newer. So some update in 9.4.37 must have made this break.

I have 3rd party libraries that does this within servlets that I can't change, so it's a showstopper for me to upgrade to the latest version of Jetty.

@anca-jalbum anca-jalbum added the Bug For general bugs on Jetty side label Feb 17, 2022
@joakime joakime self-assigned this Feb 17, 2022
@joakime
Copy link
Contributor

joakime commented Feb 17, 2022

I haven't replicated this yet, but this issue should also be impacting HttpServletResponse.encodeRedirectURL(String) as well. (similar code path).

@joakime
Copy link
Contributor

joakime commented Feb 17, 2022

I have been able to replicate easily with this test case...

d9c6414

The HttpURI is used within the encodeURL(String) method only to parse out various values.
But our HttpURI doesn't support relative path only Strings.

@joakime
Copy link
Contributor

joakime commented Feb 17, 2022

Opened PR #7616 with a fix.

gregw added a commit that referenced this issue Mar 21, 2022
 + use presence of scheme to gate parsing as HttpURI

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Mar 21, 2022
+ use presence of scheme to gate parsing as HttpURI

Signed-off-by: Greg Wilkins <[email protected]>
@joakime
Copy link
Contributor

joakime commented Mar 21, 2022

Closing, Merged PR #7763

@joakime joakime closed this as completed Mar 21, 2022
@gregw
Copy link
Contributor

gregw commented Mar 21, 2022

Reopening as not yet merged to 10, 11 & 12!

@gregw gregw reopened this Mar 21, 2022
gregw added a commit that referenced this issue Mar 21, 2022
cherry-picked from 9c30caf

Signed-off-by: Greg Wilkins <[email protected]>
gregw added a commit that referenced this issue Mar 22, 2022
fixed checkstyle

Signed-off-by: Greg Wilkins <[email protected]>
@joakime joakime linked a pull request Mar 22, 2022 that will close this issue
joakime pushed a commit that referenced this issue Mar 22, 2022
* Fix #7615 encode relative URIs

cherry-picked from 9c30caf

Signed-off-by: Greg Wilkins <[email protected]>

* Fix #7615 encode relative URIs

fixed checkstyle

Signed-off-by: Greg Wilkins <[email protected]>
@joakime joakime reopened this Mar 22, 2022
@olamy olamy moved this to In progress in Jetty 12.0.ALPHAS Sep 7, 2022
@joakime joakime closed this as completed Oct 11, 2022
Repository owner moved this from In progress to Done in Jetty 12.0.ALPHAS Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
3 participants