Closed
Description
See this twitter exchange. We should confirm the existing behavior and consider it again for 2.0.
Metadata
Metadata
Assignees
Labels
Type
Projects
Relationships
Development
No branches or pull requests
See this twitter exchange. We should confirm the existing behavior and consider it again for 2.0.
Activity
philwebb commentedon Nov 29, 2017
/cc @rstoyanchev @rwinch for opinions on this. Is changing from the MVC default a good idea?
bclozel commentedon Nov 29, 2017
TL;DR: this issue is about flipping the default for
favorPathExtension
tofalse
in Spring Boot MVC auto-configuration.We could at least add a configuration properties so that developers can change that easily.
rstoyanchev commentedon Nov 29, 2017
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 viaContentNegotiationConfigurer
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 aParameterContentTypeResolver
(i.e. query parameter) if you want URL-based alternative but there is no built-in resolver for file name extensions.rwinch commentedon Nov 29, 2017
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 commentedon Nov 30, 2017
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:
bclozel commentedon Jan 5, 2018
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.
7 remaining items
Move spring.mvc.media-types to content-negotiation
dpash commentedon Mar 7, 2018
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 commentedon Mar 7, 2018
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 commentedon Apr 6, 2018
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 viaRequestMappingInfoHandlerMapping
andAbstractHandlerMethodMapping
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 commentedon Jun 10, 2018
no clear documents, changing fast. The spring security and suffix things waste a lot of people time, totally stupid, nutty!
philwebb commentedon Jun 11, 2018
For those looking for documentation, the migration guide should help.
maunzCache commentedon Jul 5, 2018
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 commentedon Jul 5, 2018
@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?
How were you matching that
@PathVariable
previously? Could you share the controller method signature?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"
?What happens if you change your route pattern with something like
@GetMapping("/email/{email:.+}")
?maunzCache commentedon Jul 6, 2018
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 commentedon Jul 6, 2018
@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 commentedon Jul 6, 2018
I know what went wrong here. There was an empty annotated class using
@EnableWebMvc
.Sorry for bloating that closed issue.