Skip to content

Commit 8a475e3

Browse files
jzheauxrwinch
authored andcommitted
Write Security Headers Before Servlet Include
HeaderWriterFilter wraps request dispatcher so it can write security headers before the include occurs. Fixes: gh-5499
1 parent ccc4e1c commit 8a475e3

File tree

2 files changed

+71
-4
lines changed

2 files changed

+71
-4
lines changed

web/src/main/java/org/springframework/security/web/header/HeaderWriterFilter.java

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2012 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,8 +19,12 @@
1919
import java.util.List;
2020

2121
import javax.servlet.FilterChain;
22+
import javax.servlet.RequestDispatcher;
2223
import javax.servlet.ServletException;
24+
import javax.servlet.ServletRequest;
25+
import javax.servlet.ServletResponse;
2326
import javax.servlet.http.HttpServletRequest;
27+
import javax.servlet.http.HttpServletRequestWrapper;
2428
import javax.servlet.http.HttpServletResponse;
2529

2630
import org.springframework.security.web.util.OnCommittedResponseWrapper;
@@ -33,6 +37,7 @@
3337
* and X-Content-Type-Options.
3438
*
3539
* @author Marten Deinum
40+
* @author Josh Cummings
3641
* @since 3.2
3742
*
3843
*/
@@ -62,8 +67,11 @@ protected void doFilterInternal(HttpServletRequest request,
6267

6368
HeaderWriterResponse headerWriterResponse = new HeaderWriterResponse(request,
6469
response, this.headerWriters);
70+
HeaderWriterRequest headerWriterRequest = new HeaderWriterRequest(request,
71+
headerWriterResponse);
72+
6573
try {
66-
filterChain.doFilter(request, headerWriterResponse);
74+
filterChain.doFilter(headerWriterRequest, headerWriterResponse);
6775
}
6876
finally {
6977
headerWriterResponse.writeHeaders();
@@ -106,4 +114,39 @@ private HttpServletResponse getHttpResponse() {
106114
return (HttpServletResponse) getResponse();
107115
}
108116
}
117+
118+
static class HeaderWriterRequest extends HttpServletRequestWrapper {
119+
private final HeaderWriterResponse response;
120+
121+
HeaderWriterRequest(HttpServletRequest request, HeaderWriterResponse response) {
122+
super(request);
123+
this.response = response;
124+
}
125+
126+
@Override
127+
public RequestDispatcher getRequestDispatcher(String path) {
128+
return new HeaderWriterRequestDispatcher(super.getRequestDispatcher(path), this.response);
129+
}
130+
}
131+
132+
static class HeaderWriterRequestDispatcher implements RequestDispatcher {
133+
private final RequestDispatcher delegate;
134+
private final HeaderWriterResponse response;
135+
136+
HeaderWriterRequestDispatcher(RequestDispatcher delegate, HeaderWriterResponse response) {
137+
this.delegate = delegate;
138+
this.response = response;
139+
}
140+
141+
@Override
142+
public void forward(ServletRequest request, ServletResponse response) throws ServletException, IOException {
143+
this.delegate.forward(request, response);
144+
}
145+
146+
@Override
147+
public void include(ServletRequest request, ServletResponse response) throws ServletException, IOException {
148+
this.response.onResponseCommitted();
149+
this.delegate.include(request, response);
150+
}
151+
}
109152
}

web/src/test/java/org/springframework/security/web/header/HeaderWriterFilterTests.java

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2013 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
1818
import java.io.IOException;
1919
import java.util.ArrayList;
2020
import java.util.Arrays;
21+
import java.util.Collections;
2122
import java.util.List;
2223

2324
import javax.servlet.FilterChain;
@@ -84,7 +85,9 @@ public void additionalHeadersShouldBeAddedToTheResponse() throws Exception {
8485

8586
verify(this.writer1).writeHeaders(request, response);
8687
verify(this.writer2).writeHeaders(request, response);
87-
assertThat(filterChain.getRequest()).isEqualTo(request); // verify the filterChain
88+
HeaderWriterFilter.HeaderWriterRequest wrappedRequest = (HeaderWriterFilter.HeaderWriterRequest)
89+
filterChain.getRequest();
90+
assertThat(wrappedRequest.getRequest()).isEqualTo(request); // verify the filterChain
8891
// continued
8992
}
9093

@@ -112,4 +115,25 @@ public void doFilter(ServletRequest request, ServletResponse response)
112115

113116
verifyNoMoreInteractions(this.writer1);
114117
}
118+
119+
// gh-5499
120+
@Test
121+
public void doFilterWhenRequestContainsIncludeThenHeadersStillWritten() throws Exception {
122+
HeaderWriterFilter filter = new HeaderWriterFilter(
123+
Collections.singletonList(this.writer1));
124+
125+
MockHttpServletRequest mockRequest = new MockHttpServletRequest();
126+
MockHttpServletResponse mockResponse = new MockHttpServletResponse();
127+
128+
filter.doFilter(mockRequest, mockResponse, (request, response) -> {
129+
verifyZeroInteractions(HeaderWriterFilterTests.this.writer1);
130+
131+
request.getRequestDispatcher("/").include(request, response);
132+
133+
verify(HeaderWriterFilterTests.this.writer1).writeHeaders(
134+
any(HttpServletRequest.class), any(HttpServletResponse.class));
135+
});
136+
137+
verifyNoMoreInteractions(this.writer1);
138+
}
115139
}

0 commit comments

Comments
 (0)