Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use PathPatternParser by default in Spring MVC #28607

Closed
rstoyanchev opened this issue Jun 10, 2022 · 3 comments
Closed

Use PathPatternParser by default in Spring MVC #28607

rstoyanchev opened this issue Jun 10, 2022 · 3 comments
Assignees
@rstoyanchev
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone
6.0.0-M5

Comments

@rstoyanchev
Copy link
Contributor

PathPatternParser was introduced for WebFlux in 5.0 as a replacement for AntPathMatcher that uses pre-compiled patterns, supports mapping without decoding the full URL path, and optimizes the pattern syntax for web (vs file system) paths.

It has been available for use in Spring MVC since version 5.3 and has been enabled as the default in Spring Boot since version 2.6 with spring-projects/spring-boot#24805.

This issue is to consider switching the default in Spring Framework to PathPatternParser for 6.0, and potentially deprecate PathMatcher and related options.
@rstoyanchev rstoyanchev added for: team-attention in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Jun 10, 2022
@rstoyanchev rstoyanchev added this to the Triage Queue milestone Jun 10, 2022
@rstoyanchev rstoyanchev self-assigned this Jun 10, 2022
@rstoyanchev rstoyanchev mentioned this issue Jun 10, 2022
Closed
@bclozel
Copy link
Member

bclozel commented Jun 21, 2022

I think we should do this (including the deprecation) for 6.0.

@rstoyanchev rstoyanchev changed the title Consider a switch to PathPatternParser by default in Spring MVC Use PathPatternParser by default in Spring MVC Jun 21, 2022
@rstoyanchev rstoyanchev modified the milestones: Triage Queue, 6.0.0-M5 Jun 21, 2022
@rstoyanchev
Copy link
Contributor Author

Team decision: we'll go ahead with it and align with the default in Boot.

@rstoyanchev rstoyanchev mentioned this issue Jun 28, 2022
Closed
rstoyanchev added a commit that referenced this issue Jun 29, 2022
@rstoyanchev
Improve MappingMatch determination in mock request
8a9b082 
MockHttpServletRequest now checks the requestURI and servletPath to
check whether they imply a Servlet path mapping, which is the case
when the requestURI is longer than the contextPath + servletPath.

This is essential when parsed patterns are in use in which case the
request path is parsed taking into account only the requestURI and
the contextPath. However, if the MappingMatch indicates a match by
Servlet path, then the servletPath is also taken into account.

See gh-28607
@rstoyanchev
Copy link
Contributor Author

rstoyanchev commented Jun 29, 2022 •
edited

Parsed patterns are now enabled by default in Spring MVC. This means, each HandlerMapping, or rather every subclass of AbstractHandlerMapping has a PathPatternParser instance, and that effectively means parsed patterns are used instead of String path matching with AntPathMatcher.

To minimize unnecessary breakage where intentions are clear, String path matching is still enabled through the MVC config, when either of the following is true:

  • PathPatternParser is not explicitly set, while AntPathMatcher or UrlPathHelper related options are customized.
  • PathPatternParser is explicitly set to null.

Further steps have also been taken to minimize failures in existing tests. For example the URL path is parsed per handler lookup if not called by the DispacherServlet, which pre-parses and caches the path. Likewise, MockHttpServletRequest detects a Servlet path mapping, to ensure a Servlet path prefix is taken into account.

@rstoyanchev rstoyanchev mentioned this issue Jun 29, 2022
Closed
rstoyanchev added a commit that referenced this issue Jun 29, 2022
@rstoyanchev
Deprecate UrlPathHelper shortcuts on AbstractHandlerMapping
14fd260 
Given the availability of two alternatives mechanisms for URL path
matching, PathPatternParser and AntPathMatcher, and now that
parsed patterns are enabled by default, it makes sense to reduce the
proliferation of options on AbstractHandlerMapping by deprecating
shortcuts related to String path matching. Most applications rely
on Boot and on the MVC config to do all this.

See gh-28607
@bclozel bclozel mentioned this issue Dec 6, 2022
Closed
@bclozel bclozel mentioned this issue Dec 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rstoyanchev rstoyanchev

Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Milestone
6.0.0-M5
Development

No branches or pull requests
2 participants
@bclozel @rstoyanchev