search

spring-projects / spring-framework

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

HttpHeaders#writableHttpHeaders not effective with read-only delegate #33795

Closed ilgrosso closed 2 weeks ago

ilgrosso commented 2 weeks ago

As described in https://github.com/spring-cloud/spring-cloud-gateway/issues/3570#issuecomment-2437407696 starting with Spring Boot 3.3.5, we are observing that HttpHeaders#writableHttpHeaders is failing to return a writable instance in case the given headers argument is an instance of Spring Security's org.springframework.security.web.server.firewall.StrictServerWebExchangeFirewall.StrictFirewallServerWebExchange.StrictFirewallHttpRequest.StrictFirewallHttpHeaders

This seems to be related to https://github.com/spring-projects/spring-security/commit/0e257b56ce35402558a260ffa6b368982f9a7934

I think it would be enough to cherry-pick https://github.com/spring-projects/spring-framework/commit/ef77b4064fe0cc6da3986059cbeac3f471a8fd92 to the branch 6.1.x

jhoeller commented 2 weeks ago

I suppose this duplicates #33789?

bclozel commented 2 weeks ago

Yes it is, closing as a duplicate. Please let us know if SNAPSHOTs don't cover this use case.

ilgrosso commented 2 weeks ago

@jhoeller @bclozel I have just tested 6.1.15-SNAPSHOT and I can confirm that https://github.com/spring-projects/spring-framework/issues/33789 is fixing what I report above, so I think it is correct to mark this issue as duplicate.

One question though: why making a different fix in https://github.com/spring-projects/spring-framework/commit/a06bbccf9ecccd41f1099e0462005478cc84974b instead of cherry picking https://github.com/spring-projects/spring-framework/commit/ef77b4064fe0cc6da3986059cbeac3f471a8fd92 ?

bclozel commented 2 weeks ago

https://github.com/spring-projects/spring-framework/commit/ef77b4064fe0cc6da3986059cbeac3f471a8fd92 is about case sensitive headers, whereas https://github.com/spring-projects/spring-framework/commit/a06bbccf9ecccd41f1099e0462005478cc84974b is about unwrapping read-only HttpHeaders instance. Rob accurately reported the problem in https://github.com/spring-projects/spring-framework/issues/33789 that reproduces the double-wrapping of HttpHeaders (once with read-only, and another time with security's headers) that was creating issues.