Clarify that MVC components provided through WebMvcRegistrations are subject to subsequent processing and configuration by MVC

[vpavic]

Previously, I was able to disable Spring MVC's trailing slash matching using something like this:

@Configuration(proxyBeanMethods = false)
class WebMvcConfiguration {

    @Bean
    WebMvcRegistrations webMvcRegistrations() {
        return new WebMvcRegistrations() {

            @Override
            public RequestMappingHandlerMapping getRequestMappingHandlerMapping() {
                RequestMappingHandlerMapping handlerMapping = new RequestMappingHandlerMapping();
                handlerMapping.setUseTrailingSlashMatch(false);
                return handlerMapping;
            }

         };
    }

}

This doesn't work any more since 2.6.0 and the change of behavior appears to be related to introduction of PathPatternParser as the default path matching strategy.

I've put together a reproduced in https://github.com/vpavic/mcve-spring-mvc-trailing-slash.

By default, trailing slash matching is enabled and curl -sv -u user:password http://localhost:8080/resources/ (note the trailing slash) returns 200 response. There are two profiles that disable trailing slash matching - configurer and registrations. The former uses WebMvcConfigurer based approach and this works (meaning, cURL request returns 404), while the latter uses WebMvcRegistrations based approach (as in the snippet above) and this doesn't work (returns 200).

Downgrading Spring Boot to 2.5.x makes the registrations profile work as expected.

I'm reporting this here for starters because what's failing is a Spring Boot specific configuration mechanism (WebMvcRegistrations) even though it seems to me that the fix might very well be needed in the Framework.

[wilkinsona]

Thanks for the report and, in particular, the MCVE.

WebMvcRegistrations plugs into Spring MVC via WebMvcConfigurationSupport.createRequestMappingHandlerMapping(). Its javadoc states that it is a "protected method for plugging in a custom subclass of RequestMappingHandlerMapping" Based on this, I would argue that your WebMvcRegistrations instance isn't using the plug point as intended. The javadoc of WebMvcRegistrations.getRequestMappingHandlerMapping also alludes to the current behaviour as it says that the method should "return the custom RequestMappingHandlerMapping that should be used and processed by the MVC configuration". The key part here is "and processed by the MVC configuration" as it's this subsequent processing that is causing the trailing slash configuration to be lost.

In short, your WebMvcConfigurer-based approach is using the extension points and configuration mechanisms as intended. We can use this issue to clarify the javadoc and reference documentation around WebMvcRegistrations to make it clear that it's intended for providing custom subclasses of MVC components and that configuration should be done via WebMvcConfigurer.

[vpavic]

Thanks for taking a look at this.

I don't recall where the idea to use WebMvcRegistrations based approach to disable trailing slash was picked up, but I've traced the use of it in one project all the way back to early 2017 and Spring Boot 1.5 (where it was still WebMvcRegistrationsAdapter).

What I probably failed to emphasize a bit better in the opening comment is that the behavior of the WebMvcRegistrations based approach really depends on path matching strategy. In other words, if I flip spring.mvc.pathmatch.matching-strategy back to ant_path_matcher (which was the default pre-2.6) it works as it did before.

[wilkinsona]

if I flip spring.mvc.pathmatch.matching-strategy back to ant_path_matcher (which was the default pre-2.6) it works as it did before

This would have to be addressed in Framework. This difference in behaviour is due to different logic in Framework being applied depending on whether or not a pattern parser has been configured. For example, when a pattern parser has been configured, RequestMappingInfo's default builder expects all the configuration to be encapsulated in the parser which causes it to ignore the trailing slash match config option.

I can certainly see how the difference in behavior of the two different matching strategies is confusing. I'll raise this with the Framework team as I want to be sure that the documentation changes I'm proposing above are aligned with the intent in MVC.

[vpavic]

OK, thanks. I suspected if a change was to be made it would be in Framework but it's good to also have additional clarifications in Boot's docs.

[rstoyanchev]

WebMvcRegistrations uses WebMvcConfigurationSupport protected methods that expose the creation of @RequestMapping infrastructure instances that are then further initialized, also with participation from WebMvcConfigurers. So you can use WebMvcRegistrations to replace instance, but cannot (reliably) change default values. The intended way to customize values is through WebMvcConfigurer methods, configurePathMatch in this case.

Note that RequestMappingHandlerMapping#setUseTrailingSlashMatch does make an effort to update the PathPatternParser it contains, but PathPatternParser is not set by default, which means setUseTrailingSlashMatch is called first, and later Boot sets the PathPatternParser. I'm not sure there is much we can do in Framework either to improve this. We cannot change an externally provided PathPatternParser instance since we don't know if it's configured as it is, intentionally or not.

[vpavic]

which means setUseTrailingSlashMatch is called first, and later Boot sets the PathPatternParser

Exactly, but the issue is that you then end up in an inconsistent state, where useTrailingSlashMatch in RequestMappingHandlerMapping is false but matchOptionalTrailingSeparator in the supplied PathPatternParser is true.

IMO at the very least (if no changes in behavior are to be made) Framework could override #setPatternParser in RequestMappingHandlerMapping to detect the mismatch and log a warning to help users that might end up in this situation. If that's acceptable from the Framework perspective, I can open a PR to address that.

Again, the problem is that the WebMvcRegistrations approach is working with ant path matching and with the switch of default of default strategy in Spring Boot 2.6 some users might silently get exposed to some of the concerns outlined in spring-projects/spring-framework#28552.

[quaff]

Can we support spring.mvc.pathmatch.use-trailing-slash-match like spring.mvc.pathmatch.use-suffix-pattern ?

spring-boot/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/WebMvcProperties.java

Line 411 in 47da706

private boolean useSuffixPattern = false;

[vpavic]

I like the idea @quaff - I've put together a quick PR (see #31327) to consider it.

[rstoyanchev]

I agree that the current situation is a bit strange. The useTrailingSlashMatch option on RMHM tries to work as a convenience method by also setting the same on PathPatternParser, but the fact that PathPatternParser isn't set by default and must be set explicitly, in effect means that you must invoked those in the correct order, or otherwise one can override the other unintentionally.

For 6.0, I am proposing to use PathPatternParser by default in Spring Framework with spring-projects/spring-framework#28607, which would remove the need for a PathPatternParser to be set explicitly. That would address the issue by itself but in addition I am also proposing to deprecate the trailingSlash option altogether in spring-projects/spring-framework#28552 (comment). Both of these are pending team decisions at the moment but we should be able to decide on that soon.