From ecaa2c5b1c361e356fd8caf3ffd1040e39132da1 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 17 Jul 2018 22:13:33 -0500 Subject: [PATCH] Cache Control disabled for 304 Fixes: gh-5534 --- .../web/header/writers/CacheControlHeadersWriter.java | 3 ++- .../header/CacheControlServerHttpHeadersWriter.java | 4 ++++ .../writers/CacheControlHeadersWriterTests.java | 11 +++++++++++ .../CacheControlServerHttpHeadersWriterTests.java | 11 +++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/web/src/main/java/org/springframework/security/web/header/writers/CacheControlHeadersWriter.java b/web/src/main/java/org/springframework/security/web/header/writers/CacheControlHeadersWriter.java index 9355ae6b98..9500e38a7c 100644 --- a/web/src/main/java/org/springframework/security/web/header/writers/CacheControlHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/header/writers/CacheControlHeadersWriter.java @@ -22,6 +22,7 @@ import java.util.List; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.http.HttpStatus; import org.springframework.security.web.header.Header; import org.springframework.security.web.header.HeaderWriter; import org.springframework.util.ReflectionUtils; @@ -59,7 +60,7 @@ public final class CacheControlHeadersWriter implements HeaderWriter { @Override public void writeHeaders(HttpServletRequest request, HttpServletResponse response) { if (hasHeader(response, CACHE_CONTROL) || hasHeader(response, EXPIRES) - || hasHeader(response, PRAGMA)) { + || hasHeader(response, PRAGMA) || response.getStatus() == HttpStatus.NOT_MODIFIED.value()) { return; } this.delegate.writeHeaders(request, response); diff --git a/web/src/main/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriter.java b/web/src/main/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriter.java index ac6b094980..aae78795db 100644 --- a/web/src/main/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriter.java +++ b/web/src/main/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriter.java @@ -16,6 +16,7 @@ package org.springframework.security.web.server.header; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.web.server.ServerWebExchange; import reactor.core.publisher.Mono; @@ -61,6 +62,9 @@ public class CacheControlServerHttpHeadersWriter implements ServerHttpHeadersWri @Override public Mono writeHttpHeaders(ServerWebExchange exchange) { + if (exchange.getResponse().getStatusCode() == HttpStatus.NOT_MODIFIED) { + return Mono.empty(); + } return CACHE_HEADERS.writeHttpHeaders(exchange); } diff --git a/web/src/test/java/org/springframework/security/web/header/writers/CacheControlHeadersWriterTests.java b/web/src/test/java/org/springframework/security/web/header/writers/CacheControlHeadersWriterTests.java index a4e1b1bb42..f774d3b3f0 100644 --- a/web/src/test/java/org/springframework/security/web/header/writers/CacheControlHeadersWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/header/writers/CacheControlHeadersWriterTests.java @@ -23,6 +23,7 @@ import org.junit.runner.RunWith; import org.powermock.core.classloader.annotations.PrepareOnlyThisForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.http.HttpStatus; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.util.ReflectionUtils; @@ -124,4 +125,14 @@ public class CacheControlHeadersWriterTests { assertThat(this.response.getHeaderValue("Cache-Control")).isNull(); assertThat(this.response.getHeaderValue("Pragma")).isNull(); } + + @Test + // gh-5534 + public void writeHeadersDisabledIfNotModified() { + this.response.setStatus(HttpStatus.NOT_MODIFIED.value()); + + this.writer.writeHeaders(this.request, this.response); + + assertThat(this.response.getHeaderNames()).isEmpty(); + } } diff --git a/web/src/test/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriterTests.java b/web/src/test/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriterTests.java index 07248475da..57f1d3c572 100644 --- a/web/src/test/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/server/header/CacheControlServerHttpHeadersWriterTests.java @@ -19,6 +19,7 @@ import static org.assertj.core.api.Assertions.assertThat; import org.junit.Test; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.web.server.ServerWebExchange; @@ -83,4 +84,14 @@ public class CacheControlServerHttpHeadersWriterTests { assertThat(headers.get(HttpHeaders.EXPIRES)).containsOnly(expires); } + @Test + // gh-5534 + public void writeHeadersWhenNotModifiedThenNoCacheControlHeaders() { + exchange.getResponse().setStatusCode(HttpStatus.NOT_MODIFIED); + + writer.writeHttpHeaders(exchange); + + assertThat(headers).isEmpty(); + } + }