Bug 58660

Summary: Different redirect behaviour when accessing path without a /
Product: Tomcat 8 Reporter: per.lewau
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal CC: dushkin
Priority: P2    
Version: 8.0.29   
Target Milestone: ----   
Hardware: PC   
OS: All   

Description per.lewau 2015-11-27 09:02:31 UTC
Hi,

Tomcat 8.0.29 changes the behaviour of requests for paths without a trailing slash. It seems to be limited to context roots, e.g. /examples instead of /examples/servlets.

When calling /examples on 8.0.28 it responds with a 302 redirecting to /examples/, but 8.0.29 responds with a 200.

I would say that people should be more careful with their URL:s, but this change in default behaviour is probably going to cause problems. It is probably not correct to respond with /examples/index.html, when accessing /examples. This changes any relative paths on a website, since there will be a / missing. This may indeed be verified by clicking any of the links in the 8.0.29 /examples response HTML. Clicking them causes the browser to fetch e.g. /servlets instead of /examples/servlets.

Accessing /examples/servlets however causes a 302 to /examples/servlets/, just like 8.0.28.


To reproduce:

1. Download Tomcat 8.0.28 and 8.0.29

% wget http://archive.apache.org/dist/tomcat/tomcat-8/v8.0.28/bin/apache-tomcat-8.0.28.zip
% wget http://apache.mirrors.spacedump.net/tomcat/tomcat-8/v8.0.29/bin/apache-tomcat-8.0.29.zip

Unzip the two archives.

2. Access /examples for Tomcat 8.0.28

