PathMatcher: fix handling of **/

**/ should match only directories, but not files

Change-Id: I885c83e5912cac5bff338ba657faf6bb9ec94064
Signed-off-by: Marc Strapetz <marc.strapetz@syntevo.com>
This commit is contained in:
Marc Strapetz 2018-02-17 13:20:57 +01:00 committed by David Pursehouse
parent 0e20df710a
commit 49cb6ba5dd
11 changed files with 185 additions and 47 deletions

View File

@ -365,6 +365,34 @@ public void testDirectoryMatchSubRecursiveBacktrack6() throws Exception {
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");

View File

@ -266,6 +266,34 @@ public void testDirectoryMatchSubRecursiveBacktrack5() throws Exception {
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("[", "]", "[]", "][", "[[]", "[]]", "[[]]");

View File

@ -48,10 +48,18 @@
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 void testWildmatch() {
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 void testWildmatchDoNotMatch() {
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 void testSplit() {
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 @@ private boolean match(String pattern, String target) {
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;

View File

@ -152,11 +152,35 @@ public FastIgnoreRule(String pattern) {
* 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;
}

View File

@ -58,8 +58,7 @@ public boolean matches(String path, boolean assumeDirectory,
}
@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 boolean matches(String segment, int startIncl, int endExcl,
* 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);
}

View File

@ -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;

View File

@ -91,12 +91,12 @@ public boolean matches(String path, boolean assumeDirectory,
}
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 boolean matches(String path, boolean assumeDirectory,
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 boolean matches(String path, boolean assumeDirectory,
/** {@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();

View File

@ -61,7 +61,10 @@
*/
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 @@ private static List<IMatcher> createMatchers(List<String> segments,
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 static IMatcher createPathMatcher(String pattern,
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 @@ private static String trim(String pattern) {
}
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 @@ private boolean simpleMatch(String path, boolean assumeDirectory,
/** {@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 @@ private boolean iterate(final String path, final int startIncl,
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 @@ private boolean iterate(final String path, final int startIncl,
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 @@ private boolean iterate(final String path, final int startIncl,
}
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;
}
}

View File

@ -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

View File

@ -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();
}
}

View File

@ -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;
}
}