Assess the situation with Framework's trailing slash matching changes

[bclozel]

Right now our build is failing because of recent changes in Framework's defaults for trailing slash match: spring-projects/spring-framework#28552.

This issue should assess the situation and:

• fix the build
• reconsider an option for trailing slash matching for backwards compatibility (see Add property to allow configuring Spring MVC trailing slash match #31327)
• check if there is a way to consider an option for the main application, and another one for the actuator web support

[vpavic]

Note that there's also #31547 which I assume this issue partly duplicates.

[wilkinsona]

We see the two as separate problems. Boot's been using PathPatternParser as the default for some time now. #31547 is about adapting to Framework now using it by default too. Those changes didn't break Boot's build but we may need to review the configuration properties that we offer and what happens when certain properties are used in combination. This issue's about adapting to a change in how trailing slashes are handled which did break our build.

[wilkinsona]

e9136e0 will hopefully fix the build. It raises a few questions:

• Endpoint paths are configured using /**. This means that both /actuator/endpoint and /actuator/endpoint/ work, irrespective of the trailing slash matching configuration. Do we want them to continue to behave this way?
• The links "endpoint" doesn't behave in the same way. It's either available at /actuator or, when on a separate port with no base path, at /. Are we OK with this inconsistency?
• Servlet and reactive EndpointRequestMatcher match using /**. This means that both /actuator/endpoint and /actuator/endpoint/ would be matched. Do we want them to continue to behave this way?

[wilkinsona]

We've discussed this today and we are happy with things the way they are. We are not going to introduce a property to enable a deprecated Spring Framework feature.

[vpavic]

Can you clarify what does happy with things the way they are mean?

I'm mostly interested in the first point from your previous comment:

Endpoint paths are configured using /**. This means that both /actuator/endpoint and /actuator/endpoint/ work, irrespective of the trailing slash matching configuration. Do we want them to continue to behave this way?

Am I reading this right that the Actuator endpoints will continue to behave the same as if trailing slash matching was on?

[wilkinsona]

Yes. An actuator endpoint may only support /actuator/endpoint, but others may also support /actuator/endpoint/some/arbitrary/length/sub/path. To support the latter we use /actuator/endpoint/** which has the side-effect of matching both /actuator/endpoint and actuator/endpoint/. The EndpointRequestMatcher implementations align with this behaviour, matching /actuator/endpoint and everything beneath that point to ensure that any security rules match the endpoint's matching behaviour.

[vpavic]

The EndpointRequestMatcher sort of assumes the use of Spring Security (which might not be the case) and doesn't really solve the request matching ambiguity concerns that were expressed in spring-projects/spring-framework#28552. If possible, I'd like to avoid such behavior by default in Actuator endpoints as well.

Also, this is now a bit surprising as it diverges from the new default behavior in the Framework.

[wilkinsona]

@rstoyanchev Is there a pattern or combination of patterns that we can use that will match /actuator/endpoint and /actuator/endpoint/some/arbitrary/length/sub/path but not /actuator/endpoint/?

[rstoyanchev]

* and ** are defined as matching 0 or more characters, and likewise {*rest} matches 0 or more path segments, so it makes sense that /actuator/endpoint/ matches when any of those are used.

A path variable, on the other hand, expects a path segment, and so /actuator/endpoint/{var} does not match /actuator/endpoint/, which means a combination of the following could do it technically speaking:

/actuator/endpoint
/actuator/endpoint/{var}
/actuator/endpoint/{var}/**

That said, taking a step back, the trailing slash option essentially is about retrying with the same pattern + trailing slash. A pattern such as /actuator/endpoint/** is effectively a match by prefix, which expects /actuator/endpoint at a minimum, and it can be followed by anything, and that means anything, which to me includes a trailing slash as well. Furthermore, such an extra trailing slash check normally goes after the pattern, e.g. after the **, but that would never even be used since ** would match the trailing slash, which again is the end of the entire pattern (not after the prefix).

In short, I'm questioning whether a prefix pattern is in the same category as other patterns. I would expect a similar strategy for securing through a pattern by prefix and there should be no mismatch there as far as I can see.