From 8a458eb9e1cf0349fc4884cd718972c933948e60 Mon Sep 17 00:00:00 2001 From: borlafu Date: Tue, 7 Mar 2017 21:04:13 +0100 Subject: [PATCH] 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 ++++++++-- .../frameoptions/FrameOptionsHeaderWriterTests.java | 13 +++++++++++++ 2 files changed, 21 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 1c9c32b1ca..75fbb16432 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 final class XFrameOptionsHeaderWriter implements HeaderWriter { 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()); } } 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 69b2a61131..4a9fe66a5b 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 class FrameOptionsHeaderWriterTests { 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"); + } }