RedirectView should allow for bypassing HttpServletResponse#encodeRedirectURL [SPR-13693]

[spring-projects-issues]

Marvin S. Addison opened SPR-13693 and commented

RedirectView presently calls HttpServletResponse#.encodeRedirectURL(targetUrl) unconditionally, which has the effect of appending ";jsessionid" path fragments to the redirect URL unexpectedly in some cases. While that is fine for (context) relative URLs, for absolute URLs the programmer typically intends to redirect to a foreign system that is not likely to understand the decorated URL. I believe it's most correct to add some conditional logic to detect an absolute URL and avoid the call to encodeRedirectURL in that case.

One could argue that the servlet container ought to provide this kind of conditional logic; however, it's been my observation that not all platforms do the right thing.

---

Affects: 4.2.2

Referenced from: commits c45ad30

1 votes, 6 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

I don't think it would be so simple. Redirecting to an absolute URL does not necessarily mean a foreign system. We'd have to compare the actual host and that may vary based on forwarded requests, x-forwarded headers from proxies, etc. Furthermore frameworks may also wrap the response and apply their own encoding logic.

Can you be more specific, which platform/container and under what circumstances it adds undesired "jsessionid"? From the Javadoc of encodRedirectURL:

* Encodes the specified URL for use in the
* <code>sendRedirect</code> method or, if encoding is not needed,
* returns the URL unchanged.  The implementation of this method
* includes the logic to determine whether the session ID
* needs to be encoded in the URL.  For example, if the browser supports
* cookies, or session tracking is turned off, URL encoding is
* unnecessary.  Because the rules for making this determination can
* differ from those used to decide whether to
* encode a normal link, this method is separated from the
* <code>encodeURL</code> method.

[spring-projects-issues]

Marvin S. Addison commented

My use case is Spring WebFlow where I'm using externalRedirect:#{absoluteURL}, and I think my assertion that the intent to redirect to a foreign system in that case is sound. In that case it's org.springframework.webflow.action.ExternalRedirectAction that's handling the redirection logic, not org.springframework.web.servlet.view.RedirectView as I had thought. Both components behave similarly, however, in that they unconditionally call HttpServletResponse#.encodeRedirectURL(targetUrl) under the hood.

I'm running my servlet on Jetty, which actually has logic in the Response object to try to detect whether the redirect is to a remote host:

https://github.com/eclipse/jetty.project/blob/jetty-9.3.6.v20151106/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java#L399

if (sessionManager.isCheckingRemoteSessionIdEncoding() && URIUtil.hasScheme(url))
{
    uri = new HttpURI(url);
    String path = uri.getPath();
    path = (path == null ? "" : path);
    int port = uri.getPort();
    if (port < 0)
        port = HttpScheme.HTTPS.asString().equalsIgnoreCase(uri.getScheme()) ? 443 : 80;
        // Is it the same server?
    if (!request.getServerName().equalsIgnoreCase(uri.getHost()))
        return url;
    if (request.getServerPort() != port)
        return url;
    if (!path.startsWith(request.getContextPath())) //TODO the root context path is "", with which every non null string starts
        return url;
}

Interestingly, the isCheckingRemoteSessionIdEncoding() flag arose out of a similar request, https://bugs.eclipse.org/bugs/show_bug.cgi?id=291448. I was able to work around the problem behavior in my case by toggling the flag, but I really think a global solution (at the servlet context level) is inappropriate. Ideally there would be additional handles/flags in the Spring components that allow the programmer to control behavior.

There absolutely is a case for sending a redirect to a remote system that does not understand jsessionid path parameters; programmers ought to be able to suppress that behavior on a case-by-case basis. That's to say that I agree with you that trying to infer meaning is problematic; it would be better to expose additional configuration to let the author decide.

[spring-projects-issues]

Rossen Stoyanchev commented

Okay thanks for the extra detail. What exactly do you think we should do?

We cannot assume by default that redirecting to a full URL means we can skip encodeRedirectURL. An application may be providing full URLs even when redirecting within the same application. It would also not be easy to determine if the complete URL is to the same application or not. Last but not least, it is not only the Servlet container that may want to encode the URL. Frameworks and applications too may wrap the response and implement encodeRedirectURL. I'm just having a hard time seeing what specifically we can do.

[spring-projects-issues]

Scott Cantor commented

Notwithstanding that it's a hard problem, I'd like to note that it's essentially a security bug to attach a session ID to the URL of a foreign system. So fundamentally the whole scheme being imposed on us by the containers is flawed, but it is what it is.

There are definitely application scenarios in which it's very simply to enumerate the "local" vhost prefixes. If that could be configured via a property to control the Spring layer's determination about whether to call that method, would that fly?

[spring-projects-issues]

Rossen Stoyanchev commented

BTW does the use case involve cookies being turned off? Just trying to understand the scope of the issue.

There are many instances where this is used by 3rd party libraries which decorate the response in order to hook in. They may be doing things for which the assumptions about what should and shouldn't be encoded may vary. In that same line of thought you have a similar option to wrap the response and intercept all calls to encodeRedirectURL. Generally speaking I hesitate to generally turn off the call to encodeRedirectURL at the framework level.

[spring-projects-issues]

Marvin S. Addison commented

+1 for Scott Cantor's proposal. I believe the following Spring components should respect the proposed flag:

1. org.springframework.webflow.action.ExternalRedirectAction
2. org.springframework.web.servlet.view.RedirectView

[spring-projects-issues]

Marvin S. Addison commented

The original use case where I first saw the problem was one where cookies were disabled, yes. We just identified a new case today where a rewrite rule is causing the cookie to not be returned to the server due to path mismatches. That's arguably a configuration problem, but a different root cause.

We not suggesting to disable encodeRedirectURL globally, but selectively according to some configuration parameter that specifies the "local" host names. If the parameter is defined, encoding is performed only for those URLs whose host matches a value in that list; otherwise not. If the configuration parameter is not defined, behavior is as it is today.

[spring-projects-issues]

Scott Cantor commented

No, it's happening in the case Marvin and I are seeing because of some rewrites the deployer is doing that are fooling the container into thinking it needs to re-encode the ID on the URL.

Your point is well-taken though, if encodeRedirectURL is a filterable method on HttpServletResponse, that would be another way.

I wasn't, obviously, suggesting you would default this off in Spring or SWF, just that putting the logic the filter/wrapper would have to perform into a property-driven feature would obviously make that available to more people with less duplication. Absent a property value to whitelist anything, Spring would keep doing what it does now I think.

[spring-projects-issues]

Juergen Hoeller commented

Elaborating on Rossen's point: encodeRedirectURL is also used for decoration purposes in Servlet Filter implementations, usually post-processing selected URLs after the Servlet container itself encoded the URL, e.g. for conversation ids and other kinds of user tracking. So we cannot simply assume that its sole purpose is session id encoding if cookies are turned off.

As Rossen hinted at, this can also be used for workarounds for the time being: A custom Filter could selectively suppress the delegation to the target encodeRedirectURL call, e.g. based on a host analysis, simply returning the given URL as-is in this case. Point taken though that we should have a first-class way to bypassing such encoding, at least as of 4.3.

Juergen

[spring-projects-issues]

Rossen Stoyanchev commented

Let me know if this fix works for you.

[spring-projects-issues]

Scott Cantor commented

Yes, I see where we'd wire that in easily. Thank you.

[spring-projects-issues]

Marvin S. Addison commented

Looks straightforward for the MVC case. How would one leverage that via "externalRedirect" SpEL?

[spring-projects-issues]

Rossen Stoyanchev commented

I think we'd have to add that separately in Web Flow. If you could please open one jira.springsource.org/browse/SWF.

[spring-projects-issues]

Scott Cantor commented

SWF relies on the MVC view resolvers, so I thought it would end up in the same code. Maybe they don't process the redirects SWF is doing?

[spring-projects-issues]

Marvin S. Addison commented

https://jira.spring.io/browse/SWF-1680

[spring-projects-issues]

Rossen Stoyanchev commented

These are all Web Flow constructs. Spring MVC has no idea what "externalRedirect:" is.