From 828cac888915e93966662f82f37e61b0c0d147fd Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Wed, 1 Dec 2021 12:36:22 -0600 Subject: [PATCH] Fix case sensitive headers comparison Closes gh-10557 --- .../header/StaticServerHttpHeadersWriter.java | 13 +++++++++--- .../StaticServerHttpHeadersWriterTests.java | 21 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriter.java b/web/src/main/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriter.java index 1f636f5cd3..1e7d422a4b 100644 --- a/web/src/main/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriter.java @@ -17,7 +17,6 @@ package org.springframework.security.web.server.header; import java.util.Arrays; -import java.util.Collections; import reactor.core.publisher.Mono; @@ -41,8 +40,16 @@ public class StaticServerHttpHeadersWriter implements ServerHttpHeadersWriter { @Override public Mono writeHttpHeaders(ServerWebExchange exchange) { HttpHeaders headers = exchange.getResponse().getHeaders(); - boolean containsOneHeaderToAdd = Collections.disjoint(headers.keySet(), this.headersToAdd.keySet()); - if (containsOneHeaderToAdd) { + // Note: We need to ensure that the following algorithm compares headers + // case insensitively, which should be true of headers.containsKey(). + boolean containsNoHeadersToAdd = true; + for (String headerName : this.headersToAdd.keySet()) { + if (headers.containsKey(headerName)) { + containsNoHeadersToAdd = false; + break; + } + } + if (containsNoHeadersToAdd) { this.headersToAdd.forEach(headers::put); } return Mono.empty(); diff --git a/web/src/test/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriterTests.java b/web/src/test/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriterTests.java index 86809627fc..9b1edef195 100644 --- a/web/src/test/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/server/header/StaticServerHttpHeadersWriterTests.java @@ -16,11 +16,14 @@ package org.springframework.security.web.server.header; +import java.util.Locale; + import org.junit.Test; import org.springframework.http.HttpHeaders; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.util.LinkedMultiValueMap; import org.springframework.web.server.ServerWebExchange; import static org.assertj.core.api.Assertions.assertThat; @@ -56,6 +59,24 @@ public class StaticServerHttpHeadersWriterTests { .containsOnly(headerValue); } + // gh-10557 + @Test + public void writeHeadersWhenHeaderWrittenWithDifferentCaseThenDoesNotWriteHeaders() { + String headerName = HttpHeaders.CACHE_CONTROL.toLowerCase(Locale.ROOT); + String headerValue = "max-age=120"; + this.headers.set(headerName, headerValue); + // Note: This test inverts which collection uses case sensitive headers, + // due to the fact that gh-10557 reports NettyHeadersAdapter as the + // response headers implementation, which is not accessible here. + HttpHeaders caseSensitiveHeaders = new HttpHeaders(new LinkedMultiValueMap<>()); + caseSensitiveHeaders.set(HttpHeaders.CACHE_CONTROL, CacheControlServerHttpHeadersWriter.CACHE_CONTRTOL_VALUE); + caseSensitiveHeaders.set(HttpHeaders.PRAGMA, CacheControlServerHttpHeadersWriter.PRAGMA_VALUE); + caseSensitiveHeaders.set(HttpHeaders.EXPIRES, CacheControlServerHttpHeadersWriter.EXPIRES_VALUE); + this.writer = new StaticServerHttpHeadersWriter(caseSensitiveHeaders); + this.writer.writeHttpHeaders(this.exchange); + assertThat(this.headers.get(headerName)).containsOnly(headerValue); + } + @Test public void writeHeadersWhenMultiHeaderThenWritesAllHeaders() { this.writer = StaticServerHttpHeadersWriter.builder()