PathMatcher: fix handling of **/

**/ should match only directories, but not files

Change-Id: I885c83e5912cac5bff338ba657faf6bb9ec94064
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>

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
@@ -366,6 +366,34 @@ public class CGitAttributesTest extends RepositoryTestCase {
 	}
 
 	@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");
 		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 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
@@ -267,6 +267,34 @@ public class CGitIgnoreTest extends RepositoryTestCase {
 	}
 
 	@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("[", "]", "[]", "][", "[[]", "[]]", "[[]]");
 		writeTrashFile(".gitignore", "[[]]\n");
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. <br>
+	 * 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<IMatcher> 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;
+	}
 }