Issue:
Created 2 mappings as following in the controller, code is in Controller
.java
of the attached project.
@GetMapping("/foo/**")
public ResponseEntity getFoo() {}
@GetMapping("/foo/**/bar")
public ResponseEntity getFooBar() {}
- In
webmvc
whenever request comes as/foo/1/2/3/bar
(as far as last path segment isbar
) it maps togetFooBar
. - If
bar
is not specified ast last path segment then it maps togetFoo
. i.e allfoo/1
,foo/1/2
,foo/ab/cd
.... maps togetFoo
. - The issue is in webflux, when I switch to
webflux
all mappings map togetFoo
. No matter if path consists ofbar
in the last path segment. - For eg.
/foo/ab/cd/bar
maps togetFoo
instead ofgetFooBar
.
Uploaded the project here , to switch between
webflux
andwebmvc
please comment out appropriate starter dependency inpom.xml
.
Repro project here : https://github.com/kaladhar-mummadi/demo-issue
Notes:
- Webmvc AntPathMatcherTests has tests like
/bla/**/bla
, which are not covered in PathPatternTests of WebFlux.
Comment From: bclozel
Thanks for raising this, it seems we need to improve that case in our implementation and in the reference docs.
First, I believe this is by design (see the first comment in #19112); if I remember correctly, not supporting "**"
segments in the middle of patterns was done on purpose, for several reasons:
* this does not perform well, since it requires backtracking
* it's been often the source of misunderstanding and complex matches which lead to strange matches or even security issues in particular applications
PathPattern
does support **
at the end of the pattern only (see PathPattern
Javadoc) - the same way it supports the new "/path/{*captureTheRest}"
syntax (but with additional capturing).
I'll turn this issue into an enhancement with two goals:
- Ensure that
"/path/**/other"
patterns are rejected withPatternParseException
- Fix the WebFlux reference documentation on URI matching which does not reflect this limitation nor additional capabilities of this implementation
Comment From: bclozel
Note: this issue can be reproduced with a simple test:
PathPatternParser parser = new PathPatternParser();
PathPattern pattern = parser.parse("/test/**/spring");
PathContainer pathContainer = PathContainer.parsePath("/test/project/other/spring");
boolean matches = pattern.matches(pathContainer);
assertThat(matches).isTrue();
Comment From: ranarula
@bclozel - I see a recent ticket https://github.com/spring-projects/spring-framework/issues/24945 to align MVC towards using PathPatternParser. Won't this be a breaking change ?
I wonder why there are different strategies used for WebFlux and MVC. Would have been better if the change remained consistent across both.
Comment From: bclozel
I see a recent ticket #24945 to align MVC towards using PathPatternParser. Won't this be a breaking change ?
This won't be a breaking change since we'll support both AntPathMatcher
and PathPatternParser
infrastructures for Spring MVC, leaving the current one as the default. This just gives an extra choice for developers interested in optimal path matching performance (we're getting quite a few requests about that).
I wonder why there are different strategies used for WebFlux and MVC. Would have been better if the change remained consistent across both.
WebFlux was a completely new effort and we took that opportunity to revisit parts we wanted to improve. Aligning both MVC and WebFlux right away in Spring Framework 5.0 wouldn't have been a wise choice since at that point we didn't have much experience/feedback on running WebFlux applications in production.
As for adding that feature to PathPatternParser
, I don't think we should do that for the reasons listed above. Even if WebFlux applications are less popular than MVC apps, I believe it's the first time we're getting an issue about this since 5.0 is out (in 2017).
Comment From: ranarula
Thanks @bclozel for the explanation. Make sense.
If both AntPathMatcher
and PathPatternParser
options are available for MVC I was wondering if there will be an option available to the developer to choose one or the other infrastructure ?
Comment From: bclozel
@ranarula Yes, we’ll provide a configuration option in the usual places in Spring Framework and we’ll probably make it a configuration property in Spring Boot.
Comment From: rstoyanchev
Ensure that
"/path/**/other"
patterns are rejected with PatternParseException
This is a good idea. The error could suggest re-writing as "/path/*/other"
and that should be an easy correction.
Fix the WebFlux reference documentation on URI matching..
Also the PathPattern
Javadoc could be made more explicit. Currently it says only:
** matches zero or more path segments until the end of the path