spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.25k stars 37.98k forks source link

Inconsistent handling of X-Forwarded-Prefix in servlet and reactive stack #33465

Closed krezovic closed 2 weeks ago

krezovic commented 2 weeks ago

Affects: 6.1

I have stumbled upon hard-to-debug issue which affects the reactive stack (webflux) but works fine in servlet stack (webmvc)

The spring-cloud-gateway had sent the X-Forwarded-Prefix = '/myapp,/' header due to multiple "rewrite" usages.

Now, with server.forward-headers-strategy=framework everything works fine in servlet stack.

However, in reactive stack, it fails with

2024-09-02T16:21:11.153+02:00 DEBUG 12844 --- [o-auto-1-exec-1] o.s.w.s.adapter.HttpWebHandlerAdapter    : Failed to apply forwarded headers to HTTP GET "/test"

java.lang.IllegalArgumentException: Invalid contextPath: '/forwardedprefix/': must start with '/' and not end with '/'
        at org.springframework.http.server.DefaultRequestPath.validateContextPath(DefaultRequestPath.java:77) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.DefaultRequestPath.initContextPath(DefaultRequestPath.java:56) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.DefaultRequestPath.<init>(DefaultRequestPath.java:41) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.RequestPath.parse(RequestPath.java:79) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.RequestPath.parse(RequestPath.java:68) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.reactive.AbstractServerHttpRequest.<init>(AbstractServerHttpRequest.java:89) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.reactive.DefaultServerHttpRequestBuilder$MutatedServerHttpRequest.<init>(DefaultServerHttpRequestBuilder.java:195) ~[spring-web-6.1.12.jar:6.1.12]
        at org.springframework.http.server.reactive.DefaultServerHttpRequestBuilder.build(DefaultServerHttpRequestBuilder.java:135) ~[spring-web-6.1.12.jar:6.1.12]

I'd appreciate if this was unified across both servlet and reactive stacks. It took quite a while to debug this as we kept receiving error 400 and no @ControllerAdvice @ExceptionHandler was being called.

Reproducer:

https://github.com/krezovic/spring-forwardedprefix

Simply run ./mvnw test

sdeleuze commented 2 weeks ago

Looks like a off-by-one error in ForwardedHeaderTransformer#getForwardedPrefix.

quaff commented 2 weeks ago

May I ask why not /myapp but /myapp,/ ?

krezovic commented 2 weeks ago

It was a misconfiguration;

We tried to implement sub-path based routing, but rather than using spring-cloud-gateway builtin "stripPrefix" filter, we used "rewritePath" twice - and each rewritePath invocation adds a segment to X-Forwarded-Prefix

We did eventually switch to stripPrefix, so it ended up as /myapp ... but that was after several hours of debugging "why the webflux app suddenly fails"

Since it went unnoticed for more than a year on our side, I figured it would be nice to report the issue to save somebody else the trouble.