From 49cb6ba5dd337dfdc9302bb33248e8448f530d3d Mon Sep 17 00:00:00 2001 From: Marc Strapetz Date: Sat, 17 Feb 2018 13:20:57 +0100 Subject: [PATCH] PathMatcher: fix handling of **/ **/ should match only directories, but not files Change-Id: I885c83e5912cac5bff338ba657faf6bb9ec94064 Signed-off-by: Marc Strapetz --- .../jgit/attributes/CGitAttributesTest.java | 28 +++++++++ .../eclipse/jgit/ignore/CGitIgnoreTest.java | 28 +++++++++ .../jgit/ignore/FastIgnoreRuleTest.java | 49 +++++++++++++++- .../eclipse/jgit/ignore/FastIgnoreRule.java | 26 ++++++++- .../jgit/ignore/internal/IMatcher.java | 9 +-- .../internal/LeadingAsteriskMatcher.java | 3 +- .../jgit/ignore/internal/NameMatcher.java | 9 ++- .../jgit/ignore/internal/PathMatcher.java | 58 ++++++++++++------- .../internal/TrailingAsteriskMatcher.java | 3 +- .../jgit/ignore/internal/WildCardMatcher.java | 3 +- .../jgit/ignore/internal/WildMatcher.java | 16 ++--- 11 files changed, 185 insertions(+), 47 deletions(-) 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 34838138e3..344d1af6a0 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 @@ -365,6 +365,34 @@ public class CGitAttributesTest extends RepositoryTestCase { assertSameAsCGit(); } + @Test + public void testDirectoryWildmatchDoesNotMatchFiles1() throws Exception { + createFiles("a", "dir/b", "dir/sub/c"); + writeTrashFile(".gitattributes", "**/ bar\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles2() throws Exception { + createFiles("a", "dir/b", "dir/sub/c"); + writeTrashFile(".gitattributes", "**/**/ bar\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles3() throws Exception { + createFiles("a", "x/b", "sub/x/c", "sub/x/d/e"); + writeTrashFile(".gitattributes", "x/**/ bar\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles4() throws Exception { + createFiles("a", "dir/x", "dir/sub1/x", "dir/sub2/x/y"); + writeTrashFile(".gitattributes", "x/**/ 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 aa42e4943b..155bd27303 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 @@ -266,6 +266,34 @@ public class CGitIgnoreTest extends RepositoryTestCase { assertSameAsCGit(); } + @Test + public void testDirectoryWildmatchDoesNotMatchFiles1() throws Exception { + createFiles("a", "dir/b", "dir/sub/c"); + writeTrashFile(".gitignore", "**/\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles2() throws Exception { + createFiles("a", "dir/b", "dir/sub/c"); + writeTrashFile(".gitignore", "**/**/\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles3() throws Exception { + createFiles("a", "x/b", "sub/x/c", "sub/x/d/e"); + writeTrashFile(".gitignore", "x/**/\n"); + assertSameAsCGit(); + } + + @Test + public void testDirectoryWildmatchDoesNotMatchFiles4() throws Exception { + createFiles("a", "dir/x", "dir/sub1/x", "dir/sub2/x/y"); + writeTrashFile(".gitignore", "**/x/\n"); + assertSameAsCGit(); + } + @Test public void testUnescapedBracketsInGroup() throws Exception { createFiles("[", "]", "[]", "][", "[[]", "[]]", "[[]]"); diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java index 06164c8a91..2a1721e66c 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/FastIgnoreRuleTest.java @@ -48,10 +48,18 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import org.junit.Before; import org.junit.Test; public class FastIgnoreRuleTest { + private boolean pathMatch; + + @Before + public void setup() { + pathMatch = false; + } + @Test public void testSimpleCharClass() { assertMatched("][a]", "]a"); @@ -410,6 +418,19 @@ public class FastIgnoreRuleTest { assertMatched("a/**/b/**/c", "a/c/b/d/c"); assertMatched("a/**/**/b/**/**/c", "a/c/b/d/c"); + + assertMatched("**/", "a/"); + assertMatched("**/", "a/b"); + assertMatched("**/", "a/b/c"); + assertMatched("**/**/", "a/"); + assertMatched("**/**/", "a/b"); + assertMatched("**/**/", "a/b/"); + assertMatched("**/**/", "a/b/c"); + assertMatched("x/**/", "x/a/"); + assertMatched("x/**/", "x/a/b"); + assertMatched("x/**/", "x/a/b/"); + assertMatched("**/x/", "a/x/"); + assertMatched("**/x/", "a/b/x/"); } @Test @@ -424,6 +445,10 @@ public class FastIgnoreRuleTest { assertNotMatched("!/**/*.zip", "c/a/b.zip"); assertNotMatched("!**/*.zip", "c/a/b.zip"); assertNotMatched("a/**/b", "a/c/bb"); + + assertNotMatched("**/", "a"); + assertNotMatched("**/**/", "a"); + assertNotMatched("**/x/", "a/b/x"); } @SuppressWarnings("unused") @@ -465,6 +490,28 @@ public class FastIgnoreRuleTest { split("/a/b/c/", '/').toArray()); } + @Test + public void testPathMatch() { + pathMatch = true; + assertMatched("a", "a"); + assertMatched("a/", "a/"); + assertNotMatched("a/", "a/b"); + + assertMatched("**", "a"); + assertMatched("**", "a/"); + assertMatched("**", "a/b"); + + assertNotMatched("**/", "a"); + assertNotMatched("**/", "a/b"); + assertMatched("**/", "a/"); + assertMatched("**/", "a/b/"); + + assertNotMatched("x/**/", "x/a"); + assertNotMatched("x/**/", "x/a/b"); + assertMatched("x/**/", "x/a/"); + assertMatched("x/**/", "x/y/a/"); + } + private void assertMatched(String pattern, String path) { boolean match = match(pattern, path); String result = path + " is " + (match ? "ignored" : "not ignored") @@ -520,7 +567,7 @@ public class FastIgnoreRuleTest { FastIgnoreRule r = new FastIgnoreRule(pattern); // If speed of this test is ever an issue, we can use a presetRule field // to avoid recompiling a pattern each time. - boolean match = r.isMatch(target, isDirectory); + boolean match = r.isMatch(target, isDirectory, pathMatch); if (r.getNegation()) match = !match; return match; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/FastIgnoreRule.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/FastIgnoreRule.java index 460f0ed4b1..31e173bf23 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/FastIgnoreRule.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/FastIgnoreRule.java @@ -152,11 +152,35 @@ public class FastIgnoreRule { * result. */ public boolean isMatch(String path, boolean directory) { + return isMatch(path, directory, false); + } + + /** + * Returns true if a match was made.
+ * This function does NOT return the actual ignore status of the target! + * Please consult {@link #getResult()} for the negation status. The actual + * ignore status may be true or false depending on whether this rule is an + * ignore rule or a negation rule. + * + * @param path + * Name pattern of the file, relative to the base directory of + * this rule + * @param directory + * Whether the target file is a directory or not + * @param pathMatch + * {@code true} if the match is for the full path: see + * {@link IMatcher#matches(String, int, int)} + * @return True if a match was made. This does not necessarily mean that the + * target is ignored. Call {@link #getResult() getResult()} for the + * result. + * @since 4.11 + */ + public boolean isMatch(String path, boolean directory, boolean pathMatch) { if (path == null) return false; if (path.length() == 0) return false; - boolean match = matcher.matches(path, directory, false); + boolean match = matcher.matches(path, directory, pathMatch); return match; } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java index dbf06385c5..14440d253d 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java @@ -58,8 +58,7 @@ public interface IMatcher { } @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { return false; } }; @@ -91,11 +90,7 @@ public interface IMatcher { * start index, inclusive * @param endExcl * end index, exclusive - * @param assumeDirectory - * true to assume this path as directory (even if it doesn't end - * with a slash) * @return true if this matcher pattern matches given string */ - boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory); + boolean matches(String segment, int startIncl, int endExcl); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/LeadingAsteriskMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/LeadingAsteriskMatcher.java index 2bae8f229f..fdd39bcc79 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/LeadingAsteriskMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/LeadingAsteriskMatcher.java @@ -57,8 +57,7 @@ public class LeadingAsteriskMatcher extends NameMatcher { /** {@inheritDoc} */ @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { // faster local access, same as in string.indexOf() String s = subPattern; diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java index 6a4b2b8677..33f0fe71d2 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java @@ -91,12 +91,12 @@ public class NameMatcher extends AbstractMatcher { } boolean match; if (lastSlash < start) { - match = matches(path, start, stop, assumeDirectory); + match = matches(path, start, stop); } else { // Can't match if the path contains a slash if the pattern is // anchored at the beginning match = !beginning - && matches(path, lastSlash + 1, stop, assumeDirectory); + && matches(path, lastSlash + 1, stop); } if (match && dirOnly) { match = assumeDirectory; @@ -108,7 +108,7 @@ public class NameMatcher extends AbstractMatcher { if (end < 0) { end = stop; } - if (end > start && matches(path, start, end, assumeDirectory)) { + if (end > start && matches(path, start, end)) { // make sure the directory matches: either if we are done with // segment and there is next one, or if the directory is assumed return !dirOnly || assumeDirectory || end < stop; @@ -123,8 +123,7 @@ public class NameMatcher extends AbstractMatcher { /** {@inheritDoc} */ @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { // faster local access, same as in string.indexOf() String s = subPattern; int length = s.length(); 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 50e78ae751..3c0f17ab3d 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 @@ -61,7 +61,10 @@ import org.eclipse.jgit.ignore.internal.Strings.PatternState; */ public class PathMatcher extends AbstractMatcher { - private static final WildMatcher WILD = WildMatcher.INSTANCE; + private static final WildMatcher WILD_NO_DIRECTORY = new WildMatcher(false); + + private static final WildMatcher WILD_ONLY_DIRECTORY = new WildMatcher( + true); private final List matchers; @@ -94,11 +97,15 @@ public class PathMatcher extends AbstractMatcher { for (int i = 0; i < segments.size(); i++) { String segment = segments.get(i); IMatcher matcher = createNameMatcher0(segment, pathSeparator, - dirOnly); - if (matcher == WILD && i > 0 - && matchers.get(matchers.size() - 1) == WILD) - // collapse wildmatchers **/** is same as ** - continue; + dirOnly, i == segments.size() - 1); + if (i > 0) { + final IMatcher last = matchers.get(matchers.size() - 1); + if (isWild(matcher) && isWild(last)) + // collapse wildmatchers **/** is same as **, but preserve + // dirOnly flag (i.e. always use the last wildmatcher) + matchers.remove(matchers.size() - 1); + } + matchers.add(matcher); } return matchers; @@ -126,7 +133,7 @@ public class PathMatcher extends AbstractMatcher { int slashIdx = pattern.indexOf(slash, 1); if (slashIdx > 0 && slashIdx < pattern.length() - 1) return new PathMatcher(pattern, pathSeparator, dirOnly); - return createNameMatcher0(pattern, pathSeparator, dirOnly); + return createNameMatcher0(pattern, pathSeparator, dirOnly, true); } /** @@ -153,12 +160,13 @@ public class PathMatcher extends AbstractMatcher { } private static IMatcher createNameMatcher0(String segment, - Character pathSeparator, boolean dirOnly) + Character pathSeparator, boolean dirOnly, boolean lastSegment) throws InvalidPatternException { // check if we see /** or ** segments => double star pattern if (WildMatcher.WILDMATCH.equals(segment) || WildMatcher.WILDMATCH2.equals(segment)) - return WILD; + return dirOnly && lastSegment ? WILD_ONLY_DIRECTORY + : WILD_NO_DIRECTORY; PatternState state = checkWildCards(segment); switch (state) { @@ -218,8 +226,7 @@ public class PathMatcher extends AbstractMatcher { /** {@inheritDoc} */ @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { throw new UnsupportedOperationException( "Path matcher works only on entire paths"); //$NON-NLS-1$ } @@ -241,18 +248,18 @@ public class PathMatcher extends AbstractMatcher { if (right == -1) { if (left < endExcl) { match = matches(matcher, path, left, endExcl, - assumeDirectory); + assumeDirectory, pathMatch); } else { // a/** should not match a/ or a - match = match && matchers.get(matcher) != WILD; + match = match && !isWild(matchers.get(matcher)); } if (match) { if (matcher < matchers.size() - 1 - && matchers.get(matcher) == WILD) { + && isWild(matchers.get(matcher))) { // ** can match *nothing*: a/**/b match also a/b matcher++; match = matches(matcher, path, left, endExcl, - assumeDirectory); + assumeDirectory, pathMatch); } else if (dirOnly && !assumeDirectory) { // Directory expectations not met return false; @@ -264,14 +271,15 @@ public class PathMatcher extends AbstractMatcher { wildmatchBacktrackPos = right; } if (right - left > 0) { - match = matches(matcher, path, left, right, assumeDirectory); + match = matches(matcher, path, left, right, assumeDirectory, + pathMatch); } else { // path starts with slash??? right++; continue; } if (match) { - boolean wasWild = matchers.get(matcher) == WILD; + boolean wasWild = isWild(matchers.get(matcher)); if (wasWild) { lastWildmatch = matcher; wildmatchBacktrackPos = -1; @@ -317,9 +325,19 @@ public class PathMatcher extends AbstractMatcher { } private boolean matches(int matcherIdx, String path, int startIncl, - int endExcl, - boolean assumeDirectory) { + int endExcl, boolean assumeDirectory, boolean pathMatch) { IMatcher matcher = matchers.get(matcherIdx); - return matcher.matches(path, startIncl, endExcl, assumeDirectory); + + final boolean matches = matcher.matches(path, startIncl, endExcl); + if (!matches || !pathMatch || matcherIdx < matchers.size() - 1 + || !(matcher instanceof AbstractMatcher)) { + return matches; + } + + return assumeDirectory || !((AbstractMatcher) matcher).dirOnly; + } + + private static boolean isWild(IMatcher matcher) { + return matcher == WILD_NO_DIRECTORY || matcher == WILD_ONLY_DIRECTORY; } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/TrailingAsteriskMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/TrailingAsteriskMatcher.java index 7df42b76cf..1e0335cef8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/TrailingAsteriskMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/TrailingAsteriskMatcher.java @@ -57,8 +57,7 @@ public class TrailingAsteriskMatcher extends NameMatcher { /** {@inheritDoc} */ @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { // faster local access, same as in string.indexOf() String s = subPattern; // we don't need to count '*' character itself diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildCardMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildCardMatcher.java index ca8c581d4b..3bbac8189f 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildCardMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildCardMatcher.java @@ -66,8 +66,7 @@ public class WildCardMatcher extends NameMatcher { /** {@inheritDoc} */ @Override - public boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public boolean matches(String segment, int startIncl, int endExcl) { return p.matcher(segment.substring(startIncl, endExcl)).matches(); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java index ba8015cc09..2ad87dade6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java @@ -55,24 +55,26 @@ public final class WildMatcher extends AbstractMatcher { // double star for the beginning of pattern static final String WILDMATCH2 = "/**"; //$NON-NLS-1$ - static final WildMatcher INSTANCE = new WildMatcher(); - - private WildMatcher() { - super(WILDMATCH, false); + WildMatcher(boolean dirOnly) { + super(WILDMATCH, dirOnly); } /** {@inheritDoc} */ @Override public final boolean matches(String path, boolean assumeDirectory, boolean pathMatch) { - return true; + return !dirOnly || assumeDirectory + || !pathMatch && isSubdirectory(path); } /** {@inheritDoc} */ @Override - public final boolean matches(String segment, int startIncl, int endExcl, - boolean assumeDirectory) { + public final boolean matches(String segment, int startIncl, int endExcl) { return true; } + private static boolean isSubdirectory(String path) { + final int slashIndex = path.indexOf('/'); + return slashIndex >= 0 && slashIndex < path.length() - 1; + } } -- 2.39.5