Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve host header and pass x-forwarded headers #104

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

edevosc2c
Copy link
Member

@edevosc2c edevosc2c commented Feb 16, 2024

This fixes both bug reported in #102

Also, this helps with the issue #94 like explained in #94 (comment)

I made the decision to avoid the gateway modifying the x-forwarded-* headers so that the final application receive the headers in the correct format sent by the original reverse proxy.

You can find documentation about the changes here:

@f-necas
Copy link
Collaborator

f-necas commented Feb 19, 2024

Tested and it works well. Didn't notice impact when it's set on all routes by now.

@edevosc2c edevosc2c merged commit 95c775c into main Feb 20, 2024
2 of 3 checks passed
@edevosc2c edevosc2c deleted the preserve-host-pass-x-forwarded branch February 20, 2024 09:33
@pmauduit
Copy link
Member

This broke the testsuite, we should have been more wary about it ; reverting the application.yml file by removing the PreserveHostHeader filter fixes it.

pmauduit added a commit that referenced this pull request Feb 20, 2024
Following #104, the `Host` http header became mandatory, else throwing a
NPE in Netty's code when reaching the NettyRoutingFilter code:

```
java.lang.NullPointerException: value
	at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
	Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
		at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117)
		at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364)
		at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139)
		at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137)
		at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117)
```

Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row.

Notes:

* It might deserve a trace in the documentation that the `Host`
  header is now mandatory, somehow ?
* I did not have to patch every WebTestClient calls strangely enough,
  only the one expecting a `200 - OK` return code.
pmauduit added a commit that referenced this pull request Feb 20, 2024
Following #104, the `Host` http header became mandatory, else throwing a
NPE in Netty's code when reaching the NettyRoutingFilter code:

```
java.lang.NullPointerException: value
	at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
	Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
		at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117)
		at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364)
		at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139)
		at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137)
		at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117)
```

Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row.

Notes:

* It might deserve a trace in the documentation that the `Host`
  header is now mandatory, somehow ?
* I did not have to patch every WebTestClient calls strangely enough,
  only the one expecting a `200 - OK` return code.
f-necas pushed a commit that referenced this pull request May 17, 2024
Following #104, the `Host` http header became mandatory, else throwing a
NPE in Netty's code when reaching the NettyRoutingFilter code:

```
java.lang.NullPointerException: value
	at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
	Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
		at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117)
		at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364)
		at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139)
		at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137)
		at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117)
```

Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row.

Notes:

* It might deserve a trace in the documentation that the `Host`
  header is now mandatory, somehow ?
* I did not have to patch every WebTestClient calls strangely enough,
  only the one expecting a `200 - OK` return code.
f-necas pushed a commit that referenced this pull request May 17, 2024
Following #104, the `Host` http header became mandatory, else throwing a
NPE in Netty's code when reaching the NettyRoutingFilter code:

```
java.lang.NullPointerException: value
	at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
	Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
		at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117)
		at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364)
		at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139)
		at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137)
		at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117)
```

Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row.

Notes:

* It might deserve a trace in the documentation that the `Host`
  header is now mandatory, somehow ?
* I did not have to patch every WebTestClient calls strangely enough,
  only the one expecting a `200 - OK` return code.
f-necas pushed a commit that referenced this pull request May 28, 2024
Following #104, the `Host` http header became mandatory, else throwing a
NPE in Netty's code when reaching the NettyRoutingFilter code:

```
java.lang.NullPointerException: value
	at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
	Suppressed: The stacktrace has been enhanced by Reactor, refer to additional information below:
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.cloud.gateway.filter.WeightCalculatorWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.AuthorizationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authorization.ExceptionTranslationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.logout.LogoutWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.savedrequest.ServerRequestCacheWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.authentication.AuthenticationWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.context.ReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers$MutatorFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP GET "/path/..." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:39)
		at io.netty.handler.codec.DefaultHeaders.fromObject(DefaultHeaders.java:1117)
		at io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:364)
		at io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:115)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.lambda$filter$0(NettyRoutingFilter.java:139)
		at reactor.netty.http.client.HttpClient.headers(HttpClient.java:1038)
		at org.springframework.cloud.gateway.filter.NettyRoutingFilter.filter(NettyRoutingFilter.java:133)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$GatewayFilterAdapter.filter(FilteringWebHandler.java:137)
		at org.springframework.cloud.gateway.filter.OrderedGatewayFilter.filter(OrderedGatewayFilter.java:44)
		at org.springframework.cloud.gateway.handler.FilteringWebHandler$DefaultGatewayFilterChain.lambda$filter$0(FilteringWebHandler.java:117)
```

Tests: `mvn clean verify` on gateway subdir went fine 3 times in a row.

Notes:

* It might deserve a trace in the documentation that the `Host`
  header is now mandatory, somehow ?
* I did not have to patch every WebTestClient calls strangely enough,
  only the one expecting a `200 - OK` return code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants