From c9a94dc1eeabeda212ed0b2eab0afdd67331b848 Mon Sep 17 00:00:00 2001 From: Robin Rosenberg Date: Tue, 2 Apr 2013 09:00:58 +0200 Subject: [PATCH] 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 --- .../jgit/treewalk/filter/PathFilterGroupTest.java | 12 +++++++++++- .../jgit/treewalk/filter/PathFilterGroup.java | 13 +++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) 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 8038206e9..4c329cb19 100644 --- 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 void setup() { "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 void testExact() throws MissingObjectException, 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 void testFilterIsPrefixOfKey() throws MissingObjectException, 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 void testStopWalk() throws MissingObjectException, // 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 66d9f87a7..bdfde0bfc 100644 --- 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 @@ private Group(final PathFilter[] pathFilters) { 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) {