Skip to content

Add option to configure PathPatternParser in Spring MVC #21694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bclozel opened this issue Jun 4, 2020 · 2 comments
Closed

Add option to configure PathPatternParser in Spring MVC #21694

bclozel opened this issue Jun 4, 2020 · 2 comments
Assignees
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Jun 4, 2020

As of Spring Framework 5.3 (See spring-projects/spring-framework#24945), it will be possible to use PathPatternParser to parse and match request mapping path patterns, as an alternative to the current default AntPathMatcher.

This new implementation has been used for a while already in Spring WebFlux and is stable.
It’s been designed for consistency and performance, fixing issues we’ve seen in the past with the AntPathMatcher variant.

We should consider switching to this new variant in MVC; there are behavior differences and incompatible changes between the two. For example, double wildcards are rejected when used in the middle of the pattern. See spring-projects/spring-framework#24952

We could offer a new configuration property for switching between the two implementations and even choose PathPatternParser as the default one. In case existing applications use invalid patterns, a PatternParseException will be raised during application startup - we could create a dedicated failure analyzer to guide developers in those cases: fix the pattern if possible or switch back the implementation using the new property.

Edit: in the meantime, a blog post explaining these changes has been published.

@bclozel bclozel added type: enhancement A general enhancement status: noteworthy A noteworthy issue to call out in the release notes labels Jun 4, 2020
@bclozel bclozel added this to the 2.4.x milestone Jun 4, 2020
@bclozel bclozel self-assigned this Jun 4, 2020
@bclozel
Copy link
Member Author

bclozel commented Jul 6, 2020

I've been working on this change and I've found some failures in our build.

As underlined by Spring Framework in its docs, some configuration options/combinations are not supported with the PathPatternParser; they're not best practices and some are deprecated already:

Note that server.servlet.context-path is supported. It is used by many applications, for prefixing all endpoints with a common context when deployed behind a gateway or front-end proxy.

So from a Spring Boot perspective, I'm wondering how we should work with that.

  1. Should we still make the PathPatternParser the default now or just make it an option?
  2. If some of those options are used with PathPatternParser, things break or behave unexpectedly. Should we fail hard if the wrong combination of options is used? How?
  3. With all the deprecations, the servlet prefix for DispatcherServlet seems to be deprecated as a result (if AntPathMatcher will be retired and the new one doesn't support it). Should we also deprecate that feature in Spring Boot altogether in favor of something else? I've discussed that last point with the Framework team: there might be a way to make the servlet path work (only with prefix) but as many things with the Servlet conf, there are important points to consider.

@bclozel bclozel added the for: team-attention An issue we'd like other members of the team to review label Jul 6, 2020
@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Jul 6, 2020
@bclozel
Copy link
Member Author

bclozel commented Jul 6, 2020

Because there are many possible sources of issues when switching to PathPatternParser (incompatible options suffix matching options, servlet mapping, illegal patterns) - we think that changing this default in 2.4 is too big of a change.

We're going to introduce a new configuration option that allows developers to switch to PathPatternParser with Spring MVC. Hopefully this will give enough feedback to both the Framework and the Boot teams about this new infrastructure in Spring MVC.

@bclozel bclozel changed the title Consider switching to PathPatternParser in Spring MVC Add option to configure PathPatternParser in Spring MVC Jul 9, 2020
@bclozel bclozel modified the milestones: 2.4.x, 2.4.0-M2 Jul 9, 2020
@bclozel bclozel closed this as completed in 0f264b6 Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: noteworthy A noteworthy issue to call out in the release notes type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants