PathPatternPredicate still changes variables after failed AndPredicate [SPR-17210]

[spring-projects-issues]

Brian Clozel opened SPR-17210 and commented

PathPatternPredicate is a RequestPredicate implementation that has side-effects on the request; it modifies the state of request attributes, by adding URI template variables information to it.

This issue is a variant of #21233, but this time using the "and" predicate to chain several predicates.

Consider the following sample:

route(path("/spring/{project}").and(method(HttpMethod.POST)), myHandler::variables)
		.andRoute(all(), myHandler::variables);

Given a GET /spring/framework request, the following happens:

1. the path pattern predicate matches and sets the uri template variables
2. the and predicate tests the method predicate, which doesn't match
3. the "all" predicate matches, and its handler has access to uri template variables that should not be present

---

Affects: 5.0.8, 5.1 RC2

Issue Links:

• Provide matched pattern information in WebFlux fn [SPR-17098] #21635 Provide matched pattern information in WebFlux fn
• PathPatternPredicate leaves path variables in request even if predicate doesn't match [SPR-16692] #21233 PathPatternPredicate leaves path variables in request even if predicate doesn't match

Referenced from: commits a680880

[spring-projects-issues]

Brian Clozel commented

I'm wondering if, instead of mutating the request along the way, we should maybe register callbacks and apply them all, in order, right before the handler is called. this would make test side-effect free again and would probably make #21635 easier to implement. 

[spring-projects-issues]

Arjen Poutsma commented

The downside to callbacks would be that any nested predicates that depend on (attribute) changes made by a parent predicate would fail to work, because those changes would only be committed when the callback has been called. And that might have unforeseen consequences, so I am only willing to do it if I can't find another way to fix this.

[spring-projects-issues]

Arjen Poutsma commented

Fixed in a680880