Bug 62150 - Behavior of relative paths with RequestDispatcher has changed
Summary: Behavior of relative paths with RequestDispatcher has changed
Status: RESOLVED WONTFIX
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.5.16
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 62851 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-03-01 22:58 UTC by Myron Uecker
Modified: 2021-05-11 16:03 UTC (History)
1 user (show)



Attachments
Patch (1.31 KB, patch)
2018-03-02 09:07 UTC, Remy Maucherat
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myron Uecker 2018-03-01 22:58:37 UTC
It appears that relative paths now resolve differently on the request URI when forwarding between pages.

Prior to 8.5.16, if you were currently at a url of the form:
    test/mypage.jsp

and you attempted to forward the request to another page using a relative path, you could use
    ../test/mypage2.jsp

The Request URI would resolve to
    test/mypage2.jsp

After 8.5.16, the Request URI now resolves to
    test/../test/mypage2.jsp
Comment 1 Remy Maucherat 2018-03-02 09:07:10 UTC
Created attachment 35750 [details]
Patch

Ok, I think it is risky to not use the normalized path instead. r1799115 which changed this is still kind of odd to me, so I'll let Mark review the thing.
Comment 2 Mark Thomas 2018-03-02 12:30:05 UTC
It maybe that the scope of r1799115 that was intended to fix Bug 61185 was a little too wide. I'll take a look.
Comment 3 Myron Uecker 2018-03-02 13:19:10 UTC
It was one of those questionable decisions somebody made years ago in our code to use relative paths for navigation that no longer works after that change. Thank you for looking at this.
Comment 4 Remy Maucherat 2018-03-02 13:40:53 UTC
Using a relative path to get the RD is not bad. However, far more unusual is using the requestURI, since you have to deal with annoying stuff when you do that (URL encoding ...).
Comment 5 Mark Thomas 2018-03-02 16:07:53 UTC
Indeed. The behaviour of getRequestURI() is at the root of this bug report and bug 61185.

In terms of guidance from the spec, what we have is:

- getRequestURI() Returns the part of this request's URL from the protocol name up to the query string in the first line of the HTTP request. The web container does not decode this String.

- For forward(), the path elements of the request object exposed to the target servlet must reflect the path used to obtain the RequestDispatcher.


The implication that the value returned by getRequestURI() is unencoded is that it is also not normalized. This is because, for untrusted URIs, you have to decode first to ensure any encoded '.' or '/' characters are correctly handled.

Note that for a RequestDispatcher we have a little more leeway because the paths are trusted.

Bug 61185 was, essentially, that if an encoded path was used to obtain a RequestDispatcher then when getRequestURI() was called it should return that original, encoded path.

By extension, if a non-normalized absolute path is used to obtain a request dispatcher then the expectation is that, for a forward(), getRequestURI() should return that original, non-normalized path.

Where things get tricky is when a RequestDispatcher is obtained via a relative path. Which path should the relative path be resolved against (original or decoded + normalized) and should the path be normalized after it has been made absolute?

Maybe some examples will help:

Original         RD Path         URI after forward()
/aaa/bbb         zzz             /aaa/zzz
/aaa/../bbb      zzz             /zzz
                                 /aaa/../zzz
/aaa/bbb         ../zzz          /zzz
                                 /aaa/../zzz
/aaa/bbb/../ccc  zzz/xxx/../yyy  /aaa/bbb/../zzz/xxx/../yyy
                                 /aaa/zzz/xxx/../yyy
                                 /aaa/zzz/yyy

Looking at these I'm leaning towards the current behaviour as being closest to the intention of the spec but I confess that is a purely subjective judgement.

Feedback and further thoughts on the above welcome.

If we do want to normalize something here, I do think the proposed patch isn't quite right as a RequestDispatcher obtained with a non-normalized path would not then return that non-normalized path for getRequestURI() after a forward. If we go this route I think the normalization would need to be earlier.
Comment 6 Remy Maucherat 2018-03-02 16:33:15 UTC
Ok, so the option is to put a normalize step in Request.getRequestDispatcher if this is a relative path. The problem is it's a bit more annoying to do it earlier as it needs splitting the query string before putting it back.

If you think it is compliant, let's do nothing. Or it could be something optional with a config setting (there's context.getDispatchersUseEncodedPaths too). I'll update the bug to be a possible enhancement.

Overall, the spec provides requestURI, but it's mostly useless as you have to replicate the same complex processing on it that the container does to avoid security problems.
Comment 7 Remy Maucherat 2018-10-26 15:34:07 UTC
*** Bug 62851 has been marked as a duplicate of this bug. ***
Comment 8 Mark Thomas 2021-05-11 16:03:44 UTC
Reviewing this, I am resolving it as WONTFIX.