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 33711146934a..4caeef3e71d7 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 Builder options(BuilderConfiguration options) { @SuppressWarnings("deprecation") public RequestMappingInfo build() { - PathPatternsRequestCondition pathPatterns = null; - PatternsRequestCondition patterns = null; + PathPatternsRequestCondition pathPatternsCondition = null; + PatternsRequestCondition patternsCondition = null; - if (this.options.patternParser != null) { - pathPatterns = (ObjectUtils.isEmpty(this.paths) ? - EMPTY_PATH_PATTERNS : - new PathPatternsRequestCondition(this.options.patternParser, this.paths)); - } - else { - patterns = (ObjectUtils.isEmpty(this.paths) ? + if (this.options.getPathMatcher() != null) { + patternsCondition = (ObjectUtils.isEmpty(this.paths) ? EMPTY_PATTERNS : new PatternsRequestCondition( this.paths, null, this.options.getPathMatcher(), this.options.useSuffixPatternMatch(), this.options.useTrailingSlashMatch(), this.options.getFileExtensions())); } + else { + PathPatternParser parser = (this.options.getPatternParser() != null ? + this.options.getPatternParser() : new PathPatternParser()); + pathPatternsCondition = (ObjectUtils.isEmpty(this.paths) ? + EMPTY_PATH_PATTERNS : new PathPatternsRequestCondition(parser, this.paths)); + } ContentNegotiationManager manager = this.options.getContentNegotiationManager(); return new RequestMappingInfo( - this.mappingName, pathPatterns, patterns, + this.mappingName, pathPatternsCondition, patternsCondition, ObjectUtils.isEmpty(this.methods) ? EMPTY_REQUEST_METHODS : new RequestMethodsRequestCondition(this.methods), ObjectUtils.isEmpty(this.params) ? @@ -737,8 +738,7 @@ public RequestMappingInfo build() { ObjectUtils.isEmpty(this.produces) && !this.hasAccept ? EMPTY_PRODUCES : new ProducesRequestCondition(this.produces, this.headers, manager), this.customCondition != null ? - new RequestConditionHolder(this.customCondition) : EMPTY_CUSTOM, - this.options); + new RequestConditionHolder(this.customCondition) : EMPTY_CUSTOM, this.options); } } @@ -894,7 +894,9 @@ public static class BuilderConfiguration { * {@link AbstractHandlerMapping#setPatternParser(PathPatternParser)}. *

Note: 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 UrlPathHelper getUrlPathHelper() { /** * Set a custom PathMatcher to use for the PatternsRequestCondition. - *

By 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 bca7f56f1741..1d7cc8e64f38 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.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 @@ void getHandlerMappedInterceptors() throws Exception { @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 @@ void handleMatchUriTemplateVariables(TestRequestMappingInfoHandlerMapping mappin @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 @@ void handleMatchUriTemplateVariablesDecode(TestRequestMappingInfoHandlerMapping 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 @@ void handleMatchUriTemplateVariablesDecode(TestRequestMappingInfoHandlerMapping @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 1e2914959cef..3192621b1797 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.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 @@ void createEmpty(RequestMappingInfo.Builder infoBuilder) { 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 @@ void matchPatternsCondition(RequestMappingInfo.Builder builder) { @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 @@ void matchParamsCondition() { @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 @@ void matchHeadersCondition() { @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 @@ void matchConsumesCondition() { @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 @@ void matchProducesCondition() { @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 @@ void compareToWithImplicitVsExplicitHttpMethodDeclaration() { 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 @@ void compareToWithImplicitVsExplicitHttpMethodDeclaration() { @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 @@ void equalsMethod(RequestMappingInfo.Builder infoBuilder) { @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 72dffa046ff0..872f501bfd9b 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.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 @@ protected RequestMappingInfo getMappingForMethod(Method method, Class handler 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())