Fix PathFilterGroup not to throw StopWalkException too early

Due to the Git internal sort order a directory is sorted as if it ended
with a '/', this means that the path filter didn't set the last possible
matching entry to the correct value. In the reported issue we had the
following filters.

	org.eclipse.jgit.console
	org.eclipse.jgit

As an optimization we throw a StopWalkException when the walked tree
passes the last possible filter, which was this:
	org.eclipse.jgit.console

Due to the git sorting order, the tree was processed in this order:
	org.eclipse.jgit.console
	org.eclipse.jgit.test
	org.eclipse.jgit

At org.eclipse.jgit.test we threw the StopWalkException preventing the
walk from completing successfully.

A correct last possible match should be:
	org.eclipse.jgit/

For simplicit we define it as:
	org/eclipse/jgit/

This filter would be the maximum if we also had e.g. org and org.eclipse
in the filter, but that would require more work so we simply replace all
characters lower than '/' by a slash.

We believe the possible extra walking does not not warrant the extra
analysis.

Bug: 362430
Change-Id: I4869019ea57ca07d4dff6bfa8e81725f56596d9f

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/filter/PathFilterGroupTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/filter/PathFilterGroupTest.java
index 8038206e9b6646db360d5ee63dba69798d5b73a8..4c329cb19194cda66a367f7bad717bd122e48ef9 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/filter/PathFilterGroupTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/filter/PathFilterGroupTest.java
@@ -76,7 +76,8 @@ public class PathFilterGroupTest {
                                "b/c",
                                "c/d/e",
                                "c/d/f",
-                               "d/e/f/g"
+                               "d/e/f/g",
+                               "d/e/f/g.x"
                                };
                // @formatter:on
                filter = PathFilterGroup.createFromStrings(paths);
@@ -90,6 +91,7 @@ public class PathFilterGroupTest {
                assertTrue(filter.include(fakeWalk("c/d/e")));
                assertTrue(filter.include(fakeWalk("c/d/f")));
                assertTrue(filter.include(fakeWalk("d/e/f/g")));
+               assertTrue(filter.include(fakeWalk("d/e/f/g.x")));
        }
 
        @Test
@@ -132,6 +134,11 @@ public class PathFilterGroupTest {
                assertTrue(filter.include(fakeWalk("c/d/e/f")));
                assertTrue(filter.include(fakeWalk("c/d/f/g")));
                assertTrue(filter.include(fakeWalk("d/e/f/g/h")));
+               assertTrue(filter.include(fakeWalk("d/e/f/g/y")));
+               assertTrue(filter.include(fakeWalk("d/e/f/g.x/h")));
+               // listed before g/y, so can't StopWalk here, but it's not included
+               // either
+               assertFalse(filter.include(fakeWalk("d/e/f/g.y")));
        }
 
        @Test
@@ -172,6 +179,9 @@ public class PathFilterGroupTest {
                        // good
                }
 
+               // less obvious #2 due to git sorting order
+               filter.include(fakeWalk("d/e/f/g/h.txt"));
+
                // non-ascii
                try {
                        filter.include(fakeWalk("\u00C0"));

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/PathFilterGroup.java b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/PathFilterGroup.java
index 66d9f87a7701e83f0933c1d21e0b65464857ec0d..bdfde0bfcd089bda6612aff022bd21cb1e555b37 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/PathFilterGroup.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/PathFilterGroup.java
@@ -208,6 +208,19 @@ public class PathFilterGroup {
                                if (compare(max, pf.pathRaw) < 0)
                                        max = pf.pathRaw;
                        }
+                       // Adjust max for the git sort order. A path we compare
+                       // with may end with a slash at any position (but the
+                       // first, but we ignore that here since it's not relevant).
+                       // Such paths must be included in the processing
+                       // before we can give up and throw a StopWalkException.
+                       byte[] newMax = new byte[max.length + 1];
+                       for (int i = 0; i < max.length; ++i)
+                               if ((max[i] & 0xFF) < '/')
+                                       newMax[i] = '/';
+                               else
+                                       newMax[i] = max[i];
+                       newMax[newMax.length - 1] = '/';
+                       max = newMax;
                }
 
                private static int compare(byte[] a, byte[] b) {