diff --git a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy index 212956cd0f..c0f3d1fe09 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy @@ -131,6 +131,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { when: 'user cannot access otheruser' request = new MockHttpServletRequest(method:'GET', servletPath : '/user/otheruser/abc') login(request, 'user', 'password') + response = new MockHttpServletResponse() chain.reset() springSecurityFilterChain.doFilter(request,response,chain) then: 'The response is OK' @@ -138,6 +139,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { when: 'user can access case insensitive URL' request = new MockHttpServletRequest(method:'GET', servletPath : '/USER/user/abc') login(request, 'user', 'password') + response = new MockHttpServletResponse() chain.reset() springSecurityFilterChain.doFilter(request,response,chain) then: 'The response is OK' @@ -164,6 +166,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { when: 'user cannot access otheruser' request = new MockHttpServletRequest(method:'GET', servletPath : '/user/otheruser/abc') login(request, 'user', 'password') + response = new MockHttpServletResponse() chain.reset() springSecurityFilterChain.doFilter(request,response,chain) then: 'The response is OK' @@ -171,6 +174,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { when: 'user can access case insensitive URL' request = new MockHttpServletRequest(method:'GET', servletPath : '/USER/user/abc') login(request, 'user', 'password') + response = new MockHttpServletResponse() chain.reset() springSecurityFilterChain.doFilter(request,response,chain) then: 'The response is OK' diff --git a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java index 0fb87f73d9..5b1010d02c 100644 --- a/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java +++ b/web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java @@ -135,6 +135,9 @@ public class ExceptionTranslationFilter extends GenericFilterBean { } if (ase != null) { + if (response.isCommitted()) { + throw new ServletException("Unable to handle the Spring Security Exception because the response is already committed.", ex); + } handleSpringSecurityException(request, response, chain, ase); } else { diff --git a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java b/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java index 5602c71a2c..14d779f292 100644 --- a/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java +++ b/web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java @@ -15,6 +15,23 @@ */ package org.springframework.security.web.access; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assertions.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyZeroInteractions; + +import java.io.IOException; +import java.util.Locale; + +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -36,20 +53,6 @@ import org.springframework.security.web.WebAttributes; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; import org.springframework.security.web.savedrequest.SavedRequest; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; -import java.io.IOException; -import java.util.Locale; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; -import static org.mockito.Matchers.any; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; - /** * Tests {@link ExceptionTranslationFilter}. * @@ -302,7 +305,26 @@ public class ExceptionTranslationFilterTests { } } - private final AuthenticationEntryPoint mockEntryPoint = new AuthenticationEntryPoint() { + @Test + public void doFilterWhenResponseCommittedThenRethrowsException() throws Exception { + this.mockEntryPoint = mock(AuthenticationEntryPoint.class); + FilterChain chain = (request, response) -> { + HttpServletResponse httpResponse = (HttpServletResponse) response; + httpResponse.sendError(HttpServletResponse.SC_BAD_REQUEST); + throw new AccessDeniedException("Denied"); + }; + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + ExceptionTranslationFilter filter = new ExceptionTranslationFilter(mockEntryPoint); + + assertThatThrownBy(() -> filter.doFilter(request, response, chain)) + .isInstanceOf(ServletException.class) + .hasCauseInstanceOf(AccessDeniedException.class); + + verifyZeroInteractions(mockEntryPoint); + } + + private AuthenticationEntryPoint mockEntryPoint = new AuthenticationEntryPoint() { public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException {