Turn off useSuffixPatternMatching by default

[rupert-madden-abbott]

When useSuffixPatternMatching is set to true, then

a method mapped to "/users" also matches to "/users.*"

The default value is true. Please can this be changed to false.

Defaults shouldn't be surprising
It is not obvious to developers that this will occur. They intend to setup a static mapping of "/users" and find that this mapping works. They have to explicit visit "/users.blah" to discover that they have unintentionally mapped more than they intended.

There are lots of examples of this causing surprise with developers being confused by the cause and purpose of this function:

• https://stackoverflow.com/questions/9688065/spring-mvc-application-filtering-html-in-url-is-this-a-security-issue
• https://stackoverflow.com/questions/30610607/how-to-change-spring-request-mapping-to-disallow-url-pattern-with-suffix
• https://stackoverflow.com/questions/30307678/why-does-requestmapping-spring-annotation-in-controller-capture-more-that-i-wan
• https://stackoverflow.com/questions/22845672/requestmapping-in-spring-with-weird-patterns

The behaviour is clearly documented but that isn't good enough to stop the surprise because it isn't clear from the application code itself. All a developer sees is that "/users" is mapped and there is nothing explicit to make them realise something else is going on.

Defaults should encourage best practice
As documented, this default is enabling a practice that used to be widespread but has fallen out of favour due to the problems it causes. Developers should be using the Accept header and not file extensions for content negotiation.

It would be better to allow developers to opt-in to this feature, rather than having the best practice be an opt-in.

Defaults should be secure
antMatchers still exists in Spring Security. Whilst everybody should be using mvcMatchers, it's still really easy to open up a security hole by having this switched on.

It is also much more risky than the optional trailing slash feature which developers are far more likely to expect and anticipate since such a feature is common to many web frameworks. Developers are very unlikely to add a file extension to their URL if they haven't mapped one and thus can still easily miss this vulnerability if they have failed to use mvcMatchers

[rstoyanchev]

These are all valid arguments from our own reference docs, and in a greenfield situation like WebFlux, those options don't even exist. In Spring MVC however, the surprise will also apply to all those who already rely on the current behavior. That is the main sticking point.

[rstoyanchev]

Team decision:

In 5.2, deprecate config options (see #24179) for path matching and for content negotiation via path extensions in favor of content negotiation by Accept header, or query parameter if necessary, and path matching with @RequestMapping(produces="...").

In 5.3, enable suffix pattern matching via registered extensions only:

Setting	Current Default	New Default
useSuffixPatternMatch	true	true
useRegisteredSuffixPatternMatch	false	true
ContentNegotiationManager	Path extensions, Accept header	Path extensions, Accept header

For extra context, the relevant Spring Boot web properties and their settings:

• spring.mvc.pathmatch.use-suffix-pattern = false
• spring.mvc.pathmatch.use-registered-suffix-pattern = false
• spring.mvc.contentnegotiation.favor-path-extension = false
• spring.mvc.contentnegotiation.favor-parameter=false

[mikesmithson]

@rstoyanchev - is this issue something a first-time contributor can submit a PR for? I saw that it was self-assigned back in December and wasn't sure if it was being actively worked on. I have a PR that I can submit if this issue is still available for outside contribution.

[rstoyanchev]

In light of #24642 it's become clear for the deprecation to work properly, we must change defaults in 5.3 to turn off use of path extension completely. In other words:

Setting	Current Default	New Default
useSuffixPatternMatch	true	false
useRegisteredSuffixPatternMatch	false	false
ContentNegotiationManager	Accept header	Accept header

This will match the defaults in Boot and will allow applications to stop using those deprecated methods before they are eventually removed.

[rstoyanchev]

Re-opening in order to change the default in the XML config as well.

[rwinch]

For users who need to temporarily revert to the previous behavior you can adjust the path matching configuration:

@Configuration
@EnableWebMvc
public class WebConfig implements WebMvcConfigurer {

    @Override
    public void configurePathMatch(PathMatchConfigurer configurer) {
        configurer
            // ...
            .setUseRegisteredSuffixPatternMatch(true);
    }
}

<mvc:annotation-driven>
    <mvc:path-matching suffix-pattern="true"/>
</mvc:annotation-driven>

[mikesmithson]

@rstoyanchev - Is this still open for contribution or do you want to finish this one up?