From 9c1e033d145aad44bb6b2146030e69ea345812e6 Mon Sep 17 00:00:00 2001 From: borlafu Date: Tue, 7 Mar 2017 21:04:13 +0100 Subject: [PATCH 1/3] Avoid multiple X-Frame-Options headers XFrameOptionsHeaderWriter should not *add*, but *set* the X-Frame-Options header. According to https://tools.ietf.org/html/rfc7034#section-2.1, having multiple values for the header is disallowed: "There are three different values for the header field. These values are mutually exclusive; that is, the header field MUST be set to exactly one of the three values." With this change, only the latest XFrameOptionsHeaderWriter will remain. --- .../frameoptions/XFrameOptionsHeaderWriter.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/header/writers/frameoptions/XFrameOptionsHeaderWriter.java b/web/src/main/java/org/springframework/security/web/header/writers/frameoptions/XFrameOptionsHeaderWriter.java index 1c9c32b1ca7..75fbb164326 100644 --- a/web/src/main/java/org/springframework/security/web/header/writers/frameoptions/XFrameOptionsHeaderWriter.java +++ b/web/src/main/java/org/springframework/security/web/header/writers/frameoptions/XFrameOptionsHeaderWriter.java @@ -74,16 +74,22 @@ public XFrameOptionsHeaderWriter(AllowFromStrategy allowFromStrategy) { this.allowFromStrategy = allowFromStrategy; } + /** + * Writes the X-Frame-Options header value, overwritting any previous value. + * + * @param request the servlet request + * @param response the servlet response + */ public void writeHeaders(HttpServletRequest request, HttpServletResponse response) { if (XFrameOptionsMode.ALLOW_FROM.equals(frameOptionsMode)) { String allowFromValue = allowFromStrategy.getAllowFromValue(request); if (allowFromValue != null) { - response.addHeader(XFRAME_OPTIONS_HEADER, + response.setHeader(XFRAME_OPTIONS_HEADER, XFrameOptionsMode.ALLOW_FROM.getMode() + " " + allowFromValue); } } else { - response.addHeader(XFRAME_OPTIONS_HEADER, frameOptionsMode.getMode()); + response.setHeader(XFRAME_OPTIONS_HEADER, frameOptionsMode.getMode()); } } From 0eaaff1f138a24406a3edff0c43b623247fe5fe7 Mon Sep 17 00:00:00 2001 From: borlafu Date: Tue, 7 Mar 2017 21:17:23 +0100 Subject: [PATCH 2/3] Test for multiple X-Frame-Options headers --- .../frameoptions/FrameOptionsHeaderWriterTests.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java b/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java index 69b2a61131c..f92ae888f62 100644 --- a/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java @@ -108,4 +108,17 @@ public void writeHeadersSameOrigin() { assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER)) .isEqualTo("SAMEORIGIN"); } + + @Test + public void writeHeadersTwiceLastWins() { + writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.SAMEORIGIN); + writer.writeHeaders(request, response); + + writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.DENY); + writer.writeHeaders(request, response); + + assertThat(response.getHeaderNames().size()).isEqualTo(1); + assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER)) + .isEqualTo("DENY"); + } } From d59b5eb1a0799867a11417fcea723a2fbf0b8180 Mon Sep 17 00:00:00 2001 From: borlafu Date: Tue, 7 Mar 2017 21:53:56 +0100 Subject: [PATCH 3/3] removing trailing spaces --- .../writers/frameoptions/FrameOptionsHeaderWriterTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java b/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java index f92ae888f62..4a9fe66a5bd 100644 --- a/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java +++ b/web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java @@ -113,7 +113,7 @@ public void writeHeadersSameOrigin() { public void writeHeadersTwiceLastWins() { writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.SAMEORIGIN); writer.writeHeaders(request, response); - + writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.DENY); writer.writeHeaders(request, response);