% curl -vso /dev/null http://localhost:8080/examples
* STATE: INIT => CONNECT handle 0x6000572f0; line 1090 (connection #-5000)
* Added connection 0. The cache now contains 1 members
*   Trying ::1...
* STATE: CONNECT => WAITCONNECT handle 0x6000572f0; line 1143 (connection #0)
* Connected to localhost (::1) port 8080 (#0)
* STATE: WAITCONNECT => SENDPROTOCONNECT handle 0x6000572f0; line 1240 (connection #0)
* STATE: SENDPROTOCONNECT => DO handle 0x6000572f0; line 1258 (connection #0)
> GET /examples HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.45.0
> Accept: */*
>
* STATE: DO => DO_DONE handle 0x6000572f0; line 1337 (connection #0)
* STATE: DO_DONE => WAITPERFORM handle 0x6000572f0; line 1464 (connection #0)
* STATE: WAITPERFORM => PERFORM handle 0x6000572f0; line 1474 (connection #0)
* HTTP 1.1 or later with persistent connection, pipelining supported
< HTTP/1.1 302 Found
* Server Apache-Coyote/1.1 is not blacklisted
< Server: Apache-Coyote/1.1
< Location: http://localhost:8080/examples/
< Transfer-Encoding: chunked
< Date: Fri, 27 Nov 2015 08:36:11 GMT
<
{ [5 bytes data]
* STATE: PERFORM => DONE handle 0x6000572f0; line 1632 (connection #0)
* Curl_done
* Connection #0 to host localhost left intact
* Expire cleared


3. Access /examples for Tomcat 8.0.29

* STATE: INIT => CONNECT handle 0x6000572f0; line 1090 (connection #-5000)
* Added connection 0. The cache now contains 1 members
*   Trying ::1...
* STATE: CONNECT => WAITCONNECT handle 0x6000572f0; line 1143 (connection #0)
* Connected to localhost (::1) port 8080 (#0)
* STATE: WAITCONNECT => SENDPROTOCONNECT handle 0x6000572f0; line 1240 (connection #0)
* STATE: SENDPROTOCONNECT => DO handle 0x6000572f0; line 1258 (connection #0)
> GET /examples HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.45.0
> Accept: */*
>
* STATE: DO => DO_DONE handle 0x6000572f0; line 1337 (connection #0)
* STATE: DO_DONE => WAITPERFORM handle 0x6000572f0; line 1464 (connection #0)
* STATE: WAITPERFORM => PERFORM handle 0x6000572f0; line 1474 (connection #0)
* HTTP 1.1 or later with persistent connection, pipelining supported
< HTTP/1.1 200 OK
* Server Apache-Coyote/1.1 is not blacklisted
< Server: Apache-Coyote/1.1
< Accept-Ranges: bytes
< ETag: W/"1156-1448007578000"
< Last-Modified: Fri, 20 Nov 2015 08:19:38 GMT
< Content-Type: text/html
< Content-Length: 1156
< Date: Fri, 27 Nov 2015 08:36:38 GMT
<
{ [1156 bytes data]
* STATE: PERFORM => DONE handle 0x6000572f0; line 1632 (connection #0)
* Curl_done
* Connection #0 to host localhost left intact

Cheers, 
Per Lewau
Comment 1 Mark Thomas 2015-11-27 09:04:08 UTC
Read the change log.
Comment 2 Mark Thomas 2015-11-27 09:12:32 UTC
The change in where processing takes place is intentional but the end result should have been the same. Need to look at what is going on for the context root.
Comment 3 Konstantin Kolinko 2015-11-27 09:20:57 UTC
This is reproducible in Tomcat 7.0.66 (release candidate) as well.
Comment 4 Konstantin Kolinko 2015-11-27 09:44:16 UTC
Tomcat 8.0.29 - Workaround:
In conf/context.xml set the following attribute:
<Context mapperContextRootRedirectEnabled="true">


Tomcat 7.0.66 (release candidate) - The workaround does not work.

The cause is that MapperListener calls wrong (deprecated) version of mapper.addContextVersion() method and does not pass the flags to the Mapper.
Comment 5 Remy Maucherat 2015-11-27 10:32:24 UTC
Specifically for the context root, the default behavior could be to redirect in the mapper (mapperContextRootRedirectEnabled could be true).
Comment 6 Mark Thomas 2015-11-27 11:31:05 UTC
I'm neutral on the default behaviour for context root redirects but either way the current issue needs to be fixed. I'm currently working on this.
Comment 7 Remy Maucherat 2015-11-27 11:44:26 UTC
Well, I messed up as well, since I was certain this was fine (the code change in the default servlet should have redirected the root path as far as I am concerned), although my own patch never removed the "" -> "/" redirect in the mapper of course.
Comment 8 Konstantin Kolinko 2015-11-27 11:49:21 UTC
One more place that fails - a FormAuthenticator if it is configured to a web application as a whole and if its authentication form uses a relative URL to j_security_check, like the examples app does.

8.0.29
1. In webapps/examples/WEB-INF/web.xml
In <security-constraint> elements (there are 2 of them) replace 
         <url-pattern>/jsp/security/protected/*</url-pattern>
with
         <url-pattern>/*</url-pattern>

2. In conf/tomcat-users.xml uncomment test users.

3. Start Tomcat

4. Go to http://localhost:8080/examples

A login form is displayed.

5. Type in user name and password (as configured in conf/tomcat-users.xml)

6. The login form sends POST request to
http://localhost:8080/j_security_check;jsessionid=<...>

This fails with 404, because request goes to a wrong web application (ROOT, not examples).


One more area of concern: anything that sets a cookie before a redirect happens. See "sessionCookiePathUsesTrailingSlash" option on Context. 

(FormAuthenticator is an example here as well, but as I wrote above there is a more serious issue with it.)
Comment 9 Remy Maucherat 2015-11-27 12:06:26 UTC
Well, since we think everything else works, then the easiest/safest is to unconditionally add back the root path redirect in the mapper.
Comment 10 Mark Thomas 2015-11-27 12:14:34 UTC
You say that just as I think I have put together a fix. Happy to change the default but I'd like to keep the option of having the Default Servlet do the redirect
Comment 11 Remy Maucherat 2015-11-27 13:53:47 UTC
The issue is probably that the mapper rewrites (for mapping purposes I think) a "no servlet path" as "/", and the default servlet has no way to know this happened. If I remove this rewrite code, it doesn't work any better though.

If you make the root path redirection in default servlet completely functional, then I'd say it's fine to keep the option (even enabled by default), but otherwise ...

If it is acked avoiding the root path redirect in the mapper is not something mandatory, it should be possible to add it back and remove the configuration option for now to resolve the regression, and work on it for the next release.
Comment 12 Mark Thomas 2015-11-27 14:03:29 UTC
That is exactly the issue I have been working through. I think I have it solved for the Default Servlet. I'm currently working on issue with FORM login at the context root that Konstantin highlighted.
Comment 13 Mark Thomas 2015-11-27 14:54:26 UTC
I've committed my first pass at a fix for this to 9.0.x. Please test and I'll work on a 8.0.x release as soon as we are happy with this.

At this point I'm still neutral on what the default should be for mapperContextRootRedirectEnabled. I think Remy is in favor of it being true. Any other views?
Comment 14 Remy Maucherat 2015-11-27 15:35:43 UTC
Testing looks ok (but I probably tested the same things you did). I would have preferred not adding the root path redirect flag though.
Comment 15 Mark Thomas 2015-11-27 16:32:37 UTC
I think there are valid use cases for the flag so I'd like to keep it but I'd be happy to change the default.

I found a few issues with my first pass which I have now fixed. I'm currently thinking more testing now and over the weekend with a back-port to 8.0.x towards the end of the weekend and tag on Monday.
Comment 16 Mark Thomas 2015-11-28 15:24:37 UTC
*** Bug 58665 has been marked as a duplicate of this bug. ***
Comment 17 Mark Thomas 2015-11-30 09:39:02 UTC
Fixed in 9.0.x (for 9.0.0.M2 onwards), 8.0.x (for 8.0.30 onwards), 7.0.x (for 7.0.67 onwards) and 6.0.x (for 6.0.45 onwards).
Comment 18 Andrii Pitukh 2016-01-22 15:03:32 UTC
Hi, this defect is still reproducible in 8.0.30. Is it really fixed or should I use mapperContextRootRedirectEnabled parameter? It is unclear from dev discussion about solution.
Comment 19 Mark Thomas 2016-01-22 15:05:48 UTC
This issue is fixed in 8.0.30

Bugzilla is not a support. If you have questions, please use the users' mailing list. If that discussion identifies a further regression then this can be re-opened.
Comment 20 Andrii Pitukh 2016-01-22 15:18:41 UTC
I've just reproduced following case in 8.0.30 (8.0.28 is ok):

Steps:
1. Download Tomcat installation
2. Open webapps/examples/WEB-INF/web.xml folder
3. Change lines:
	<servlet-mapping>
        <servlet-name>ChatServlet</servlet-name>
        <url-pattern>/servlets/chat/chat</url-pattern>
    </servlet-mapping>
	
	to
	
	<servlet-mapping>
        <servlet-name>ChatServlet</servlet-name>
        <url-pattern>/</url-pattern>
    </servlet-mapping>
5. Open http://localhost:8080/examples?key=param 

Expected result:
Server returns response with status 302 to redirect user to http://localhost:8080/examples/?key=param and then to http://localhost:8080/examples/login.jsp

Actual result:
Server returns response with status 302 to redirect user to http://localhost:8080/login.jsp
Comment 21 Mark Thomas 2016-01-22 17:22:38 UTC
That behaviour is expected in 8.0.30. The ChatServlet isn't written to handle reuqests to the root of the context that don't have a trailing slash. Whether it should or not is open to debate.

The default for mapperContextRootRedirectEnabled has changed for back to the pre 8.0.29 behaviour for 8.0.31 onward.
Comment 22 Andrii Pitukh 2016-01-25 10:38:09 UTC
(In reply to Mark Thomas from comment #21)
> That behaviour is expected in 8.0.30. The ChatServlet isn't written to
> handle reuqests to the root of the context that don't have a trailing slash.
> Whether it should or not is open to debate.
> 
> The default for mapperContextRootRedirectEnabled has changed for back to the
> pre 8.0.29 behaviour for 8.0.31 onward.

Ok, so I'll just set to "true" mapperContextRootRedirectEnabled and mapperDirectoryRedirectEnabled, to restore old behavior and not bother about default values.

BTW, in changelog (https://tomcat.apache.org/tomcat-8.0-doc/changelog.html) it's said:
"Move the functionality that provides redirects for context roots and directories where a trailing / is added from the Mapper to the DefaultServlet."

And in "context config" (https://tomcat.apache.org/tomcat-8.0-doc/config/context.html) the description of mapperContextRootRedirectEnabled is following:
"If enabled, requests for a web application context root will be redirected (adding a trailing slash) if necessary by the Mapper rather than the default Servlet.".

I suppose, you need to update "context config" given that redirect functionality is now only in DefaultServlet.
Comment 23 Isaac Wu 2016-11-21 07:31:48 UTC
Hi I saw the issue is marked as fixed in 7.0.67/8.0.30 changelog. However as I tried to set mapperContextRootRedirectEnabled=true (in the latest version 7, 8.0.73) to restore the behavior in the previous releases, the url without a trailing slash still doesn't work (404). So I would like to check if anyone find the parameter mapperContextRootRedirectEnabled really works.