Skip to content

Commit 8a458eb

Browse files
borlafuRob Winch
authored andcommitted
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.
1 parent d2524ea commit 8a458eb

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

web/src/main/java/org/springframework/security/web/header/writers/frameoptions/XFrameOptionsHeaderWriter.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,16 +74,22 @@ public XFrameOptionsHeaderWriter(AllowFromStrategy allowFromStrategy) {
7474
this.allowFromStrategy = allowFromStrategy;
7575
}
7676

77+
/**
78+
* Writes the X-Frame-Options header value, overwritting any previous value.
79+
*
80+
* @param request the servlet request
81+
* @param response the servlet response
82+
*/
7783
public void writeHeaders(HttpServletRequest request, HttpServletResponse response) {
7884
if (XFrameOptionsMode.ALLOW_FROM.equals(frameOptionsMode)) {
7985
String allowFromValue = allowFromStrategy.getAllowFromValue(request);
8086
if (allowFromValue != null) {
81-
response.addHeader(XFRAME_OPTIONS_HEADER,
87+
response.setHeader(XFRAME_OPTIONS_HEADER,
8288
XFrameOptionsMode.ALLOW_FROM.getMode() + " " + allowFromValue);
8389
}
8490
}
8591
else {
86-
response.addHeader(XFRAME_OPTIONS_HEADER, frameOptionsMode.getMode());
92+
response.setHeader(XFRAME_OPTIONS_HEADER, frameOptionsMode.getMode());
8793
}
8894
}
8995

web/src/test/java/org/springframework/security/web/header/writers/frameoptions/FrameOptionsHeaderWriterTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,17 @@ public void writeHeadersSameOrigin() {
108108
assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER))
109109
.isEqualTo("SAMEORIGIN");
110110
}
111+
112+
@Test
113+
public void writeHeadersTwiceLastWins() {
114+
writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.SAMEORIGIN);
115+
writer.writeHeaders(request, response);
116+
117+
writer = new XFrameOptionsHeaderWriter(XFrameOptionsMode.DENY);
118+
writer.writeHeaders(request, response);
119+
120+
assertThat(response.getHeaderNames().size()).isEqualTo(1);
121+
assertThat(response.getHeader(XFrameOptionsHeaderWriter.XFRAME_OPTIONS_HEADER))
122+
.isEqualTo("DENY");
123+
}
111124
}

0 commit comments

Comments
 (0)