StaticServerHttpHeadersWriter should work with case-insensitive header names

[mat-mik]

Affects: 5.3.13

---

I'm using Spring Cloud Gateway with Spring Security and the issue is connected with default Security Headers.

When downstream response with a lowercase key header for Cache-Control, e.g. cache-control: max-age=120 then Spring thinks Cache Headers are not present and overwrites them with Cache-Control: no-cache, no-store, max-age=0, must-revalidate which is incorrect behavior and violates RFC 2616.

Expected behavior: Compliance with RFC 2616, so Header names are case-insensitive, so no headers overwrite here.

I analyzed the source code, here my notes:
org.springframework.security.web.server.header.CacheControlServerHttpHeadersWriter calls org.springframework.security.web.server.header.StaticServerHttpHeadersWriter calls Collections.disjoint() and passes org.springframework.http.client.reactive.NettyHeadersAdapter$HeaderNames.

StaticServerHttpHeadersWriter using Collections.disjoint() assumes that passed collections are case-insensitive, which is not true in case of NettyHeadersAdapter$HeaderNames.

Here's a test to reproduce it: https://github.com/mat-mik/gateway-headers/blob/425944a7fcfa52b1e555d81e554d1901e1e08cdd/src/test/java/com/example/gatewayheaders/GatewayHeadersApplicationTests.java#L43

---

Looking at the current design I believe that issue belongs to Spring Framework (because of NettyHeadersAdapter$HeaderNames), but I'm not sure if, in the long run, the StaticServerHttpHeadersWriter (so Spring Security) shouldn't take care of case-insensitiveness (but that's just an invitation for discussion)

[JintaoXIAO]

it is an issue by the mockerserver you used.

[mat-mik]

it is an issue by the mockerserver you used.

Dear @JintaoXIAO, I found the problem in a production system and wrote a test using mockserver to reproduce it, so mockserver acts here as a Stub only. Of course, in the production system, as a quick workaround, I changed cache-control header name to Cache-Control in the downstream system, but that's a workaround only. I think, Spring should react correctly to both cache-control and Cache-Control (and probably for CaChE-cOnTrOl as well :)).

[mat-mik]

JettyHeadersAdapter has a similar issue.

I think it's quite an important bug as HTTP2 uses lower-case header names.

[bclozel]

Hi @mat-mik

I don't think we can handle this problem at the Spring Framework level.

I've looked into our adapters implementations (and the ones in Netty and Jetty), and they're all case insensitive by default - by that I mean that the add/set/get/... methods all work with case insensitive keys. But when asked for the keys contained in the Map, they do return the actual keys as they were entered.

We can verify that with an additional unit test in org.springframework.http.server.reactive.HeadersAdaptersTests:

	@ParameterizedHeadersTest
	void shouldSupportCaseInsensitiveHeaders(String displayName, MultiValueMap<String, String> headers) {
		headers.add("Test-Header", "first");
		assertThat(headers.keySet()).hasSize(1);
		assertThat(headers.containsKey("test-header")).isTrue();
		assertThat(headers.getFirst("test-header")).isEqualTo("first");
	}

I think that the problem is that org.springframework.security.web.server.header.StaticServerHttpHeadersWriter uses Collections.disjoint() with the keySet. If this was iterating over the headers to add and using containsKey(String key), this wouldn't replace the value for cache-control.

Another way to think about it - what would be the expected header names keySet() returned by this collection? Should names be all forced as lowercased or capitalized? Or should they just look like the way they were put in the first place?

Could you raise this issue against Spring Security and link back here for future reference?

[rstoyanchev]

@mat-mik hold off on re-creating maybe. We can move it instead I think..