From d031b6466757ddaa3ef453d4b9d385b0d35d7ab1 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Sun, 13 Aug 2017 15:08:16 +0200 Subject: [PATCH] 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 --- .../attributes/AttributesHandlerTest.java | 44 ++++++++++++++++++ .../jgit/attributes/CGitAttributesTest.java | 45 +++++++++++++++++++ .../eclipse/jgit/ignore/CGitIgnoreTest.java | 37 +++++++++++++++ .../jgit/ignore/internal/PathMatcher.java | 25 ++++++++++- 4 files changed, 150 insertions(+), 1 deletion(-) 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 0717379e2..5868482c8 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 @@ -451,6 +451,50 @@ public void testDirectoryMatchSubRecursive() throws Exception { endWalk(); } + @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); 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 c8ff6c875..6188fa60c 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 @@ -319,6 +319,51 @@ public void testDirectoryMatchSubRecursive() throws Exception { assertSameAsCGit(); } + @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"); 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 3ea894f49..c61f2ea7f 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 void testDirectoryMatchSubRecursive() throws Exception { 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 85073ecfb..9b3a2aac7 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 @@ private boolean iterate(final String path, final int startIncl, 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 @@ private boolean iterate(final String path, final int startIncl, } 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 @@ private boolean iterate(final String path, final int startIncl, 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 @@ private boolean iterate(final String path, final int startIncl, 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; }