Bug 58655 - IllegalStateException: calling HttpServletResponse#sendRedirect() with RemoteIpFilter
Summary: IllegalStateException: calling HttpServletResponse#sendRedirect() with Remote...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.0.x-trunk
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-26 11:52 UTC by Cristian Klein
Modified: 2015-12-02 09:41 UTC (History)
0 users



Attachments
stacktrace of error (3.06 KB, text/plain)
2015-11-26 11:52 UTC, Cristian Klein
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Cristian Klein 2015-11-26 11:52:10 UTC
Created attachment 33299 [details]
stacktrace of error

When calling `HttpServletResponse#sendRedirect()`, if `RemoteIpFilter` is in use, an `IllegalStateException` is thrown (see attached stack-trace). 

The error steams from the way `RemoteIpFilter.XForwardedResponse` tries to rewrite the 'Location' header. According to the servlet API, a response is considered committed after calling `sendRequest()` and it is illegal to call either `reset()` or another `sendRequest()` thereafter.

WORKAROUND:

Comment the code after `super.sendRedirect(location);`. However, this effectively disables the `RemoteIpFilter` on the response path and makes the server return an URL with incorrect scheme.

[1] https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/filters/RemoteIpFilter.java#L679

[2] https://tomcat.apache.org/tomcat-8.0-doc/servletapi/index.html
Comment 1 Cristian Klein 2015-11-26 11:54:10 UTC
Forgot to mention, this bug is not triggered with Eclipse's servlet engine (I think Jetty), but only occurs when the servlet is deployed on Tomcat.
Comment 2 Mark Thomas 2015-11-27 21:21:56 UTC
Using relative redirects (see bug 56917) should make this fixable.
Comment 3 Mark Thomas 2015-11-30 16:30:09 UTC
Thanks for the report.

This has been fixed in 9.0.x (for 9.0.0.M2), 8.0.x (for 8.0.30) and 7.0.x (for 7.0.67).

6.0.x was not affected.
Comment 4 Cristian Klein 2015-11-30 19:00:15 UTC
Thanks for the fix. I'm not sure to understand how the fix helps. What line or what mechanism rewrites the scheme from `http` to `https`?
Comment 5 Mark Thomas 2015-11-30 20:09:43 UTC
The scheme isn't re-written. If you redirect using an absolute URI with a specific scheme then that is what you get. If you want the scheme to be "rewritten"/correct then use a relative redirect.
Comment 6 Cristian Klein 2015-12-01 10:46:52 UTC
Are you sure this works? The "Location" header eventually has to contain the absolute URL. [1] If this is left to the "non-RemoteIpFilter" code, wouldn't the scheme be filled incorrectly?

[1] http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html
Comment 7 Mark Thomas 2015-12-01 11:05:48 UTC
RFC2616 is obsolete. See RFC7231.
Comment 8 Konstantin Kolinko 2015-12-01 16:29:00 UTC
Is there a reason why one uses RemoteIpFilter ? There exists a RemoteIpValve that can be used instead.

1. There are redirects that are performed before a request reaches the filter.
E.g. when using a FORM authentication (FormAuthenticator)

It cannot be solved by using a filter. One has to use RemoteIpValve here.


2. There is an edge case. It is allowed to call sendRedirect() with an absolute URL. With simple implementation (using relative redirects) it won't be rewritten.

You have to bear with it (such calls are unlikely) or duplicate a lot of code from o.a.c.connector.Response.sendRedirect() to implement this feature.
Comment 9 Cristian Klein 2015-12-02 09:41:03 UTC
I'm confused. I thought `RemoteIpValve` was deprecated in favour of `RemoteIpFilter`. Otherwise, I feel they both serve the same purpose.