Exclude file matching: fix backtracking on match failures after **

** matching always tries the empty match first. If a mismatch occurs
later, the ** must be extended by exactly one segment and matching must
resume with the matcher following the ** matcher.

Bug: 520920
Change-Id: Id019ad1c773bd645ae92e398021952f8e961f45c
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

4 files changed, 150 insertions, 1 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesHandlerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesHandlerTest.java
index 0717379e24..5868482c88 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesHandlerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/AttributesHandlerTest.java
@@ -452,6 +452,50 @@ public class AttributesHandlerTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack() throws Exception {
+		setupRepo(null, null, "**/sub/new/ bar", null);
+		writeTrashFile("sub/new/foo.txt", "1");
+		writeTrashFile("foo/sub/new/foo.txt", "2");
+		writeTrashFile("sub/sub/new/foo.txt", "3");
+		walk = beginWalk();
+		assertIteration(F, ".gitattributes");
+		assertIteration(D, "foo");
+		assertIteration(D, "foo/sub");
+		assertIteration(D, "foo/sub/new", attrs("bar"));
+		assertIteration(F, "foo/sub/new/foo.txt");
+		assertIteration(D, "sub");
+		assertIteration(F, "sub/a.txt");
+		assertIteration(D, "sub/new", attrs("bar"));
+		assertIteration(F, "sub/new/foo.txt");
+		assertIteration(D, "sub/sub");
+		assertIteration(D, "sub/sub/new", attrs("bar"));
+		assertIteration(F, "sub/sub/new/foo.txt");
+		endWalk();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack2() throws Exception {
+		setupRepo(null, null, "**/**/sub/new/ bar", null);
+		writeTrashFile("sub/new/foo.txt", "1");
+		writeTrashFile("foo/sub/new/foo.txt", "2");
+		writeTrashFile("sub/sub/new/foo.txt", "3");
+		walk = beginWalk();
+		assertIteration(F, ".gitattributes");
+		assertIteration(D, "foo");
+		assertIteration(D, "foo/sub");
+		assertIteration(D, "foo/sub/new", attrs("bar"));
+		assertIteration(F, "foo/sub/new/foo.txt");
+		assertIteration(D, "sub");
+		assertIteration(F, "sub/a.txt");
+		assertIteration(D, "sub/new", attrs("bar"));
+		assertIteration(F, "sub/new/foo.txt");
+		assertIteration(D, "sub/sub");
+		assertIteration(D, "sub/sub/new", attrs("bar"));
+		assertIteration(F, "sub/sub/new/foo.txt");
+		endWalk();
+	}
+
+	@Test
 	public void testDirectoryMatchSubComplex() throws Exception {
 		setupRepo(null, null, "s[uv]b/n*/ bar", null);
 		writeTrashFile("sub/new/foo.txt", "1");
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/CGitAttributesTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/CGitAttributesTest.java
index c8ff6c8752..6188fa60c9 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/CGitAttributesTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/attributes/CGitAttributesTest.java
@@ -320,6 +320,51 @@ public class CGitAttributesTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack() throws Exception {
+		createFiles("src/new/foo.txt", "src/src/new/foo.txt");
+		writeTrashFile(".gitattributes", "**/src/new/ bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack2() throws Exception {
+		createFiles("src/new/foo.txt", "src/src/new/foo.txt");
+		writeTrashFile(".gitattributes", "**/**/src/new/ bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack3() throws Exception {
+		createFiles("src/new/src/new/foo.txt",
+				"foo/src/new/bar/src/new/foo.txt");
+		writeTrashFile(".gitattributes", "**/src/new/ bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack4() throws Exception {
+		createFiles("src/src/src/new/foo.txt",
+				"foo/src/src/bar/src/new/foo.txt");
+		writeTrashFile(".gitattributes", "**/src/ bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack5() throws Exception {
+		createFiles("x/a/a/b/foo.txt", "x/y/z/b/a/b/foo.txt",
+				"x/y/a/a/a/a/b/foo.txt", "x/y/a/a/a/a/b/a/b/foo.txt");
+		writeTrashFile(".gitattributes", "**/*/a/b bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack6() throws Exception {
+		createFiles("x/a/a/b/foo.txt", "x/y/a/b/a/b/foo.txt");
+		writeTrashFile(".gitattributes", "**/*/**/a/b bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
 	public void testDirectoryMatchSubComplex() throws Exception {
 		createFiles("src/new/foo.txt", "foo/src/new/foo.txt", "sub/src/new");
 		writeTrashFile(".gitattributes", "s[rs]c/n*/ bar\n");
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
index 3ea894f492..c61f2ea7f6 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/CGitIgnoreTest.java
@@ -203,4 +203,41 @@ public class CGitIgnoreTest extends RepositoryTestCase {
 		writeTrashFile(".gitignore", "**/src/new/");
 		assertSameAsCGit();
 	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack() throws Exception {
+		createFiles("src/new/foo.txt", "src/src/new/foo.txt");
+		writeTrashFile(".gitignore", "**/src/new/");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack2() throws Exception {
+		createFiles("src/new/foo.txt", "src/src/new/foo.txt");
+		writeTrashFile(".gitignore", "**/**/src/new/");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack3() throws Exception {
+		createFiles("x/a/a/b/foo.txt");
+		writeTrashFile(".gitignore", "**/*/a/b/");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack4() throws Exception {
+		createFiles("x/a/a/b/foo.txt", "x/y/z/b/a/b/foo.txt",
+				"x/y/a/a/a/a/b/foo.txt", "x/y/a/a/a/a/b/a/b/foo.txt");
+		writeTrashFile(".gitignore", "**/*/a/b bar\n");
+		assertSameAsCGit();
+	}
+
+	@Test
+	public void testDirectoryMatchSubRecursiveBacktrack5() throws Exception {
+		createFiles("x/a/a/b/foo.txt", "x/y/a/b/a/b/foo.txt");
+		writeTrashFile(".gitignore", "**/*/**/a/b bar\n");
+		assertSameAsCGit();
+	}
+
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
index 85073ecfb2..9b3a2aac78 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
@@ -225,6 +225,11 @@ public class PathMatcher extends AbstractMatcher {
 		int right = startIncl;
 		boolean match = false;
 		int lastWildmatch = -1;
+		// ** matches may get extended if a later match fails. When that
+		// happens, we must extend the ** by exactly one segment.
+		// wildmatchBacktrackPos records the end of the segment after a **
+		// match, so that we can reset correctly.
+		int wildmatchBacktrackPos = -1;
 		while (true) {
 			int left = right;
 			right = path.indexOf(slash, right);
@@ -250,6 +255,9 @@ public class PathMatcher extends AbstractMatcher {
 				}
 				return match && matcher + 1 == matchers.size();
 			}
+			if (wildmatchBacktrackPos < 0) {
+				wildmatchBacktrackPos = right;
+			}
 			if (right - left > 0) {
 				match = matches(matcher, path, left, right, assumeDirectory);
 			} else {
@@ -261,6 +269,7 @@ public class PathMatcher extends AbstractMatcher {
 				boolean wasWild = matchers.get(matcher) == WILD;
 				if (wasWild) {
 					lastWildmatch = matcher;
+					wildmatchBacktrackPos = -1;
 					// ** can match *nothing*: a/**/b match also a/b
 					right = left - 1;
 				}
@@ -276,11 +285,25 @@ public class PathMatcher extends AbstractMatcher {
 							return !dirOnly || assumeDirectory;
 						}
 						// Prefix matches only if pattern ended with /**
-						return wasWild;
+						if (wasWild) {
+							return true;
+						}
+						if (lastWildmatch >= 0) {
+							// Consider pattern **/x and input x/x.
+							// We've matched the prefix x/ so far: we
+							// must try to extend the **!
+							matcher = lastWildmatch + 1;
+							right = wildmatchBacktrackPos;
+							wildmatchBacktrackPos = -1;
+						} else {
+							return false;
+						}
 					}
 				}
 			} else if (lastWildmatch != -1) {
 				matcher = lastWildmatch + 1;
+				right = wildmatchBacktrackPos;
+				wildmatchBacktrackPos = -1;
 			} else {
 				return false;
 			}