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.

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

Enable PathPattern based matching for MVC actuators #24645

Closed
tommyk-gears opened this issue Jan 5, 2021 · 4 comments
Closed

Enable PathPattern based matching for MVC actuators #24645

tommyk-gears opened this issue Jan 5, 2021 · 4 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@tommyk-gears
Copy link

Spring Framework 5.3 introduced the PathPattern as an efficient alternative to AntPathMatcher URL matching.
Spring Boot 2.4 introduced (in issue #21694) a new config property to enable the PathPattern-based URL matching; spring.mvc.pathmatch.matching-strategy=path_pattern_parser.

This works well for URL matching of @Controllers etc, but org.springframework.boot.actuate.endpoint.web.servlet.WebMvcEndpointHandlerMapping does not respect the aforementioned config property - it always uses the AntPathMatcher-based URL matching. I.e. all actuator endpoints still have their URLs matched with AntPathMatcher. This is a bit painful since the WebMvcEndpointHandlerMapping typically has higher priority than the RequestMappingHandlerMapping and hence is asked to try to match every incoming request before the RequestMappingHandlerMapping gets to do its thing. I.e. every request is still matched with the AntPathMatcher even if we use spring.mvc.pathmatch.matching-strategy=path_pattern_parser.

I would expect the WebMvcEndpointHandlerMapping to respect the configuration and use the PathPattern-based URL matching in this case so that I can completely eliminate the use of AntPathMatcher.

The root cause of this seems to be that WebMvcEndpointHandlerMapping initialises its AbstractWebMvcEndpointHandlerMapping#builderConfig via a private static method that does not take any configuration into account.

In contrast RequestMappingHandlerMapping (that respects the configuration property) initialises
RequestMappingHandlerMapping#config in #afterPropertiesSet() with the pattern parser typically injected via org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter#configurePathMatch.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 5, 2021
@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 5, 2021
@philwebb philwebb added this to the 2.4.x milestone Jan 5, 2021
@philwebb philwebb added type: enhancement A general enhancement and removed type: bug A general bug labels Jan 13, 2021
@philwebb philwebb modified the milestones: 2.4.x, 2.5.x Jan 13, 2021
@philwebb philwebb added the status: pending-design-work Needs design work before any code can be developed label Jan 13, 2021
@philwebb
Copy link
Member

See also #24805

@philwebb
Copy link
Member

We might be able to share the same PathPattern instance to save memory.

@philwebb
Copy link
Member

I think it makes sense for us to completely switch our WebMvcEndpointHandlerMapping to use PathPattern rather than having it configurable. The handler mapper is an adapter to expose actuator endpoints and they already have quite tight rules around the URL patterns that are supported.

I wanted to make this change for 2.5.0.RC1, unfortunately I his a snag with spring-projects/spring-framework#26814. The same issue has also caused us to revert #24805.

@philwebb philwebb added for: team-meeting An issue we'd like to discuss as a team to make progress and removed for: team-meeting An issue we'd like to discuss as a team to make progress labels Apr 16, 2021
@mbhave mbhave self-assigned this Aug 31, 2021
@mbhave mbhave changed the title Unable to use PathPattern with actuator endpoints Enable PathPattern based matching for MVC actuators Sep 8, 2021
@mbhave mbhave removed the status: pending-design-work Needs design work before any code can be developed label Sep 8, 2021
@mbhave mbhave closed this as completed in 393081f Sep 8, 2021
@mbhave mbhave modified the milestones: 2.6.x, 2.6.0-M3 Sep 8, 2021
@hpoettker
Copy link
Contributor

This change seems to have broken compatibility with the latest release of io.springfox:springfox-boot-starter, i.e. version 3.0.0.

As springfox has not been actively maintained for a while, Spring Boot cannot be asked to actively keep up compatibility. But as springfox still seems to be popular, this might be a noteworthy remark in the release notes.

See also here: https://stackoverflow.com/questions/69108273/spring-boot-swagger-documentation-doesnt-work/69814964

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants