Browse Source

ExceptionTranslationFilter does not handle committed responses

Fixes: gh-5273
pull/5507/head
Rob Winch 8 years ago
parent
commit
32f5fb5eb2
  1. 4
      config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy
  2. 3
      web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java
  3. 52
      web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java

4
config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy

@ -131,6 +131,7 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { @@ -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 { @@ -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 { @@ -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 { @@ -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'

3
web/src/main/java/org/springframework/security/web/access/ExceptionTranslationFilter.java

@ -135,6 +135,9 @@ public class ExceptionTranslationFilter extends GenericFilterBean { @@ -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 {

52
web/src/test/java/org/springframework/security/web/access/ExceptionTranslationFilterTests.java

@ -15,6 +15,23 @@ @@ -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; @@ -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 { @@ -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 {

Loading…
Cancel
Save