ForwardedHeaderFilter should support cases where contextPath should not be replaced with X-Forwarded-Prefix [SPR-14376]

[spring-projects-issues]

Thibaud Lepretre opened SPR-14376 and commented

With latest ForwardedHeaderFilter executed on Spring application that is using context-path

server.context-path: /bar

If X-Forwarded-Prefix: /foo is present the following url http://blabla.com/bar/oauth/authorize will be converted to http://blabla.com/foo/oauth/authorize

However with following architecture (1) Reverse proxy SSL with context path /foo, (2) Zuul without context path and (3) Microservice BAR with context path /bar: rendering request will not work because http://blabla.com/foo/oauth/authorize will be 404.

Expected result should be http://blabla.com/foo/bar/oauth/authorize.

However I'm aware that for some other use cases context-path should be replaced and not prepended (like I wish). So the issue is debatable.

Possible solutions:

1. Clearly document that X-Forwarded-Prefix will not really prefix but replace the existing context-path
2. Add option to choose strategy between prepending or replacing
3. If you consider it as a bug, just prepend.

For example Spring cloud netflix prepends X-Forwarded-Prefix during Zuul filtering https://github.com/spring-cloud/spring-cloud-netflix/pull/994/files#diff-8a3a3948fd59a02ea4234d960437c3a0R120

---

Affects: 4.3 GA

Issue Links:

• ForwardedHeaderFilter could support X-Forwarded-Prefix as well [SPR-14270] #18842 ForwardedHeaderFilter could support X-Forwarded-Prefix as well
• ForwardedHeaderFilter should support case insensitive header name [SPR-14372] #18945 ForwardedHeaderFilter should support case insensitive header name

[spring-projects-issues]

Thibaud Lepretre commented

I missed the 4th possible solution: this is not an issue from your point of view so no need anything

[spring-projects-issues]

Rossen Stoyanchev commented

Indeed this very question, to replace the context path or prepend it, was discussed under #18842 and votes went for replacing the context path.

Dave Syer, Rob Winch here is the counter-example. Thoughts?

[spring-projects-issues]

Rossen Stoyanchev commented

Updated title (was: "ForwardedHeaderFilter with X-Forwarded-Prefix clarification")

[spring-projects-issues]

Dave Syer commented

The counter example makes sense to me, but is all depends on how the data are used. Fundamentally, the app has all the data it needs to create links to itself, so it shouldn't be a problem. The X-Forwarded-Prefix thing is to do with the upstream proxy, and that data needs to be taken into account irrespective of the implementation of the local app. But if the local app has a context path it should make its upstream consumers aware of that by rendering links correctly, including both prefixes.

From #18842: "the prefix needs to be applied instead of the context path, not on top of it". I think that statement is still true. The order is important. The most important thing is to ensure that consumers of the HttpServletRequest know about full path that the caller used when addressing the app. I don't think it should be complicated, and there has to be a single right answer, so it's a shame if we haven't got there yet, and I apologise if I contributed to the confusion.

At least it all still works if the app has the default context path ("/").

[spring-projects-issues]

Rossen Stoyanchev commented

In this case the full path that the caller used is "http://blabla.com/foo/bar/oauth/authorize" and by the time it gets to the app, "/foo" is the X-Forwarded-Prefix and "/bar" is the context path. We replace the context path with the current approach and end up with "http://blabla.com/foo/oauth/authorize". How does the application now go about re-inserting its context path?

As far as I can see depending on the scenario sometimes replacing the contextPath with the prefix makes sense, while there are other cases where it makes sense to prepend the prefix. This sounds more like a configuration option on ForwardedFilter (something like an overrideContextPath boolean where false would mean keep it and prepend the prefix). So is there a single right solution or does it come down to which case you have?

[spring-projects-issues]

Thibaud Lepretre commented

I'm totally agree about the fact that replacing or prepending both make sense depending of use case so we can't replace one from other. You can keep replacing by default but if you can provide such boolean to switch strategy it will be 100% ok from my point of view

[spring-projects-issues]

Thibaud Lepretre commented

Finally after thinking about my issue more deeply I don't think you should add preprend strategy. Because app should not be configured depending on possible reverse proxy or edge service. I still prefer create a custom Zuul filter for my case.

[spring-projects-issues]

Rossen Stoyanchev commented

Resolving for now.

[antechrestos]

I comment a little bit late yet I think that at least ForwardedHeaderFilter should offer the possiblity to implement the prepend strategy programmatically without closing it by private  static classes