Affects: All Versions

The AntPathMatcher.AntPatternComparator doesn't appear to be comparing correctly.

Scenario:

  • Pattern 1: /api/v1/path/**
  • Pattern 2: /api/v1/path/{pathId}/**
  • Pattern 3: /**
  • Pattern 4: /api/**
  • Path: /api/v1/path/12345/hello

Run the following

Arrays.asList("/api/v1/path/**", "/api/v1/path/{pathId}/**", "/**", "/api/**")
    .stream()
    .sorted(new AntPathMatcher().getPatternComparator("/api/v1/path/12345/hello"))
    .collect(Collectors.toList())

What Actually Happened:

/api/v1/path/**
/api/**
/api/v1/path/{pathId}/**
/**

What I Expected:

/api/v1/path/{pathId}/**
/api/v1/path/**
/api/**
/**

Comment From: jebeaudet

I'm also running into this issue, I believe the problem is https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/main/java/org/springframework/util/AntPathMatcher.java#L793, it should be return info2.getTotalCount() - info1.getTotalCount(); instead. By simply changing this I get the correct order for your example.

Comment From: jebeaudet

I forked the repo and by changing this I got 2 failing tests : https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java#L584

https://github.com/spring-projects/spring-framework/blob/master/spring-core/src/test/java/org/springframework/util/AntPathMatcherTests.java#L469

The one at line 469 is IMO an error, /hotels/{hotel}/bookings/{booking} is more precise than /hotels/{hotel}/booking.

The test at line 584 might be legit failing though, the .* are not counted in PatternInfo as a wildcard parameter on purpose.

Any thoughts from the Spring team here?

Comment From: bclozel

I agree about @kalypzo assessment. The AntPathMatcher implementation is far from ideal and we're relying more and more on PathPatternMatcher for better performance and behavior predictability.

Over the years the quirks of AntPathMatcher have become expected behavior in many applications and sometimes changing very little in its implementation can have unwanted consequences.

@jebeaudet I disagree with your analysis. Switching to return info2.getTotalCount() - info1.getTotalCount() would mean that /hotels/*/* is more precise than /hotels/{hotel}/bookings.

The definition of "more precise" can be a little loose. I'd say a pattern is more precise is it matches less possibilities than the other pattern. In your example /hotels/{hotel}/booking is actually more precise because it matches all {hotel} combinations whereas /hotels/{hotel}/bookings/{booking} matches all the {hotel} * {booking} combinations.

Comment From: bclozel

In the meantime I've tested the sample provided by @kalypzo and it works now. I think this was fixed in #23125 - I'm marking this issue as a duplicate as a result.