Browse Source

Do not let PathFilter.create("a/b") match 'a' unless 'a' is a subtree

PathFilter and PathFilterGroup form JGit's implementation of git's
path-limiting feature in commands like log and diff.  To save time
when traversing trees, a path specification

	foo/bar/baz

tells the tree walker not to traverse unrelated trees like qux/.  It
does that by returning false from include when the tree walker is
visiting qux and true when it is visiting foo.

Unfortunately that test was implemented to be slightly over-eager: it
doesn't only return true when asked whether to visit a subtree "foo"
but when asked about a plain file "foo" as well.  As a result, diffs
and logs restricted to some-file/non-existing-suffix unexpectedly
match against some-file:

 $ jgit log -- LICENSE/no-such-file
 commit 629fd0d594
 Author: Shawn O. Pearce <spearce@spearce.org>
 Date:   Fri Jul 02 14:52:49 2010 -0700

     Clean up LICENSE file
[...]

Fix it by checking against the entry's mode.

Gitiles +log has the same bug and benefits from the same fix.

Callers know not to worry about what subtrees are included in the tree
walk because shouldBeRecursive() returns true in this case, so this
behavior change should be safe.  This also better matches the behavior
of C git:

 $ empty=$(git mktree </dev/null)
 $ git diff-tree --abbrev $empty HEAD -- LICENSE/no-such-file
 $ git diff-tree --abbrev $empty HEAD -- tools/no-such-file
 :000000 040000 0000000... b62648d... A  tools

Bug: 484266
Change-Id: Ib4d53bddd8413a9548622c7b25b338d287d8889d
tags/v4.2.0.201601211800-r
Jonathan Nieder 8 years ago
parent
commit
75a0dcac18

+ 37
- 6
org.eclipse.jgit.test/tst/org/eclipse/jgit/treewalk/filter/PathFilterGroupTest.java View File

@Test @Test
public void testKeyIsPrefixOfFilter() throws MissingObjectException, public void testKeyIsPrefixOfFilter() throws MissingObjectException,
IncorrectObjectTypeException, IOException { IncorrectObjectTypeException, IOException {
assertMatches(Sets.of("b/c"), fakeWalk("b"));
assertMatches(Sets.of("c/d/e", "c/d/f"), fakeWalk("c/d"));
assertMatches(Sets.of("c/d/e", "c/d/f"), fakeWalk("c"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"), fakeWalk("d/e/f"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"), fakeWalk("d/e"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"), fakeWalk("d"));
assertMatches(Sets.of("b/c"), fakeWalkAtSubtree("b"));
assertMatches(Sets.of("c/d/e", "c/d/f"), fakeWalkAtSubtree("c/d"));
assertMatches(Sets.of("c/d/e", "c/d/f"), fakeWalkAtSubtree("c"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"),
fakeWalkAtSubtree("d/e/f"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"),
fakeWalkAtSubtree("d/e"));
assertMatches(Sets.of("d/e/f/g", "d/e/f/g.x"), fakeWalkAtSubtree("d"));

assertNoMatches(fakeWalk("b"));
assertNoMatches(fakeWalk("c/d"));
assertNoMatches(fakeWalk("c"));
assertNoMatches(fakeWalk("d/e/f"));
assertNoMatches(fakeWalk("d/e"));
assertNoMatches(fakeWalk("d"));

} }


@Test @Test
return ret; return ret;
} }


TreeWalk fakeWalkAtSubtree(final String path) throws IOException {
DirCache dc = DirCache.newInCore();
DirCacheEditor dce = dc.editor();
dce.add(new DirCacheEditor.PathEdit(path + "/README") {
public void apply(DirCacheEntry ent) {
ent.setFileMode(FileMode.REGULAR_FILE);
}
});
dce.finish();

TreeWalk ret = new TreeWalk((ObjectReader) null);
ret.addTree(new DirCacheIterator(dc));
ret.next();
while (!path.equals(ret.getPathString())) {
if (ret.isSubtree()) {
ret.enterSubtree();
}
ret.next();
}
return ret;
}
} }

+ 5
- 2
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/TreeWalk.java View File

* Test if the supplied path matches the current entry's path. * Test if the supplied path matches the current entry's path.
* <p> * <p>
* This method tests that the supplied path is exactly equal to the current * This method tests that the supplied path is exactly equal to the current
* entry, or is one of its parent directories. It is faster to use this
* entry or is one of its parent directories. It is faster to use this
* method then to use {@link #getPathString()} to first create a String * method then to use {@link #getPathString()} to first create a String
* object, then test <code>startsWith</code> or some other type of string * object, then test <code>startsWith</code> or some other type of string
* match function. * match function.
* <p>
* If the current entry is a subtree, then all paths within the subtree
* are considered to match it.
* *
* @param p * @param p
* path buffer to test. Callers should ensure the path does not * path buffer to test. Callers should ensure the path does not
// If p[ci] == '/' then pattern matches this subtree, // If p[ci] == '/' then pattern matches this subtree,
// otherwise we cannot be certain so we return -1. // otherwise we cannot be certain so we return -1.
// //
return p[ci] == '/' ? 0 : -1;
return p[ci] == '/' && FileMode.TREE.equals(t.mode) ? 0 : -1;
} }


// Both strings are identical. // Both strings are identical.

+ 3
- 3
org.eclipse.jgit/src/org/eclipse/jgit/treewalk/filter/PathFilterGroup.java View File

int hash = hasher.nextHash(); int hash = hasher.nextHash();
if (fullpaths.contains(rp, hasher.length(), hash)) if (fullpaths.contains(rp, hasher.length(), hash))
return true; return true;
if (!hasher.hasNext())
if (prefixes.contains(rp, hasher.length(), hash))
return true;
if (!hasher.hasNext() && walker.isSubtree()
&& prefixes.contains(rp, hasher.length(), hash))
return true;
} }


final int cmp = walker.isPathPrefix(max, max.length); final int cmp = walker.isPathPrefix(max, max.length);

Loading…
Cancel
Save