Fix ChannelProcessingFilter to skip decision if response already committed

In this commit, we quickly fix a bug in ChannelProcessingFilter#doFilter
where if filterInvocation.getResponse().isCommitted() is checked before
decide, it will not pass further into the filter, which is an error.
Perhaps in the future it would be better to extend the
ChannelDecisionManager interface directly, but it is deprecated and this
would be difficult and beyond the scope of this commit.

Closes: gh-6721

Signed-off-by: Andrey Litvitski <[email protected]>

Author: therepanic

web/src/main/java/org/springframework/security/web/access/channel/ChannelProcessingFilter.java

@@ -83,6 +83,7 @@
  * over HTTPS.
  *
  * @author Ben Alex
+ * @author Andrey Litvitski
  * @deprecated see {@link org.springframework.security.web.transport.HttpsRedirectFilter}
  */
 @Deprecated
@@ -125,10 +126,12 @@ public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain)
 		HttpServletResponse response = (HttpServletResponse) res;
 		FilterInvocation filterInvocation = new FilterInvocation(request, response, chain);
 		Collection<ConfigAttribute> attributes = this.securityMetadataSource.getAttributes(filterInvocation);
-		if (attributes != null) {
+		boolean committedBefore = filterInvocation.getResponse().isCommitted();
+		if (attributes != null && !committedBefore) {
 			this.logger.debug(LogMessage.format("Request: %s; ConfigAttributes: %s", filterInvocation, attributes));
 			this.channelDecisionManager.decide(filterInvocation, attributes);
-			if (filterInvocation.getResponse().isCommitted()) {
+			boolean committedAfter = filterInvocation.getResponse().isCommitted();
+			if (committedAfter) {
 				return;
 			}
 		}

web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java

@@ -32,12 +32,14 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.springframework.security.web.servlet.TestMockHttpServletRequests.get;
 
 /**
  * Tests {@link ChannelProcessingFilter}.
  *
  * @author Ben Alex
+ * @author Andrey Litvitski
  */
 public class ChannelProcessingFilterTests {
 
@@ -123,6 +125,21 @@ public void testGetterSetters() {
 		filter.afterPropertiesSet();
 	}
 
+	@Test
+	public void testDoFilterWhenResponseAlreadyCommittedBeforeFilter() throws Exception {
+		ChannelProcessingFilter filter = new ChannelProcessingFilter();
+		filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "MOCK"));
+		MockFilterInvocationDefinitionMap fids = new MockFilterInvocationDefinitionMap("/path", true, "MOCK");
+		filter.setSecurityMetadataSource(fids);
+		MockHttpServletRequest request = get("/path").build();
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		// this will set response.isCommitted() == true
+		response.flushBuffer();
+		FilterChain chain = mock(FilterChain.class);
+		filter.doFilter(request, response, chain);
+		verify(chain).doFilter(request, response);
+	}
+
 	private class MockChannelDecisionManager implements ChannelDecisionManager {
 
 		private String supportAttribute;