Skip to content

Turn off useSuffixPatternMatching by default #24576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -48,6 +48,7 @@
* @author Rossen Stoyanchev
* @author Rob Winch
* @author Sebastien Deleuze
* @author Mike Smithson
*/
public class StandaloneMockMvcBuilderTests {

Expand Down Expand Up @@ -82,7 +83,7 @@ public void suffixPatternMatch() throws Exception {

request = new MockHttpServletRequest("GET", "/persons.xml");
chain = hm.getHandler(request);
assertThat(chain).isNull();
assertThat(chain).isNotNull();
}

@Test // SPR-12553
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -160,8 +160,7 @@ public class PathPattern implements Comparable<PathPattern> {
if (elem instanceof CaptureTheRestPathElement || elem instanceof WildcardTheRestPathElement) {
this.catchAll = true;
}
if (elem instanceof SeparatorPathElement && elem.next != null &&
elem.next instanceof WildcardPathElement && elem.next.next == null) {
if (elem instanceof SeparatorPathElement && elem.next instanceof WildcardPathElement && elem.next.next == null) {
this.endsWithSeparatorWildcard = true;
}
elem = elem.next;
Expand Down Expand Up @@ -325,7 +324,7 @@ public PathContainer extractPathWithinPattern(PathContainer path) {
}
}

PathContainer resultPath = null;
PathContainer resultPath;
if (multipleAdjacentSeparators) {
// Need to rebuild the path without the duplicate adjacent separators
StringBuilder buf = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -280,11 +280,7 @@ public int compareTo(RequestMappingInfo other, ServerWebExchange exchange) {
if (result != 0) {
return result;
}
result = this.customConditionHolder.compareTo(other.customConditionHolder, exchange);
if (result != 0) {
return result;
}
return 0;
return this.customConditionHolder.compareTo(other.customConditionHolder, exchange);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@
public class PathMatchConfigurer {

@Nullable
private Boolean suffixPatternMatch;
private Boolean suffixPatternMatch = Boolean.TRUE;

@Nullable
private Boolean registeredSuffixPatternMatch;
private Boolean registeredSuffixPatternMatch = Boolean.TRUE;

@Nullable
private Boolean trailingSlashMatch;
Expand Down Expand Up @@ -83,7 +83,7 @@ public PathMatchConfigurer setUseSuffixPatternMatch(Boolean suffixPatternMatch)
* {@link WebMvcConfigurer#configureContentNegotiation configure content
* negotiation}. This is generally recommended to reduce ambiguity and to
* avoid issues such as when a "." appears in the path for other reasons.
* <p>By default this is set to "false".
* <p>By default this is set to "true".
* @see WebMvcConfigurer#configureContentNegotiation
* @deprecated as of 5.2.4. See class-level note in
* {@link RequestMappingHandlerMapping} on the deprecation of path extension
Expand Down Expand Up @@ -159,7 +159,8 @@ public Boolean isUseSuffixPatternMatch() {
}

/**
* Whether to use registered suffixes for pattern matching.
* Whether to use registered suffixes for pattern matching. By default, this
* is set to 'true'
* @deprecated as of 5.2.4. See class-level note in
* {@link RequestMappingHandlerMapping} on the deprecation of path extension
* config options.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public PatternsRequestCondition(String[] patterns, @Nullable UrlPathHelper urlPa
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
* @param urlPathHelper for determining the lookup path of a request
* @param pathMatcher for path matching with patterns
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
* @param useSuffixPatternMatch whether to enable matching by suffix (".*"), defaults to "true"
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
* @deprecated as of 5.2.4. See class-level note in
* {@link org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerMapping}
Expand All @@ -105,7 +105,7 @@ public PatternsRequestCondition(String[] patterns, @Nullable UrlPathHelper urlPa
* @param patterns the URL patterns to use; if 0, the condition will match to every request.
* @param urlPathHelper a {@link UrlPathHelper} for determining the lookup path for a request
* @param pathMatcher a {@link PathMatcher} for pattern path matching
* @param useSuffixPatternMatch whether to enable matching by suffix (".*")
* @param useSuffixPatternMatch whether to enable matching by suffix (".*"), defaults to "true"
* @param useTrailingSlashMatch whether to match irrespective of a trailing slash
* @param fileExtensions a list of file extensions to consider for path matching
* @deprecated as of 5.2.4. See class-level note in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,15 @@
* @author Arjen Poutsma
* @author Rossen Stoyanchev
* @author Sam Brannen
* @author Mike Smithson
* @since 3.1
*/
public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMapping
implements MatchableHandlerMapping, EmbeddedValueResolverAware {

private boolean useSuffixPatternMatch = true;

private boolean useRegisteredSuffixPatternMatch = false;
private boolean useRegisteredSuffixPatternMatch = true;

private boolean useTrailingSlashMatch = true;

Expand Down Expand Up @@ -113,7 +114,7 @@ public void setUseSuffixPatternMatch(boolean useSuffixPatternMatch) {
* explicitly registered with the {@link ContentNegotiationManager}. This
* is generally recommended to reduce ambiguity and to avoid issues such as
* when a "." appears in the path for other reasons.
* <p>By default this is set to "false".
* <p>By default this is set to "true".
* @deprecated as of 5.2.4. See class level comment about deprecation of
* path extension config options note also that in 5.3 the default for this
* property will switch from {@code false} to {@code true}.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand All @@ -22,7 +22,6 @@
import java.lang.annotation.Target;
import java.lang.reflect.Method;
import java.security.Principal;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Map;
Expand Down Expand Up @@ -59,6 +58,7 @@
*
* @author Rossen Stoyanchev
* @author Sam Brannen
* @author Mike Smithson
*/
public class RequestMappingHandlerMappingTests {

Expand All @@ -73,7 +73,7 @@ public class RequestMappingHandlerMappingTests {
@Test
public void useRegisteredSuffixPatternMatch() {
assertThat(this.handlerMapping.useSuffixPatternMatch()).isTrue();
assertThat(this.handlerMapping.useRegisteredSuffixPatternMatch()).isFalse();
assertThat(this.handlerMapping.useRegisteredSuffixPatternMatch()).isTrue();

Map<String, MediaType> fileExtensions = Collections.singletonMap("json", MediaType.APPLICATION_JSON);
PathExtensionContentNegotiationStrategy strategy = new PathExtensionContentNegotiationStrategy(fileExtensions);
Expand All @@ -85,7 +85,7 @@ public void useRegisteredSuffixPatternMatch() {

assertThat(this.handlerMapping.useSuffixPatternMatch()).isTrue();
assertThat(this.handlerMapping.useRegisteredSuffixPatternMatch()).isTrue();
assertThat(this.handlerMapping.getFileExtensions()).isEqualTo(Arrays.asList("json"));
assertThat(this.handlerMapping.getFileExtensions()).isEqualTo(Collections.singletonList("json"));
}

@Test
Expand Down Expand Up @@ -159,6 +159,7 @@ public void pathPrefix() throws NoSuchMethodException {
@Test // gh-23907
public void pathPrefixPreservesPathMatchingSettings() throws NoSuchMethodException {
this.handlerMapping.setUseSuffixPatternMatch(false);
this.handlerMapping.setUseRegisteredSuffixPatternMatch(false);
this.handlerMapping.setPathPrefixes(Collections.singletonMap("/api", HandlerTypePredicate.forAnyHandlerType()));
this.handlerMapping.afterPropertiesSet();

Expand All @@ -175,7 +176,7 @@ public void pathPrefixPreservesPathMatchingSettings() throws NoSuchMethodExcepti
}

@Test
public void resolveRequestMappingViaComposedAnnotation() throws Exception {
public void resolveRequestMappingViaComposedAnnotation() {
RequestMappingInfo info = assertComposedAnnotationMapping("postJson", "/postJson", RequestMethod.POST);

assertThat(info.getConsumesCondition().getConsumableMediaTypes().iterator().next().toString())
Expand All @@ -185,7 +186,7 @@ public void resolveRequestMappingViaComposedAnnotation() throws Exception {
}

@Test // SPR-14988
public void getMappingOverridesConsumesFromTypeLevelAnnotation() throws Exception {
public void getMappingOverridesConsumesFromTypeLevelAnnotation() {
RequestMappingInfo requestMappingInfo = assertComposedAnnotationMapping(RequestMethod.POST);

ConsumesRequestCondition condition = requestMappingInfo.getConsumesCondition();
Expand All @@ -206,39 +207,39 @@ public void consumesWithOptionalRequestBody() {
}

@Test
public void getMapping() throws Exception {
public void getMapping() {
assertComposedAnnotationMapping(RequestMethod.GET);
}

@Test
public void postMapping() throws Exception {
public void postMapping() {
assertComposedAnnotationMapping(RequestMethod.POST);
}

@Test
public void putMapping() throws Exception {
public void putMapping() {
assertComposedAnnotationMapping(RequestMethod.PUT);
}

@Test
public void deleteMapping() throws Exception {
public void deleteMapping() {
assertComposedAnnotationMapping(RequestMethod.DELETE);
}

@Test
public void patchMapping() throws Exception {
public void patchMapping() {
assertComposedAnnotationMapping(RequestMethod.PATCH);
}

private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) throws Exception {
private RequestMappingInfo assertComposedAnnotationMapping(RequestMethod requestMethod) {
String methodName = requestMethod.name().toLowerCase();
String path = "/" + methodName;

return assertComposedAnnotationMapping(methodName, path, requestMethod);
}

private RequestMappingInfo assertComposedAnnotationMapping(String methodName, String path,
RequestMethod requestMethod) throws Exception {
RequestMethod requestMethod) {

Class<?> clazz = ComposedAnnotationController.class;
Method method = ClassUtils.getMethod(clazz, methodName, (Class<?>[]) null);
Expand Down