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.

[iPasalski]

what is the deprecation period for PathMatchConfigurer#setUseTrailingSlashMatch ?
when setUseTrailingSlashMatch(true) it would work also for actuator api (i.e. actuator/env vs actuator/env/)?

[divStar]

Just to add to my initial response: in our project we have probably around 200 endpoints and 3 frontends, which access these endpoints in various manners (we hadn't used unified Feign Clients back when we started out).

This change means, that we have to adjust our endpoints in so they explicitly define the endpoint with a trailing slash and one without. We have to adjust all integration tests for all endpoints and we even should adjust all end-to-end tests in multiple platforms. This is easily 10+ business days of work for just this useless change. I'd resort to using setUseTrailingSlashMatch(true), but for some reason this method is marked as deprecated.

Isn't it possible to allow people, who did not pay 100% attention to the trailing slashes, opt-out of this "more security" aspect? We have implemente a few other means to strengthen our API security, so opting out of this thing would be okay.

[aka5h2017]

Thanks Spring team! I spent an entire day on figuring out why the API stopped working.

You guys do great work with the framework; but this boggles my mind, how can such a decision be made?

[dpozinen]

We have literally hundreds of clients across dozens of environments, this will stall the adoption of SB3 for months and months and will undoubtedly cause issues even with all the due diligence in the world. Absolutely crazy change.
I whole heartedly support #31366 and especially the logging of such cases

[KeatsPeeks]

We have literally hundreds of clients across dozens of environments, this will stall the adoption of SB3 for months and months and will undoubtedly cause issues even with all the due diligence in the world. Absolutely crazy change. I whole heartedly support #31366 and especially the logging of such cases

Spring upgrades have always been a pain, even between minor versions (remember "allow-bean-definition-overriding" ?).
You can use the deprecated option, it works (until it won't).

[GabrielBB]

This was a horrible update. Should have been an optional property in application.yml with default false

[ndrscodes]

I really support different solutions proposed here, such as #31366. As of now, in nearly every project, we're having to fall back to @SuppressWarnings("deprecated").

[dingshuangxi888]

This issue can be resolved by adding the following Filter:

@Component
public class ServletRequestWrapperFilter implements Filter {

    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
            throws IOException, ServletException {
        HttpServletRequestWrapper requestWrapper = new HttpServletRequestWrapper((HttpServletRequest) request) {
            @Override
            public String getRequestURI() {
                String requestURI = super.getRequestURI();
                if (StringUtils.endsWith(requestURI, "/")) {
                    return StringUtils.removeEnd(requestURI, "/");
                }
                return requestURI;
            }
        };
        chain.doFilter(requestWrapper, response);
    }
}

[rstoyanchev]

There is a now a concrete proposal for a similar Filter under #31366.

[soc]

@MartinHaeusler I'm sorry you went through this. Maybe we can improve the documentation to avoid the same issue for other developers. Can you share at which documentation you were looking at while working on this?

Did this ever happen?

I expected some kind of information of this on https://github.com/spring-projects/spring-framework/wiki/What%27s-New-in-Spring-Framework-6.x but neither trailing, slash, nor 404 brings up anything.

[bclozel]

@soc thanks for the reminder, I've just added something here: https://github.com/spring-projects/spring-framework/wiki/Upgrading-to-Spring-Framework-6.x#web-applications-1

[soc]

@bclozel Thanks, I think that will be helpful to many people!

[ndrscodes]

Though i really appreciate the addition to the wiki, i still think that some kind of default filter should be provided by the framework. Even if it's not included in the filterchain by default, it would still make life a lot easier for this very common use-case.

[bclozel]

@ndrscodes it's already implemented - please provide feedback directly on the issue when you'll test it: #31366

[Myrmod]

This issue can be resolved by adding the following Filter:

@Component
public class ServletRequestWrapperFilter implements Filter {

    @Override
    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
            throws IOException, ServletException {
        HttpServletRequestWrapper requestWrapper = new HttpServletRequestWrapper((HttpServletRequest) request) {
            @Override
            public String getRequestURI() {
                String requestURI = super.getRequestURI();
                if (StringUtils.endsWith(requestURI, "/")) {
                    return StringUtils.removeEnd(requestURI, "/");
                }
                return requestURI;
            }
        };
        chain.doFilter(requestWrapper, response);
    }
}

Another variation since it took me a while to figure out how to apply the previous example:

package com.example.myApp.Filters;

import java.io.IOException;

import org.springframework.stereotype.Component;
import org.springframework.util.StringUtils;

import jakarta.servlet.Filter;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletRequestWrapper;

@Component
public class TrailingSlashFilter implements Filter {

  @Override
  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
      throws IOException, ServletException {

    HttpServletRequestWrapper requestWrappper = new HttpServletRequestWrapper((HttpServletRequest) request) {
      @Override
      public String getRequestURI() {
        String requestURI = super.getRequestURI();

        return StringUtils.trimTrailingCharacter(requestURI, "/".toCharArray()[0]);
      }
    };

    chain.doFilter(requestWrappper, response);
  }

}

[rstoyanchev]

@Myrmod we support this now. Please, see #31366.