From 7bde3ecc212fc4d32e1f3d49de5ce3c14079fe84 Mon Sep 17 00:00:00 2001 From: Andrey Litvitski Date: Thu, 7 Aug 2025 15:50:42 +0300 Subject: [PATCH] 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 --- .../access/channel/ChannelProcessingFilter.java | 7 +++++-- .../channel/ChannelProcessingFilterTests.java | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/access/channel/ChannelProcessingFilter.java b/web/src/main/java/org/springframework/security/web/access/channel/ChannelProcessingFilter.java index b14ae3bd474..e4cf52a7913 100644 --- a/web/src/main/java/org/springframework/security/web/access/channel/ChannelProcessingFilter.java +++ b/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 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; } } diff --git a/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java b/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java index ad3f3afa664..23c2b7d416a 100644 --- a/web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java +++ b/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;