diff --git a/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilter.java b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilter.java index d7e2f17d263..ee1423659f0 100644 --- a/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilter.java +++ b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilter.java @@ -39,12 +39,29 @@ /** * Servlet {@link Filter} for recording {@link HttpExchange HTTP exchanges}. * + *

+ * This filter runs near the end of the filter chain (at {@code LOWEST_PRECEDENCE - 10}) + * so it captures enriched headers added by earlier filters. When used together with + * {@link HttpExchangesStartingFilter}, it coordinates recording to ensure that requests + * short-circuited by high-priority filters (such as Spring Security) are still captured + * by the starting filter. + * + *

+ * When {@link HttpExchangesStartingFilter} is present: + *

+ * * @author Dave Syer * @author Wallace Wadge * @author Andy Wilkinson * @author Venil Noronha * @author Madhura Bhave * @since 4.0.0 + * @see HttpExchangesStartingFilter */ public class HttpExchangesFilter extends OncePerRequestFilter implements Ordered { @@ -82,8 +99,10 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse filterChain.doFilter(request, response); return; } - RecordableServletHttpRequest sourceRequest = new RecordableServletHttpRequest(request); - HttpExchange.Started startedHttpExchange = HttpExchange.start(sourceRequest); + // Reuse the Started exchange created by HttpExchangesStartingFilter if present, + // otherwise create a new one (standalone use without + // HttpExchangesStartingFilter). + HttpExchange.Started startedExchange = getOrCreateStartedExchange(request); int status = HttpStatus.INTERNAL_SERVER_ERROR.value(); try { filterChain.doFilter(request, response); @@ -91,12 +110,29 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse } finally { RecordableServletHttpResponse sourceResponse = new RecordableServletHttpResponse(response, status); - HttpExchange finishedExchange = startedHttpExchange.finish(sourceResponse, request::getUserPrincipal, + HttpExchange finishedExchange = startedExchange.finish(sourceResponse, request::getUserPrincipal, () -> getSessionId(request), this.includes); this.repository.add(finishedExchange); + // Signal HttpExchangesStartingFilter not to record the exchange again. + request.setAttribute(HttpExchangesStartingFilter.ATTRIBUTE_FINISHED, Boolean.TRUE); } } + /** + * Returns the {@link HttpExchange.Started} stored by + * {@link HttpExchangesStartingFilter}, or creates a fresh one if the starting filter + * is not in the chain. + * @param request the source request + * @return the started exchange to use for recording + */ + private HttpExchange.Started getOrCreateStartedExchange(HttpServletRequest request) { + Object attribute = request.getAttribute(HttpExchangesStartingFilter.ATTRIBUTE_STARTED); + if (attribute instanceof HttpExchange.Started started) { + return started; + } + return HttpExchange.start(new RecordableServletHttpRequest(request)); + } + private boolean isRequestValid(HttpServletRequest request) { try { new URI(request.getRequestURL().toString()); @@ -107,6 +143,12 @@ private boolean isRequestValid(HttpServletRequest request) { } } + /** + * Return the session id for the given request, or {@code null} if the request does + * not have a session. + * @param request the source request + * @return the session id or {@code null} if there is no session + */ private @Nullable String getSessionId(HttpServletRequest request) { HttpSession session = request.getSession(false); return (session != null) ? session.getId() : null; diff --git a/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilter.java b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilter.java new file mode 100644 index 00000000000..da4171abb50 --- /dev/null +++ b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilter.java @@ -0,0 +1,144 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.servlet.actuate.web.exchanges; + +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Set; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; + +import org.springframework.boot.actuate.web.exchanges.HttpExchange; +import org.springframework.boot.actuate.web.exchanges.HttpExchangeRepository; +import org.springframework.boot.actuate.web.exchanges.Include; +import org.springframework.core.Ordered; +import org.springframework.http.HttpStatus; +import org.springframework.web.filter.OncePerRequestFilter; + +/** + * A high-priority Servlet {@link jakarta.servlet.Filter} that starts recording an + * {@link HttpExchange} at the very beginning of the filter chain. It coordinates with + * {@link HttpExchangesFilter}, which runs later with low priority and finishes recording + * for requests that reach it. For requests that are short-circuited by high-priority + * filters (such as Spring Security rejecting unauthenticated requests), this filter acts + * as a fallback and records the exchange in the {@code finally} block after the chain + * returns. + * + *

+ * This filter works together with {@link HttpExchangesFilter}: + *

