From 43700302c6a34924f727dc3594f074ca75298e24 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 27 Nov 2023 10:27:56 +0000 Subject: [PATCH] RequestMappingInfo defaults to PathPatternParser This changes ensures RequestMappingInfo uses PathPatternParser by default as all AbstractHandlerMapping implementations do as of 6.0. RequestMappingInfo instances are typically created internally and aligned with the RequestMappingHandlerMapping in terms of path mapping options. If a RequestMappingInfo is registered programmatically, the caller needs to also ensure they are aligned. However, if the two should be aligned by default. Closes gh-31662 --- .../mvc/method/RequestMappingInfo.java | 32 +++++++++++-------- ...RequestMappingInfoHandlerMappingTests.java | 29 ++++++++++++----- .../mvc/method/RequestMappingInfoTests.java | 28 ++++++++++------ .../method/annotation/CrossOriginTests.java | 7 ++-- 4 files changed, 61 insertions(+), 35 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java index 3371114693..4caeef3e71 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java @@ -705,27 +705,28 @@ public final class RequestMappingInfo implements RequestConditionNote: This property is mutually exclusive with * {@link #setPathMatcher(PathMatcher)}. - *

By default this is not enabled. + *

By default this is not set, but {@link RequestMappingInfo.Builder} + * defaults to using {@link PathPatternParser} unless + * {@link #setPathMatcher(PathMatcher)} is explicitly set. * @since 5.3 */ public void setPatternParser(@Nullable PathPatternParser patternParser) { @@ -936,7 +938,9 @@ public final class RequestMappingInfo implements RequestConditionBy default this is not set. + *

By default this is not set. You must set it explicitly if you want + * {@link PathMatcher} to be used, or otherwise {@link RequestMappingInfo} + * defaults to using {@link PathPatternParser}. */ public void setPathMatcher(@Nullable PathMatcher pathMatcher) { this.pathMatcher = pathMatcher; diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java index bca7f56f17..1d7cc8e64f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMappingTests.java @@ -38,6 +38,7 @@ import org.springframework.http.server.RequestPath; import org.springframework.http.server.observation.ServerRequestObservationContext; import org.springframework.lang.Nullable; import org.springframework.stereotype.Controller; +import org.springframework.util.AntPathMatcher; import org.springframework.util.MultiValueMap; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -258,10 +259,13 @@ class RequestMappingInfoHandlerMappingTests { @SuppressWarnings("unchecked") @PathPatternsParameterizedTest void handleMatchUriTemplateVariables(TestRequestMappingInfoHandlerMapping mapping) { - RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/{path2}").build(); + RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); + config.setPathMatcher(new AntPathMatcher()); + + RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/{path2}").options(config).build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); String lookupPath = new UrlPathHelper().getLookupPathForRequest(request); - mapping.handleMatch(key, lookupPath, request); + mapping.handleMatch(info, lookupPath, request); String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE; Map uriVariables = (Map) request.getAttribute(name); @@ -274,7 +278,10 @@ class RequestMappingInfoHandlerMappingTests { @SuppressWarnings("unchecked") @PathPatternsParameterizedTest // SPR-9098 void handleMatchUriTemplateVariablesDecode(TestRequestMappingInfoHandlerMapping mapping) { - RequestMappingInfo key = RequestMappingInfo.paths("/{group}/{identifier}").build(); + RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); + config.setPathMatcher(new AntPathMatcher()); + + RequestMappingInfo info = RequestMappingInfo.paths("/{group}/{identifier}").options(config).build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/group/a%2Fb"); UrlPathHelper pathHelper = new UrlPathHelper(); @@ -282,7 +289,7 @@ class RequestMappingInfoHandlerMappingTests { String lookupPath = pathHelper.getLookupPathForRequest(request); mapping.setUrlPathHelper(pathHelper); - mapping.handleMatch(key, lookupPath, request); + mapping.handleMatch(info, lookupPath, request); String name = HandlerMapping.URI_TEMPLATE_VARIABLES_ATTRIBUTE; Map uriVariables = (Map) request.getAttribute(name); @@ -294,20 +301,26 @@ class RequestMappingInfoHandlerMappingTests { @PathPatternsParameterizedTest void handleMatchBestMatchingPatternAttribute(TestRequestMappingInfoHandlerMapping mapping) { - RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/2", "/**").build(); + RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); + config.setPathMatcher(new AntPathMatcher()); + + RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/2", "/**").options(config).build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); - mapping.handleMatch(key, "/1/2", request); + mapping.handleMatch(info, "/1/2", request); assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)).isEqualTo("/{path1}/2"); } @PathPatternsParameterizedTest void handleMatchBestMatchingPatternAttributeInObservationContext(TestRequestMappingInfoHandlerMapping mapping) { - RequestMappingInfo key = RequestMappingInfo.paths("/{path1}/2", "/**").build(); + RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); + config.setPathMatcher(new AntPathMatcher()); + + RequestMappingInfo info = RequestMappingInfo.paths("/{path1}/2", "/**").options(config).build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/1/2"); ServerRequestObservationContext observationContext = new ServerRequestObservationContext(request, new MockHttpServletResponse()); request.setAttribute(ServerHttpObservationFilter.CURRENT_OBSERVATION_CONTEXT_ATTRIBUTE, observationContext); - mapping.handleMatch(key, "/1/2", request); + mapping.handleMatch(info, "/1/2", request); assertThat(request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE)).isEqualTo("/{path1}/2"); assertThat(observationContext.getPathPattern()).isEqualTo("/{path1}/2"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java index 1e2914959c..3192621b17 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoTests.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; +import org.springframework.util.AntPathMatcher; import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.servlet.handler.PathPatternsParameterizedTest; import org.springframework.web.servlet.handler.PathPatternsTestUtils; @@ -48,8 +49,8 @@ class RequestMappingInfoTests { @SuppressWarnings("unused") static Stream pathPatternsArguments() { RequestMappingInfo.BuilderConfiguration config = new RequestMappingInfo.BuilderConfiguration(); - config.setPatternParser(new PathPatternParser()); - return Stream.of(RequestMappingInfo.paths().options(config), RequestMappingInfo.paths()); + config.setPathMatcher(new AntPathMatcher()); + return Stream.of(RequestMappingInfo.paths(), RequestMappingInfo.paths().options(config)); } @@ -86,6 +87,13 @@ class RequestMappingInfoTests { assertThat(info.getCustomCondition()).isSameAs(result.getCustomCondition()); } + @Test // gh-31662 + void pathPatternByDefault() { + RequestMappingInfo info = RequestMappingInfo.paths().build(); + assertThat(info.getPathPatternsCondition()).isNotNull(); + assertThat(info.getPatternsCondition()).isNull(); + } + @PathPatternsParameterizedTest void matchPatternsCondition(RequestMappingInfo.Builder builder) { @@ -105,7 +113,7 @@ class RequestMappingInfoTests { @Test void matchParamsCondition() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true); request.setParameter("foo", "bar"); RequestMappingInfo info = RequestMappingInfo.paths("/foo").params("foo=bar").build(); @@ -121,7 +129,7 @@ class RequestMappingInfoTests { @Test void matchHeadersCondition() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true); request.addHeader("foo", "bar"); RequestMappingInfo info = RequestMappingInfo.paths("/foo").headers("foo=bar").build(); @@ -137,7 +145,7 @@ class RequestMappingInfoTests { @Test void matchConsumesCondition() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true); request.setContentType("text/plain"); RequestMappingInfo info = RequestMappingInfo.paths("/foo").consumes("text/plain").build(); @@ -153,7 +161,7 @@ class RequestMappingInfoTests { @Test void matchProducesCondition() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true); request.addHeader("Accept", "text/plain"); RequestMappingInfo info = RequestMappingInfo.paths("/foo").produces("text/plain").build(); @@ -169,7 +177,7 @@ class RequestMappingInfoTests { @Test void matchCustomCondition() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/foo", true); request.setParameter("foo", "bar"); RequestMappingInfo info = RequestMappingInfo.paths("/foo").params("foo=bar").build(); @@ -189,7 +197,7 @@ class RequestMappingInfoTests { RequestMappingInfo oneMethod = RequestMappingInfo.paths().methods(GET).build(); RequestMappingInfo oneMethodOneParam = RequestMappingInfo.paths().methods(GET).params("foo").build(); - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", true); Comparator comparator = (info, otherInfo) -> info.compareTo(otherInfo, request); List list = asList(noMethods, oneMethod, oneMethodOneParam); @@ -204,7 +212,7 @@ class RequestMappingInfoTests { @Test // SPR-14383 void compareToWithHttpHeadMapping() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("GET", "/", true); request.setMethod("HEAD"); request.addHeader("Accept", "application/json"); @@ -297,7 +305,7 @@ class RequestMappingInfoTests { @Test void preFlightRequest() { - MockHttpServletRequest request = PathPatternsTestUtils.initRequest("OPTIONS", "/foo", false); + MockHttpServletRequest request = PathPatternsTestUtils.initRequest("OPTIONS", "/foo", true); request.addHeader(HttpHeaders.ORIGIN, "https://domain.com"); request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "POST"); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java index 72dffa046f..872f501bfd 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -38,6 +38,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.server.RequestPath; import org.springframework.lang.Nullable; import org.springframework.stereotype.Controller; +import org.springframework.util.AntPathMatcher; import org.springframework.util.CollectionUtils; import org.springframework.web.bind.annotation.CrossOrigin; import org.springframework.web.bind.annotation.GetMapping; @@ -595,8 +596,8 @@ class CrossOriginTests { RequestMapping annotation = AnnotatedElementUtils.findMergedAnnotation(method, RequestMapping.class); if (annotation != null) { RequestMappingInfo.BuilderConfiguration options = new RequestMappingInfo.BuilderConfiguration(); - if (getPatternParser() != null) { - options.setPatternParser(getPatternParser()); + if (getPatternParser() == null) { + options.setPathMatcher(new AntPathMatcher()); } return RequestMappingInfo.paths(annotation.value()) .methods(annotation.method())