Deprecate trailing slash match and change default value from true to false

[vpavic]

Whether to match to URLs irrespective of the presence of a trailing slash. If enabled a method mapped to "/users" also matches to "/users/".
The default value is true.

Even though this behavior has been long present in Spring, it introduces ambiguity that can (combined with some other choices) easily have consequences in shape of security vulnerabilities. Consider this example:

@SpringBootApplication
@RestController
public class SampleApplication {

    public static void main(String[] args) {
        SpringApplication.run(SampleApplication.class, args);
    }

    @GetMapping("/resources")
    String resources() {
        return "Hello from /resources";
    }

    @GetMapping("/resources/{id}")
    String resourceById(@PathVariable Long id) {
        return "Hello from /resources/" + id;
    }

    @Bean
    SecurityFilterChain filterChain(HttpSecurity httpSecurity) throws Exception {
        return httpSecurity
                .authorizeHttpRequests(requests -> {
                    requests.antMatchers("/resources").hasRole("admin");
                    requests.antMatchers("/resources/**").hasRole("user");
                    requests.anyRequest().denyAll();
                })
                .httpBasic(Customizer.withDefaults())
                .build();
    }

}

spring.security.user.password=password
spring.security.user.roles=user

Default user (with role user) will get 403 attempting to GET /resources but can avoid protection by issuing GET /resources/, which wouldn't be possible with trailing slash matching disabled.

Let me note that I'm aware that using mvcMatchers instead of antMatchers would have prevented this issue but that doesn't change the fact that there are many configurations out there relying on antMatchers and that sometimes antMatchers are simply more suitable for other reasons.

Also, I personally see little real benefit of having trailing slash matching enabled because:

• if application renders server-side generated views navigation is either way established using hyperlinks
• if application exposes APIs then even more so it is expected that requests are aligned with the API docs

For places where it's really needed for application to respond to both requests, I'd argue that it's either way better solution to configure redirects instead of application wide ambiguous request mappings.

Please consider making this change for 6.0.

[rstoyanchev]

I think most users would would expect that a trailing slash doesn't make a difference. From that perspective, the default is not unreasonable, and more than just HATEOAS style navigation or API guidance, even just manually typing a URL somewhere.

If we change the default, many would look to change it back, so overall it's hard to avoid the need to align Spring Security with with Spring MVC through mvcMatchers. One could argue that it's a safer default but this applies only when used with Spring Security. It's not unsafe by itself.

For configuring redirects, do you mean externally, like in a proxy? That would solve the problem at a different level, before Spring Security and Spring MVC, so I would be more interested in that direction but we can only make it a recommendation, and we can still make such a recommendation independent of this.

[vpavic]

From that perspective, the default is not unreasonable, and more than just HATEOAS style navigation or API guidance, even just manually typing a URL somewhere.

I can see manually typing a URL as a real argument only in case of web apps (not APIs) and even there it's only potentially useful for a few select URLs that are considered to be entry points into the application and are likely to be directly entered by the users.

One could argue that it's a safer default but this applies only when used with Spring Security. It's not unsafe by itself.

This is the part I strongly disagree with - what Spring Security does is nothing special nor unique. You can end up with the same kind of risks with other Java filter based security frameworks (for example, Apache Shiro) or by applying security (authorization) in an external component that sits in front of your Spring application. After all, on a high level, conceptually these all take the same approach.

For configuring redirects, do you mean externally, like in a proxy?

Either externally in a proxy or using something like Tuckey UrlRewriteFilter or even simply using ViewControllerRegistry::addRedirectViewController to add redirects where needed.

What I would like to see in Spring are the defaults that are not ambiguous and are therefore less prone to abuse. When I see a handler annotated with @GetMapping("/api/resources") that it really maps to only what I see, unless I opt into any additional (ambiguous) behavior explicitly. This change together with #23915 would achieve that.

[rstoyanchev]

I'm not necessarily disagreeing. I'm merely making the point that by itself, trailing slash is not unsafe. It's only when combined with a proxy or filter that also interprets URLs, where it becomes a problem, and the problem more generally is that there may be differences, which could go both ways.

That said, this is also the reason, I've come to see over time that URL paths should be left alone as is as much as possible, avoiding normalization as much as possible. It's the only way for frameworks to minimize differences and align naturally. This is also why PathPatternParser was designed to not decode the full path in order to match to mappings, unlike AntPathMatcher, doesn't even support suffix patterns, etc.

In any case, the only way to really make a significant difference here I think is to deprecate the trailingSlash option entirely and encourage an alternative solution, like a proxy or filter to redirect. That enforces being more precise about where you want to do that exactly, and it can be done ahead of security decisions. Otherwise the possibility for a mismatch between web and security frameworks remains. It's just too easy to flip the default back and not think about it again.

This is something that we'll discuss as a possibility for 6.0.

[vpavic]

In any case, the only way to really make a significant difference here I think is to deprecate the trailingSlash option entirely and encourage an alternative solution, like a proxy or filter to redirect.

I like that even better. Thanks for the feedback.

[rstoyanchev]

Team decision: we'll go ahead with this. It aligns with various other path matching changes we've made in 5.2 and 5.3 such suffix pattern matching, path segment trimming, and path decoding, to make path matching more transparent and explicit.

[wilkinsona]

Will the default be changed to false as part of this deprecation?

[rstoyanchev]

Yes, I'm thinking that we might as well and that we'll have to, since otherwise you'd need to set it in order to stop using it. We had the same issue with suffix pattern matching.

[rstoyanchev]

The trailing slash option is now deprecated and set to false in all applicable places. The change applies mainly to annotated controllers, since SimpleUrlHandlerMapping, it turns out, was already set to false by default. Nevertheless, it's now deprecated throughout and to be removed eventually.

[bclozel]

I've just found this while browing the code @rstoyanchev :

spring-framework/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java

Line 85 in 50240bb

private boolean useTrailingSlashMatch = true;

Should this value be changed as well?

[rstoyanchev]

Looks like it, yes.