(struts) 01/01: WW-5743 fix(convention): rank path-spanning ** patterns after single-segment ones

[lu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

lukaszlenart pushed a commit to branch feature/WW-5743-path-wildcard-ranking
in repository https://gitbox.apache.org/repos/asf/struts.git

commit b0eed791ac2e8e91482873e92ff5f0c3e241cac7
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Thu Sep 17 08:44:15 2026 +0200

    WW-5743 fix(convention): rank path-spanning ** patterns after single-segment ones
    
    ActionNameSpecificityComparator compared the raw wildcard-token count before
    the ** count, so `a/**` (one token) sorted ahead of `a/*/*` (two tokens) and
    shadowed it for `a/x/y`, although `**` crosses `/` and `*` does not. The **
    count is now the first key, then the token count, then the literal count.
    
    Both `*` (WildcardHelper MATCH_FILE) and `{var}` (`([^/]+)` in
    NamedVariablePatternMatcher) are single-segment, so the same rule covers
    both matchers; the tests pin one pair for each.
    
    Co-Authored-By: Claude Opus 5 (1M context) <no...@anthropic.com>
---
 .../struts2/convention/ActionNameSpecificityComparator.java  | 11 ++++++-----
 .../convention/ActionNameSpecificityComparatorTest.java      | 12 ++++++++++++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/plugins/convention/src/main/java/org/apache/struts2/convention/ActionNameSpecificityComparator.java b/plugins/convention/src/main/java/org/apache/struts2/convention/ActionNameSpecificityComparator.java
index a0455dced..f3d771ab9 100644
--- a/plugins/convention/src/main/java/org/apache/struts2/convention/ActionNameSpecificityComparator.java
+++ b/plugins/convention/src/main/java/org/apache/struts2/convention/ActionNameSpecificityComparator.java
@@ -27,9 +27,10 @@ import java.util.Comparator;
  *
  * <p>Ordering keys, applied in order:</p>
  * <ol>
+ *   <li>fewer path-spanning {@code **} tokens first — {@code **} crosses {@code /} while {@code *}
+ *       and <code>{var}</code> never do, so any pattern relying on it is broader than one that does not;</li>
  *   <li>fewer wildcard tokens first (a {@code *}/{@code **} run, or a <code>{var}</code> group);</li>
  *   <li>more literal characters first;</li>
- *   <li>fewer path-spanning {@code **} tokens first;</li>
  *   <li>natural (alphabetical) order of the pattern, for deterministic tie-breaking.</li>
  * </ol>
  *
@@ -45,6 +46,10 @@ public class ActionNameSpecificityComparator implements Comparator<String> {
         Counts ca = count(a);
         Counts cb = count(b);
 
+        int byPathWildcards = Integer.compare(ca.pathWildcards, cb.pathWildcards);
+        if (byPathWildcards != 0) {
+            return byPathWildcards;
+        }
         int byWildcards = Integer.compare(ca.wildcards, cb.wildcards);
         if (byWildcards != 0) {
             return byWildcards;
@@ -53,10 +58,6 @@ public class ActionNameSpecificityComparator implements Comparator<String> {
         if (byLiterals != 0) {
             return byLiterals;
         }
-        int byPathWildcards = Integer.compare(ca.pathWildcards, cb.pathWildcards);
-        if (byPathWildcards != 0) {
-            return byPathWildcards;
-        }
         return a.compareTo(b);
     }
 
diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/ActionNameSpecificityComparatorTest.java b/plugins/convention/src/test/java/org/apache/struts2/convention/ActionNameSpecificityComparatorTest.java
index f97a1d1d4..38ac38662 100644
--- a/plugins/convention/src/test/java/org/apache/struts2/convention/ActionNameSpecificityComparatorTest.java
+++ b/plugins/convention/src/test/java/org/apache/struts2/convention/ActionNameSpecificityComparatorTest.java
@@ -50,6 +50,18 @@ public class ActionNameSpecificityComparatorTest {
         assertTrue(comparator.compare("a/*", "a/**") < 0);
     }
 
+    @Test
+    public void twoSingleStarsBeatOnePathStar() {
+        // "a/**" spans any depth, "a/*/*" exactly two segments -> the ** pattern is broader and must lose
+        assertTrue(comparator.compare("a/*/*", "a/**") < 0);
+    }
+
+    @Test
+    public void twoNamedVariablesBeatOnePathStar() {
+        // {var} matches a single segment like *, so the same rule applies for NamedVariablePatternMatcher users
+        assertTrue(comparator.compare("a/{x}/{y}", "a/**") < 0);
+    }
+
     @Test
     public void namedVariablesCountAsWildcards() {
         assertTrue(comparator.compare("some/usefull/{id}", "some/{id}") < 0);