Disable by default suffix pattern matching in Spring MVC

[philwebb]

See this twitter exchange. We should confirm the existing behavior and consider it again for 2.0.

[philwebb]

/cc @rstoyanchev @rwinch for opinions on this. Is changing from the MVC default a good idea?

[bclozel]

TL;DR: this issue is about flipping the default for favorPathExtension to false in Spring Boot MVC auto-configuration.
We could at least add a configuration properties so that developers can change that easily.

[rstoyanchev]

We don't like the default in Spring MVC to begin with. This is the current advice we give but we can't change it without causing at least as much pain.

Note that there are two aspects to this issue. One is suffix pattern matching (i.e. "/foo" also implies "/foo.*") and that can be turned off via PathMatchConfigurer. The other is content type determination and that can be either turned off completely via ContentNegotiationConfigurer or specific file extensions may be registered. Registering specific file extensions has the effect of restricting both suffix pattern matching and content type determination to the given file extensions.

Turning off suffix pattern matching and content type resolution by file extension in favor of using a query parameter (if needed) is the ideal place to be. The explicit registration of known extensions may be a good middle ground that significantly limits the feature while also minimizing the impact to existing applications.

One idea for a list of extensions is to see what was done as part of a broader fix for RFD attacks in AbstractMessageConverterMethodProcessor, look for safeExtensions. From the experience with the RFD fix, you can also see the counter-point to the tweet, i.e. why URL-based content type negotiation is still desirable sometimes, but again a query parameter should better serve such requirements.

For what it's worth on the WebFlux side we do not support mapping by file extension. Neither do we support content type negotiation by file extension. By default only HeaderContentTypeResolver is registered and you can add a ParameterContentTypeResolver (i.e. query parameter) if you want URL-based alternative but there is no built-in resolver for file name extensions.

[rwinch]

To add to what Rossen stated. Allowing for suffix pattern matching and matrix variables has been a large source of security bypass vulnerabilities. I would love to see path matching be less complicated to reduce the possibility of vulnerabilities in applications.

[philwebb]

It's very tempting to change our default in 2.0 and offer a quick property to flip things back. I know it could cause upgrade pain, but it also:

• Aligns WebMvc and WebFlux
• Reduces the chances of a security vulnerability
• Follows our own documentation advice

[bclozel]

I'd like to flip that default as well.

We should do that as soon as possible, because this might impact a lot of applications/clients relying on that behavior. By our own standards, changing that after the milestone phase is the very last opportunity to do that.

[dpash]

I believe this change has made WebMvcConfigurer.configureContentNegotiation(ContentNegotiationConfigurer configurer) ineffective, regardless of the @order value given, as WebMvcAutoConfiguration.configureContentNegotiation(ContentNegotiationConfigurer configurer) always seems to happen after the user configurer.

This means the only way to configure the content negotiation is through the property files.

In 1.5, I was calling configurer.favorParameter(true), but this gets overwritten by the default property, false.

[bclozel]

Hi @dpash, I've reproduced this and it seems to be a much broader issue about the ordering of Spring Boot's WebMvcConfigurer. Could you create a separate issue?

[virgiliu-ratoi-ec-ext]

The behaviour before Spring Boot 2.0 has some weird behaviour in particular scenarios.

I have a file like this {context-root}/.0.44a38907.chunk.js, please note that the file name starts with a dot . and when a request is made to download the file, Spring MVC via RequestMappingInfoHandlerMapping and AbstractHandlerMethodMapping matches this path to /.* and returns the view associated to the root / which happens to be an HTML file in my case.

Btw, it seems that whenever a request is made, the registered paths are iterated to find the matching one. Shouldn't they be cached somehow, in case a path has been matched for a request the next time a similar request is made the method mapping should not be recalculated?

[sportmachine]

no clear documents, changing fast. The spring security and suffix things waste a lot of people time, totally stupid, nutty!

[philwebb]

For those looking for documentation, the migration guide should help.

[maunzCache]

This fix really killed my application and i had no idea where to look for the fix because there was no appropiate error warning.

Please consider my story:
I have a @RestController which accepts an email address as @PathVariable . You all know the RFC which allows the . char to appear in an URI so there is no percent-encoding for this. If you'd now call the endpoint e.g. GET /person/email@provider.com the spring application would always try to serve an unknown filetype because of the .com "file ending". Even when i enforced a media type via the produces parameter it would not work.

Even though i think it is usefull to have an auto-configuration like this, it breaks existing applications with no reason and feels like an anti-pattern. I'd suggest a bug here that the @RequestMapping annotation should be always considered first to override the behavior of suffix path matching. Feel free to argue but this really killed my code. I just noticed it by pure luck while lurking through the migration guide.

[bclozel]

@getJack Sorry that you have to deal with this situation. I've got a few questions for you, so we can hopefully resolve that situation - could you answer those?

1. How were you matching that @PathVariable previously? Could you share the controller method signature?

2. Suffix pattern matching is more or less equivalent to suffix all your patterns with ".*". If you had a "/email/{email}" pattern on a controller, this means a "/email/{email}.*" pattern is considered. Did your current application work with emails like "user@domain.co.uk"?

3. What happens if you change your route pattern with something like @GetMapping("/email/{email:.+}")?

[maunzCache]

Hi @bclozel ,
sorry i didn't thought through my comment. Could've given you the information from the get go.
This may answer all your questions at once.
@GetMapping(path = "/person/{email:.+}", produces = MediaType.APPLICATION_JSON_UTF8_VALUE) ResponseEntity<Person> getPerson(@Nonnull @PathVariable("email") final String email)
So no huge suprise here.

Regarding 2): Not sure if there was ever a check for domains like ".co.uk" . Just inherited the project from a co-worker.

[bclozel]

@getJack there must be something else at play here, because this mapping still properly works in a Spring Boot 2.0 application. Do you have a sample application I can take a look at?

[maunzCache]

I know what went wrong here. There was an empty annotated class using @EnableWebMvc.
Sorry for bloating that closed issue.