spring-projects / spring-framework

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

HttpHeaders.writeableHttpHeaders(new HttpHeaders(readOnlyHttpHeaders)) is not writeable #33789

Closed rwinch closed 1 month ago

rwinch commented 1 month ago

Workaround https://github.com/spring-projects/spring-security/issues/15989#issuecomment-2442660753

HttpHeaders.writeableHttpHeaders does not create writeable HttpHeaders if new HttpHeaders(readOnlyHttpHeaders) is passed in. This is required by Spring Security's StrictServerWebExchangeFirewall to perform lazy validation of the HttpHeaders name/value pairs.

Here is a complete, minimal test with no external dependencies that fails:

@Test
void writeHttpHeadersWhenNewHttpHeadersFails() {
    HttpHeaders originalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(new HttpHeaders());
    HttpHeaders firewallHeaders = new HttpHeaders(originalExchangeHeaders);
    HttpHeaders writeable = HttpHeaders.writableHttpHeaders(firewallHeaders);
    writeable.set("test", "this"); // throws UnsupportedOperationException
}

This test demonstrates the issue with a little more context as to what is happening in StrictServerWebExchangeFirewall :

@Test
void writableHttpHeadersWhenStrictFirewallHttpHeadersFails() {
    // originalExchangeHeaders is what is passed in to WebFilter by the framework
    HttpHeaders originalExchangeHeaders = HttpHeaders.readOnlyHttpHeaders(new HttpHeaders());
    // Spring Security replaces the headers with a firewalled set of headers that
    // lazily validates headers as they are accessed
    HttpHeaders firewallHeaders = new StrictFirewallHttpHeaders(originalExchangeHeaders);
    // Spring Cloud attempts to modify the now firewalled HttpHeaders, but writeableHttpHeaders does not work
    HttpHeaders writeable = HttpHeaders.writableHttpHeaders(firewallHeaders);
    writeable.set("test", "this"); // throws UnsupportedOperationException
}

// minimal demonstration of Spring Security's HttpHeaders
private static class StrictFirewallHttpHeaders extends HttpHeaders {
    private StrictFirewallHttpHeaders(HttpHeaders original) {
        super(original);
    }

    @Override
    public String getFirst(String headerName) {
        String result = super.getFirst(headerName);
        // validate headerName / result
        return result;
    }

    // ... other validation
}

Please make it so that HttpHeaders.writableHttpHeaders returns HttpHeaders that are writeable so Spring Cloud and Spring Security's StrictServerWebExchangeFirewall work together.

sven-tsi commented 3 weeks ago

Maybe our issue has the same cause: After update Spring Boot 3.3.4 to 3.3.5 the tokenRelay .filters(GatewayFilterSpec::tokenRelay).uri("lb://calc-service")) in out Spring Cloud gateway fails with:

java.lang.UnsupportedOperationException
    at org.springframework.http.ReadOnlyHttpHeaders.set(ReadOnlyHttpHeaders.java:110)
    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 ⇢ AuthorizationWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ ExceptionTranslationWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ LogoutWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ ServerRequestCacheWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ SecurityContextServerWebExchangeWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ LogoutPageGeneratingWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ LoginPageGeneratingWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ AuthenticationWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ OAuth2LoginAuthenticationWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ OAuth2AuthorizationRequestRedirectWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ ReactorContextWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ HttpHeaderWriterWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
    *__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
    *__checkpoint ⇢ HTTP GET "/cs/tenant" [ExceptionHandlingWebHandler]
Original Stack Trace:
        at org.springframework.http.ReadOnlyHttpHeaders.set(ReadOnlyHttpHeaders.java:110)
        at org.springframework.http.ReadOnlyHttpHeaders.set(ReadOnlyHttpHeaders.java:39)
        at org.springframework.http.HttpHeaders.set(HttpHeaders.java:1735)
        at org.springframework.http.HttpHeaders.setBearerAuth(HttpHeaders.java:830)
        at org.springframework.cloud.gateway.filter.factory.TokenRelayGatewayFilterFactory.lambda$withBearerAuth$5(TokenRelayGatewayFilterFactory.java:92)
        at org.springframework.http.server.reactive.DefaultServerHttpRequestBuilder.headers(DefaultServerHttpRequestBuilder.java:117)
        at org.springframework.cloud.gateway.filter.factory.TokenRelayGatewayFilterFactory.lambda$withBearerAuth$6(TokenRelayGatewayFilterFactory.java:92)
        at org.springframework.web.server.DefaultServerWebExchangeBuilder.request(DefaultServerWebExchangeBuilder.java:58)
        at org.springframework.cloud.gateway.filter.factory.TokenRelayGatewayFilterFactory.withBearerAuth(TokenRelayGatewayFilterFactory.java:92)
        at org.springframework.cloud.gateway.filter.factory.TokenRelayGatewayFilterFactory.lambda$apply$2(TokenRelayGatewayFilterFactory.java:65)

After rolled back to Spring Boot 3.3.4 everthying is working fine.

bclozel commented 3 weeks ago

@sven-tsi yes it is. See workaround here: https://github.com/spring-projects/spring-security/issues/15989#issuecomment-2442660753

ilgrosso commented 2 weeks ago

@rwinch any chance to backport this fix to Spring Framework 5.3.x? The same issue is occurring with Spring Security 5.8.15 (while 5.8.14 works fine)

bclozel commented 2 weeks ago

@ilgrosso Spring Framework 5.3.x is out of open source support so we won't be releasing new OSS versions. We have already cut several commercial releases.