(struts) branch feature/WW-5743-path-wildcard-ranking created (now b0eed791a)

[lu...@apache.org]

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

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


      at b0eed791a WW-5743 fix(convention): rank path-spanning ** patterns after single-segment ones

This branch includes the following new commits:

     new b0eed791a WW-5743 fix(convention): rank path-spanning ** patterns after single-segment ones

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(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);