+ * + * @author Spring Boot Team + * @since 4.0.0 + * @see HttpExchangesFilter + */ +public class HttpExchangesStartingFilter extends OncePerRequestFilter implements Ordered { + + /** + * Name of the request attribute holding the {@link HttpExchange.Started} created by + * this filter. + */ + static final String ATTRIBUTE_STARTED = HttpExchangesStartingFilter.class.getName() + ".started"; + + /** + * Name of the request attribute set to {@code Boolean.TRUE} by + * {@link HttpExchangesFilter} once it has finished and recorded the exchange, + * preventing this filter from recording it a second time. + */ + static final String ATTRIBUTE_FINISHED = HttpExchangesStartingFilter.class.getName() + ".finished"; + + // Run as early as possible so we capture even security-rejected requests + private int order = Ordered.HIGHEST_PRECEDENCE + 1; + + private final HttpExchangeRepository repository; + + private final Set includes; + + /** + * Create a new {@link HttpExchangesStartingFilter} instance. + * @param repository the repository used to record events + * @param includes the include options + */ + public HttpExchangesStartingFilter(HttpExchangeRepository repository, Set includes) { + this.repository = repository; + this.includes = includes; + } + + @Override + public int getOrder() { + return this.order; + } + + public void setOrder(int order) { + this.order = order; + } + + @Override + protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) + throws ServletException, IOException { + if (!isRequestValid(request)) { + filterChain.doFilter(request, response); + return; + } + RecordableServletHttpRequest sourceRequest = new RecordableServletHttpRequest(request); + HttpExchange.Started startedExchange = HttpExchange.start(sourceRequest); + request.setAttribute(ATTRIBUTE_STARTED, startedExchange); + int status = HttpStatus.INTERNAL_SERVER_ERROR.value(); + try { + filterChain.doFilter(request, response); + status = response.getStatus(); + } + finally { + // Only record the exchange here if HttpExchangesFilter hasn't already done + // so. + // HttpExchangesFilter sets ATTRIBUTE_FINISHED when it records the exchange. + if (!Boolean.TRUE.equals(request.getAttribute(ATTRIBUTE_FINISHED))) { + RecordableServletHttpResponse sourceResponse = new RecordableServletHttpResponse(response, status); + HttpExchange finishedExchange = startedExchange.finish(sourceResponse, request::getUserPrincipal, + () -> { + HttpSession session = request.getSession(false); + return (session != null) ? session.getId() : null; + }, this.includes); + this.repository.add(finishedExchange); + } + } + } + + private boolean isRequestValid(HttpServletRequest request) { + try { + new URI(request.getRequestURL().toString()); + return true; + } + catch (URISyntaxException ex) { + return false; + } + } + +} diff --git a/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfiguration.java b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfiguration.java index fb1fea0662b..fdd6d807f25 100644 --- a/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfiguration.java +++ b/module/spring-boot-servlet/src/main/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfiguration.java @@ -28,12 +28,23 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.servlet.actuate.web.exchanges.HttpExchangesFilter; +import org.springframework.boot.servlet.actuate.web.exchanges.HttpExchangesStartingFilter; import org.springframework.context.annotation.Bean; /** * {@link EnableAutoConfiguration Auto-configuration} to record {@link HttpExchange HTTP * exchanges}. * + *

+ * Registers two filters that work together to ensure all requests — including those + * rejected by high-priority filters such as Spring Security — are captured: + *

+ * * @author Dave Syer * @since 4.0.0 */ @@ -50,4 +61,11 @@ HttpExchangesFilter httpExchangesFilter(HttpExchangeRepository repository, HttpE return new HttpExchangesFilter(repository, properties.getRecording().getInclude()); } + @Bean + @ConditionalOnMissingBean + HttpExchangesStartingFilter httpExchangesStartingFilter(HttpExchangeRepository repository, + HttpExchangesProperties properties) { + return new HttpExchangesStartingFilter(repository, properties.getRecording().getInclude()); + } + } diff --git a/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilterTests.java b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilterTests.java index 58758a2fd24..36242e10cec 100644 --- a/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilterTests.java +++ b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesFilterTests.java @@ -26,6 +26,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.Test; +import org.springframework.boot.actuate.web.exchanges.HttpExchange; import org.springframework.boot.actuate.web.exchanges.HttpExchange.Session; import org.springframework.boot.actuate.web.exchanges.InMemoryHttpExchangeRepository; import org.springframework.boot.actuate.web.exchanges.Include; @@ -121,4 +122,43 @@ void filterRejectsInvalidRequests() throws ServletException, IOException { assertThat(this.repository.findAll()).isEmpty(); } + @Test + void filterReusesStartedExchangeFromStartingFilter() throws ServletException, IOException { + // Simulate HttpExchangesStartingFilter having set the ATTRIBUTE_STARTED + MockHttpServletRequest request = new MockHttpServletRequest(); + HttpExchange.Started preStarted = HttpExchange.start(new RecordableServletHttpRequest(request)); + request.setAttribute(HttpExchangesStartingFilter.ATTRIBUTE_STARTED, preStarted); + + this.filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + + // Exchange is recorded exactly once + assertThat(this.repository.findAll()).hasSize(1); + } + + @Test + void filterSetsFinishedAttributeAfterRecording() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest(); + this.filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + + // ATTRIBUTE_FINISHED must be set so HttpExchangesStartingFilter won't + // double-record + assertThat(request.getAttribute(HttpExchangesStartingFilter.ATTRIBUTE_FINISHED)).isEqualTo(Boolean.TRUE); + } + + @Test + void twoFiltersCombinedRecordExchangeOnce() throws ServletException, IOException { + HttpExchangesStartingFilter startingFilter = new HttpExchangesStartingFilter(this.repository, + EnumSet.allOf(Include.class)); + + MockHttpServletRequest request = new MockHttpServletRequest(); + MockHttpServletResponse response = new MockHttpServletResponse(); + + // Execute the starting filter; its chain delegate calls the finishing filter. + startingFilter.doFilter(request, response, + (req, resp) -> this.filter.doFilter(req, resp, new MockFilterChain())); + + // Only one exchange should be recorded despite two filters running. + assertThat(this.repository.findAll()).hasSize(1); + } + } diff --git a/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilterTests.java b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilterTests.java new file mode 100644 index 00000000000..fcfd6c760c7 --- /dev/null +++ b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/actuate/web/exchanges/HttpExchangesStartingFilterTests.java @@ -0,0 +1,151 @@ +/* + * Copyright 2012-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.servlet.actuate.web.exchanges; + +import java.io.IOException; +import java.security.Principal; +import java.util.EnumSet; + +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServlet; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.Test; + +import org.springframework.boot.actuate.web.exchanges.HttpExchange; +import org.springframework.boot.actuate.web.exchanges.InMemoryHttpExchangeRepository; +import org.springframework.boot.actuate.web.exchanges.Include; +import org.springframework.mock.web.MockFilterChain; +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.assertThatIOException; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Tests for {@link HttpExchangesStartingFilter}. + * + * @author Spring Boot Team + */ +class HttpExchangesStartingFilterTests { + + private final InMemoryHttpExchangeRepository repository = new InMemoryHttpExchangeRepository(); + + private final HttpExchangesStartingFilter filter = new HttpExchangesStartingFilter(this.repository, + EnumSet.allOf(Include.class)); + + @Test + void filterRecordsExchangeWhenChainCompletesNormally() throws ServletException, IOException { + this.filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), new MockFilterChain()); + assertThat(this.repository.findAll()).hasSize(1); + } + + @Test + void filterSetsStartedAttributeOnRequest() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest(); + this.filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain(new HttpServlet() { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + // attribute must be set before the chain runs + assertThat(req.getAttribute(HttpExchangesStartingFilter.ATTRIBUTE_STARTED)) + .isInstanceOf(HttpExchange.Started.class); + } + })); + } + + @Test + void filterDoesNotRecordWhenFinishedAttributeIsSet() throws ServletException, IOException { + // Simulate HttpExchangesFilter having already recorded (sets ATTRIBUTE_FINISHED) + this.filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), + new MockFilterChain(new HttpServlet() { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + req.setAttribute(HttpExchangesStartingFilter.ATTRIBUTE_FINISHED, Boolean.TRUE); + } + })); + // Starting filter must NOT add a second record + assertThat(this.repository.findAll()).isEmpty(); + } + + @Test + void filterRecordsWhenChainTerminatesEarly() throws ServletException, IOException { + // Simulate Spring Security short-circuiting: chain does not call the next filter, + // ATTRIBUTE_FINISHED is never set, so the starting filter records the exchange. + MockHttpServletResponse response = new MockHttpServletResponse(); + response.setStatus(401); + this.filter.doFilter(new MockHttpServletRequest(), response, new MockFilterChain(new HttpServlet() { + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + resp.setStatus(401); + // Intentionally does NOT call filterChain.doFilter() — + // simulates security filter short-circuiting. + } + })); + assertThat(this.repository.findAll()).hasSize(1); + assertThat(this.repository.findAll().get(0).getResponse().getStatus()).isEqualTo(401); + } + + @Test + void statusIsAssumedToBe500WhenChainFails() { + assertThatIOException().isThrownBy(() -> this.filter.doFilter(new MockHttpServletRequest(), + new MockHttpServletResponse(), new MockFilterChain(new HttpServlet() { + + @Override + protected void service(HttpServletRequest req, HttpServletResponse resp) + throws ServletException, IOException { + throw new IOException(); + } + + }))) + .satisfies((ex) -> { + assertThat(this.repository.findAll()).hasSize(1); + assertThat(this.repository.findAll().get(0).getResponse().getStatus()).isEqualTo(500); + }); + } + + @Test + void filterRecordsPrincipal() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest(); + Principal principal = mock(Principal.class); + given(principal.getName()).willReturn("alice"); + request.setUserPrincipal(principal); + this.filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + assertThat(this.repository.findAll()).hasSize(1); + HttpExchange.Principal recordedPrincipal = this.repository.findAll().get(0).getPrincipal(); + assertThat(recordedPrincipal).isNotNull(); + assertThat(recordedPrincipal.getName()).isEqualTo("alice"); + } + + @Test + void filterRejectsInvalidRequests() throws ServletException, IOException { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setServerName(""); + this.filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain()); + assertThat(this.repository.findAll()).isEmpty(); + } + + @Test + void filterOrderIsHighestPrecedencePlusOne() { + assertThat(this.filter.getOrder()).isEqualTo(Integer.MIN_VALUE + 1); + } + +} diff --git a/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfigurationTests.java b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfigurationTests.java index 5e310d8b5dd..95db8c33eda 100644 --- a/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfigurationTests.java +++ b/module/spring-boot-servlet/src/test/java/org/springframework/boot/servlet/autoconfigure/actuate/web/exchanges/ServletHttpExchangesAutoConfigurationTests.java @@ -26,6 +26,7 @@ import org.springframework.boot.actuate.web.exchanges.Include; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.servlet.actuate.web.exchanges.HttpExchangesFilter; +import org.springframework.boot.servlet.actuate.web.exchanges.HttpExchangesStartingFilter; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import static org.assertj.core.api.Assertions.assertThat; @@ -44,22 +45,30 @@ class ServletHttpExchangesAutoConfigurationTests { void whenRecordingIsDisabledThenFilterIsNotCreated() { this.contextRunner.withBean(InMemoryHttpExchangeRepository.class) .withPropertyValues("management.httpexchanges.recording.enabled=false") - .run((context) -> assertThat(context).doesNotHaveBean(HttpExchangesFilter.class)); + .run((context) -> { + assertThat(context).doesNotHaveBean(HttpExchangesFilter.class); + assertThat(context).doesNotHaveBean(HttpExchangesStartingFilter.class); + }); } @Test void whenNoRepositoryIsDefinedThenFilterIsNotCreated() { - this.contextRunner.run((context) -> assertThat(context).doesNotHaveBean(HttpExchangesFilter.class)); + this.contextRunner.run((context) -> { + assertThat(context).doesNotHaveBean(HttpExchangesFilter.class); + assertThat(context).doesNotHaveBean(HttpExchangesStartingFilter.class); + }); } @Test - void filterIsCreated() { - this.contextRunner.withBean(InMemoryHttpExchangeRepository.class) - .run((context) -> assertThat(context).hasSingleBean(HttpExchangesFilter.class)); + void bothFiltersAreCreated() { + this.contextRunner.withBean(InMemoryHttpExchangeRepository.class).run((context) -> { + assertThat(context).hasSingleBean(HttpExchangesFilter.class); + assertThat(context).hasSingleBean(HttpExchangesStartingFilter.class); + }); } @Test - void usesUserProvidedWebFilter() { + void usesUserProvidedHttpExchangesFilter() { InMemoryHttpExchangeRepository repository = new InMemoryHttpExchangeRepository(); this.contextRunner.withBean(InMemoryHttpExchangeRepository.class, () -> repository) .withBean(CustomHttpExchangesFilter.class, @@ -67,6 +76,23 @@ void usesUserProvidedWebFilter() { .run((context) -> { assertThat(context).hasSingleBean(HttpExchangesFilter.class); assertThat(context.getBean(HttpExchangesFilter.class)).isInstanceOf(CustomHttpExchangesFilter.class); + // Starting filter is still auto-created + assertThat(context).hasSingleBean(HttpExchangesStartingFilter.class); + }); + } + + @Test + void usesUserProvidedHttpExchangesStartingFilter() { + InMemoryHttpExchangeRepository repository = new InMemoryHttpExchangeRepository(); + this.contextRunner.withBean(InMemoryHttpExchangeRepository.class, () -> repository) + .withBean(CustomHttpExchangesStartingFilter.class, + () -> new CustomHttpExchangesStartingFilter(repository, EnumSet.allOf(Include.class))) + .run((context) -> { + assertThat(context).hasSingleBean(HttpExchangesStartingFilter.class); + assertThat(context.getBean(HttpExchangesStartingFilter.class)) + .isInstanceOf(CustomHttpExchangesStartingFilter.class); + // Finishing filter is still auto-created + assertThat(context).hasSingleBean(HttpExchangesFilter.class); }); } @@ -78,4 +104,12 @@ private CustomHttpExchangesFilter(HttpExchangeRepository repository, Set includes) { + super(repository, includes); + } + + } + }