AntPathMatcher.AntPatternComparator algorithm wrong for hierarchical matches [SPR-8683]

[spring-projects-issues]

Dave Syer opened SPR-8683 and commented

AntPathMatcher.AntPatternComparator algorithm wrong for hierarchical matches, Consider an incoming request

GET /hotels/foo/rooms

and two patterns that match

1: /hotels/{hotel}/**
2: /**

Intuitively 1 is more specific than 2, so the comparator should return negative with compare(1,2) (i.e. 1<2). Unfortunately this is not the case because the comparator thinks that 2 is more specific because it has fewer matching points (wildcards + brackets).

---

Affects: 3.0.6

Issue Links:

• Assign lowest priority to "/**" in AntPathMatcher [SPR-10550] #15181 Assign lowest priority to "/" in AntPathMatcher (_"duplicates"**_)
• AntPatternComparator prefers a less specific match when brackets occur [SPR-8355] #13002 AntPatternComparator prefers a less specific match when brackets occur
• RequestMapping with 3 PathVariable doesn't work [SPR-6741] #11407 RequestMapping with 3 PathVariable doesn't work
• Determination of the best pattern match to map a handler to an URL [SPR-7589] #12245 Determination of the best pattern match to map a handler to an URL

Referenced from: commits spring-attic/spring-framework-issues@876f756, spring-attic/spring-framework-issues@4332c1b

6 votes, 10 watchers

[spring-projects-issues]

Chris Beams commented

Juergen, note that Dave also submitted a reproduction project via spring-attic/spring-framework-issues#6 and I've since accepted it after reproducing the issue locally.

[spring-projects-issues]

Rossen Stoyanchev commented

An almost identical example in #13002.

[spring-projects-issues]

Rossen Stoyanchev commented

The comparison favors fewer wild cards or variables as an independent priority. Or if the number of patterns is the same, the rest of the pattern (the literal parts) is taken into consideration where a greater length is preferred.

One option would be to identify patterns with nothing but wild cards and variables and consider them less specific than any pattern with a literal part.

[spring-projects-issues]

Erik Magnusson commented

I think we are also encountering the same issue in the following example.

Works as expected:

[2011-09-16 13:52:32,387] DEBUG: org.springframework.web.servlet.DispatcherServlet - DispatcherServlet with name 'Spring MVC Dispatcher Servlet' processing GET request for [/app/modul/order/5535/kop]
[2011-09-16 13:52:32,389] DEBUG: org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping - Matching patterns for request [/modul/order/5535/kop] are [/modul/order/{orderbookId}/{orderType}, /**/, /**]

Works not quite as expected:

[2011-09-16 13:56:22,923] DEBUG: org.springframework.web.servlet.DispatcherServlet - DispatcherServlet with name 'Spring MVC Dispatcher Servlet' processing GET request for [/app/modul/order/sas/5535/kop]
[2011-09-16 13:56:22,924] DEBUG: org.springframework.web.servlet.mvc.annotation.DefaultAnnotationHandlerMapping - Matching patterns for request [/modul/order/sas/5535/kop] are [/**/, /**, /modul/order/{instrumentName}/{orderbookId}/{orderType}]

[spring-projects-issues]

Erik Magnusson commented

I think the suggestion to consider any pattern with a literal part more specific than patterns with no literal part would be a good idea (in principal at least, can't comment hypothetical edge cases...).

[spring-projects-issues]

Dave Syer commented

There's a workaround in the issues project, by the way, which worked in my case. I think a variant of it will work for most cases, but it will be different for different use cases, depending on the competing patterns. My workaround was to add additional /** elements to the end of the path, so this works for the example in the description:

1: /hotels/{hotel}/**
2: /**/**/**

It pads out '2' with extra countable wildcards, so the comparator algorithm thinks it is less specific based on plain counting.

[spring-projects-issues]

Alex Antonov commented

It would help to add a section to also take into account the number of URL path segments, to see how each of the paths compares in that regard to the actual Path. The one with the closest segment count match should take precedence over the generic (wildcard + brackets) count.

[spring-projects-issues]

Andreas Wederbrand commented

I think the algorithm should compare each path segment in turn.

1. Check if one is null.
2. For each segment
3. if they are equal go to next segment
4. if one is a string literal and the other is a bracket the literal is better, regardless of if the other is much longer
5. if they are the same until one run out of path segments the longest is choose