marc schipperheyn opened SPR-7336 and commented

The AntPathMatchers in some cases prefers less specific matches over specific matches. This examples is from the case of WebContentInterceptor


<property name="cacheMappings">
    <props>
        <prop key="/**/">1800</prop>
        <prop key="/**/*.html">1800</prop>
        <prop key="/**/offerform/**/*.html">-1</prop>
        <prop key="/**/offerform/*.html">-1</prop>
        <prop key="/**/cart/">-1</prop>
        <prop key="**/cart/**/*.html">-1</prop>
    </props>
</property>

The order doesn't matter. The issue occurs also if we put /**/ at the bottom

Example urlPath

/en/offerform/step4.html?offerId=17
matches /**/offerform/**/*.html
matches /**/offerform/*.html
matches /**/ [shouldn't match at all AFAIC]
matches /**/*.html

result: 1800. Should be -1

Example urlPath

/en/cart/
matches /**/
matches /**/cart/

result: -1 correct

It seems that the issue doesn't occur when the path ends with a / in stead of with an extension *.html

Affects: 3.0.3

2 votes, 4 watchers

Comment From: spring-projects-issues

Luke Taylor commented

Did you mean to register this against Spring?

Comment From: spring-projects-issues

marc schipperheyn commented

yeah, sorry.

Comment From: spring-projects-issues

John Latham commented

Hmmm, this has gone from Bug to Improvement. Is behaviour of AntPathMatcher supposed to be consistent between 2.5 and 3.0? It isn't consistent in my experience. Is that known? Please advise whether I should log details of differences on this issue or create another.

Comment From: spring-projects-issues

marc schipperheyn commented

Yeah, AFAIC it's a bug

Comment From: spring-projects-issues

marc schipperheyn commented

As a solution direction you could offer an overridable isBetterMatch(String currentPath,String otherMatchingPath) method that retains current functionality and that you potentially implement later in a different manner

protected Integer lookupCacheSeconds(String urlPath) {
    // direct match?
    Integer cacheSeconds = this.cacheMappings.get(urlPath);
    if (cacheSeconds == null) {
        // pattern match?
        //This determines the best pattern bij simply looking if the matching patterns is the longest
        String match = "";
        for (String registeredPath : this.cacheMappings.keySet()) {
            if (this.pathMatcher.match(registeredPath, urlPath)) {
                match = isBetterMatch(match,urlPath);
            }
        }
        cacheSeconds = this.cacheMappings.get(match);
    }
    return cacheSeconds;
}

public String isBetterMatch(String currentPath,String otherMatchingPath){
    return true;
}

Comment From: spring-projects-issues

Rossen Stoyanchev commented

Although I'm not sure what we should do yet, it clearly looks like something needs to be done. The cacheMappings in WebContentInterceptor are not sorted with AntPathMatcher PatternComparator. The order in which patterns are listed is not preserved either.

Furthermore we keep going and return the last match where in fact any match is as random as the next. I'm actually not sure why the original description states the result of specific URLs where the result can be different each time as far as I can see. Am I missing anything?

If the result is truly random as it seems to be, then we can take our picks for the fix without backwards compatibility issues. The use of AntPathMatcher/Comparator with some protected method to override might make the most sense.

Comment From: rstoyanchev

Closing as outdated. We've deprecated and turned off support for suffix pattern match in #23915 and related issues. We've added support for pre-parsed PathPattern, created specifically for Web paths, and see that as the way forward. The PathPattern syntax also does not allow "**" in the middle of a path.