From e033e347b4f38f5b909a9f292b9e3e362b346323 Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Wed, 10 May 2023 16:10:32 -0600 Subject: [PATCH] Remove Redundant Close Closes gh-12787 --- .../web/ObservationFilterChainDecorator.java | 1 - .../ObservationFilterChainDecoratorTests.java | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java b/web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java index 0ff75ea8d7..939ece90b9 100644 --- a/web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java +++ b/web/src/main/java/org/springframework/security/web/ObservationFilterChainDecorator.java @@ -324,7 +324,6 @@ public final class ObservationFilterChainDecorator implements FilterChainProxy.F private void error(Throwable error) { if (this.state.get() == 1) { - this.scope.close(); this.scope.getCurrentObservation().error(error); } } diff --git a/web/src/test/java/org/springframework/security/web/ObservationFilterChainDecoratorTests.java b/web/src/test/java/org/springframework/security/web/ObservationFilterChainDecoratorTests.java index 47fe1c1b5b..d10e83bc31 100644 --- a/web/src/test/java/org/springframework/security/web/ObservationFilterChainDecoratorTests.java +++ b/web/src/test/java/org/springframework/security/web/ObservationFilterChainDecoratorTests.java @@ -30,8 +30,10 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -87,4 +89,21 @@ public class ObservationFilterChainDecoratorTests { assertThat(events.get(1).getName()).isEqualTo(filter.getClass().getSimpleName() + ".after"); } + // gh-12787 + @Test + void decorateFiltersWhenErrorsThenClosesObservationOnlyOnce() throws Exception { + ObservationHandler handler = mock(ObservationHandler.class); + given(handler.supportsContext(any())).willReturn(true); + ObservationRegistry registry = ObservationRegistry.create(); + registry.observationConfig().observationHandler(handler); + ObservationFilterChainDecorator decorator = new ObservationFilterChainDecorator(registry); + FilterChain chain = mock(FilterChain.class); + Filter filter = mock(Filter.class); + willThrow(IllegalArgumentException.class).given(filter).doFilter(any(), any(), any()); + FilterChain decorated = decorator.decorate(chain, List.of(filter)); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( + () -> decorated.doFilter(new MockHttpServletRequest("GET", "/"), new MockHttpServletResponse())); + verify(handler).onScopeClosed(any()); + } + }