PathMatcher should respect "assumeDirectory" flag

The path matcher should not fail if the rule ends with trailing slash,
target pattern does not ends with the slash and the "assumeDirectory"
flag is set.

E.g. */bin/ should also match a/bin if this pattern is threated as
directory by WorkingTreeIterator (FileMode.TREE).

The old code/tests have never tested directory rules with patterns
*without* trailing slashes but with the "assumeDirectory" flag set.
Unfortunately this is exactly what WorkingTreeIterator does... The tests
are changed to test *both* cases now (with trailing slash and without)
if the target pattern has trailing slash (represents directory).

Bug: 454672
Change-Id: I621c1644d9e94df3eb9f6f09c6de0fe51f0950a4
Also-by: Markus Duft <markus.duft@salomon.at>
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

2 files changed, 103 insertions, 15 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java
index cbfe6f2790..699542c57f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/ignore/IgnoreMatcherParametrizedTest.java
@@ -44,9 +44,12 @@ package org.eclipse.jgit.ignore;
 
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
 
 import java.util.Arrays;
 
+import org.eclipse.jgit.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -236,17 +239,63 @@ public class IgnoreMatcherParametrizedTest {
 	}
 
 	@Test
-	public void testTrailingSlash() {
+	public void testDirModeAndNoRegex() {
 		String pattern = "/src/";
 		assertMatched(pattern, "/src/");
 		assertMatched(pattern, "/src/new");
 		assertMatched(pattern, "/src/new/a.c");
 		assertMatched(pattern, "/src/a.c");
+		// no match as a "file" pattern, because rule is for directories only
 		assertNotMatched(pattern, "/src");
 		assertNotMatched(pattern, "/srcA/");
 	}
 
 	@Test
+	public void testDirModeAndRegex1() {
+		// IgnoreRule was buggy for some cases below, therefore using "Assume"
+		Boolean assume = useOldRule;
+
+		String pattern = "a/*/src/";
+		assertMatched(pattern, "a/b/src/");
+		assertMatched(pattern, "a/b/src/new");
+		assertMatched(pattern, "a/b/src/new/a.c");
+		assertMatched(pattern, "a/b/src/a.c");
+		// no match as a "file" pattern, because rule is for directories only
+		assertNotMatched(pattern, "a/b/src", assume);
+		assertNotMatched(pattern, "a/b/srcA/");
+	}
+
+	@Test
+	public void testDirModeAndRegex2() {
+		// IgnoreRule was buggy for some cases below, therefore using "Assume"
+		Boolean assume = useOldRule;
+
+		String pattern = "a/[a-b]/src/";
+		assertMatched(pattern, "a/b/src/");
+		assertMatched(pattern, "a/b/src/new");
+		assertMatched(pattern, "a/b/src/new/a.c");
+		assertMatched(pattern, "a/b/src/a.c");
+		// no match as a "file" pattern, because rule is for directories only
+		assertNotMatched(pattern, "a/b/src", assume);
+		assertNotMatched(pattern, "a/b/srcA/");
+	}
+
+	@Test
+	public void testDirModeAndRegex3() {
+		// IgnoreRule was buggy for some cases below, therefore using "Assume"
+		Boolean assume = useOldRule;
+
+		String pattern = "**/src/";
+		assertMatched(pattern, "a/b/src/", assume);
+		assertMatched(pattern, "a/b/src/new", assume);
+		assertMatched(pattern, "a/b/src/new/a.c", assume);
+		assertMatched(pattern, "a/b/src/a.c", assume);
+		// no match as a "file" pattern, because rule is for directories only
+		assertNotMatched(pattern, "a/b/src", assume);
+		assertNotMatched(pattern, "a/b/srcA/", assume);
+	}
+
+	@Test
 	public void testNameOnlyMatches() {
 		/*
 		 * Name-only matches do not contain any path separators
@@ -321,11 +370,16 @@ public class IgnoreMatcherParametrizedTest {
 	 *            Pattern as it would appear in a .gitignore file
 	 * @param target
 	 *            Target file path relative to repository's GIT_DIR
+	 * @param assume
 	 */
-	public void assertMatched(String pattern, String target) {
+	public void assertMatched(String pattern, String target, Boolean... assume) {
 		boolean value = match(pattern, target);
-		assertTrue("Expected a match for: " + pattern + " with: " + target,
-				value);
+		if (assume.length == 0 || !assume[0].booleanValue())
+			assertTrue("Expected a match for: " + pattern + " with: " + target,
+					value);
+		else
+			assumeTrue("Expected a match for: " + pattern + " with: " + target,
+					value);
 	}
 
 	/**
@@ -336,11 +390,17 @@ public class IgnoreMatcherParametrizedTest {
 	 *            Pattern as it would appear in a .gitignore file
 	 * @param target
 	 *            Target file path relative to repository's GIT_DIR
+	 * @param assume
 	 */
-	public void assertNotMatched(String pattern, String target) {
+	public void assertNotMatched(String pattern, String target,
+			Boolean... assume) {
 		boolean value = match(pattern, target);
-		assertFalse("Expected no match for: " + pattern + " with: " + target,
-				value);
+		if (assume.length == 0 || !assume[0].booleanValue())
+			assertFalse("Expected no match for: " + pattern + " with: "
+					+ target, value);
+		else
+			assumeFalse("Expected no match for: " + pattern + " with: "
+					+ target, value);
 	}
 
 	/**
@@ -355,16 +415,43 @@ public class IgnoreMatcherParametrizedTest {
 	 */
 	private boolean match(String pattern, String target) {
 		boolean isDirectory = target.endsWith("/");
+		boolean match;
+		if (useOldRule.booleanValue()) {
+			IgnoreRule r = new IgnoreRule(pattern);
+			match = r.isMatch(target, isDirectory);
+		} else {
+			FastIgnoreRule r = new FastIgnoreRule(pattern);
+			match = r.isMatch(target, isDirectory);
+		}
+
+		if (isDirectory) {
+			boolean noTrailingSlash = matchAsDir(pattern,
+					target.substring(0, target.length() - 1));
+			if (match != noTrailingSlash) {
+				String message = "Difference in result for directory pattern: "
+						+ pattern + " with: " + target
+						+ " if target is given without trailing slash";
+				Assert.assertEquals(message, match, noTrailingSlash);
+			}
+		}
+		return match;
+	}
+
+	/**
+	 *
+	 * @param target
+	 *            must not ends with a slash!
+	 * @param pattern
+	 *            same as {@link #match(String, String)}
+	 * @return same as {@link #match(String, String)}
+	 */
+	private boolean matchAsDir(String pattern, String target) {
+		assertFalse(target.endsWith("/"));
 		if (useOldRule.booleanValue()) {
 			IgnoreRule r = new IgnoreRule(pattern);
-			// If speed of this test is ever an issue, we can use a presetRule
-			// field
-			// to avoid recompiling a pattern each time.
-			return r.isMatch(target, isDirectory);
+			return r.isMatch(target, true);
 		}
 		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.
-		return r.isMatch(target, isDirectory);
+		return r.isMatch(target, true);
 	}
 }
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 dcecf303c4..d3e5f6a053 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
@@ -217,7 +217,8 @@ public class PathMatcher extends AbstractMatcher {
 						matcher++;
 						match = matches(matcher, path, left, endExcl,
 								assumeDirectory);
-					} else if (dirOnly)
+					} else if (dirOnly && !assumeDirectory)
+						// Directory expectations not met
 						return false;
 				}
 				return match && matcher + 1 == matchers.size();