From 803a137bc12b2010cd740d2f6726c4a3c10fb422 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Thu, 22 Aug 2019 20:13:29 +0200 Subject: [PATCH 1/2] Add unit tests Signed-off-by: Robert Stupp --- .../pattern/PatternMatcherFactoryTest.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/subprojects/files/src/test/groovy/org/gradle/api/internal/file/pattern/PatternMatcherFactoryTest.java b/subprojects/files/src/test/groovy/org/gradle/api/internal/file/pattern/PatternMatcherFactoryTest.java index 22e6f580012e..c6aa26cc821c 100644 --- a/subprojects/files/src/test/groovy/org/gradle/api/internal/file/pattern/PatternMatcherFactoryTest.java +++ b/subprojects/files/src/test/groovy/org/gradle/api/internal/file/pattern/PatternMatcherFactoryTest.java @@ -22,6 +22,9 @@ import org.hamcrest.Matcher; import org.junit.Test; +import java.util.LinkedHashSet; +import java.util.Set; + import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertThat; @@ -29,6 +32,42 @@ public class PatternMatcherFactoryTest { private PatternMatcher matcher; + @Test public void testNoStackOverflowForManyPatterns() { + // The only reason for this unit test is to verify that no StackOverflowException is being thrown when + // many patterns are passed to getPatternsMatcher. See https://github.com/gradle/gradle/issues/10329 + Set manyPatterns = new LinkedHashSet(); + for (int i = 0; i < 5000; i++) { + manyPatterns.add("some/package/Some" + i + "ClassName.class"); + manyPatterns.add("some/package/Some" + i + "ClassName.java"); + manyPatterns.add("some/package/Some" + i + "ClassName.h"); + manyPatterns.add("some/package/Some" + i + "ClassName$*.class"); + manyPatterns.add("some/package/Some" + i + "ClassName$*.java"); + manyPatterns.add("some/package/Some" + i + "ClassName$*.h"); + } + matcher = PatternMatcherFactory.getPatternsMatcher(true, true, manyPatterns); + assertThat(matcher, not(matchesFile("some", "package", "SomeClassName.java"))); + assertThat(matcher, matchesFile("some", "package", "Some123ClassName.java")); + } + + @Test public void testManyOr() { + Set manyPatterns = new LinkedHashSet(); + manyPatterns.add("some/package/SomeClassName.class"); + manyPatterns.add("some/package/SomeClassName.java"); + manyPatterns.add("some/package/SomeClassName.h"); + manyPatterns.add("some/package/SomeClassName$*.class"); + manyPatterns.add("some/package/SomeClassName$*.java"); + manyPatterns.add("some/package/SomeClassName$*.h"); + matcher = PatternMatcherFactory.getPatternsMatcher(true, true, manyPatterns); + assertThat(matcher, not(matchesFile())); + assertThat(matcher, matchesFile("some", "package", "SomeClassName.java")); + assertThat(matcher, matchesFile("some", "package", "SomeClassName.class")); + assertThat(matcher, matchesFile("some", "package", "SomeClassName.h")); + assertThat(matcher, matchesFile("some", "package", "SomeClassName$*.java")); + assertThat(matcher, matchesFile("some", "package", "SomeClassName$*.class")); + assertThat(matcher, matchesFile("some", "package", "SomeClassName$*.h")); + assertThat(matcher, not(matchesFile("a"))); + } + @Test public void testEmpty() { matcher = PatternMatcherFactory.getPatternMatcher(true, true, ""); assertThat(matcher, matchesFile()); From 1351cacc09389f78d7b57cd2716d24499800053f Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Wed, 21 Aug 2019 13:16:38 +0200 Subject: [PATCH 2/2] Prevent StackOverflowException caused by excessive 'or' via PatternMatcher Signed-off-by: Robert Stupp --- .../internal/file/pattern/PatternMatcher.java | 67 +++++++++++++++---- 1 file changed, 55 insertions(+), 12 deletions(-) diff --git a/subprojects/files/src/main/java/org/gradle/api/internal/file/pattern/PatternMatcher.java b/subprojects/files/src/main/java/org/gradle/api/internal/file/pattern/PatternMatcher.java index 352cb46f8740..020bd62edfa6 100644 --- a/subprojects/files/src/main/java/org/gradle/api/internal/file/pattern/PatternMatcher.java +++ b/subprojects/files/src/main/java/org/gradle/api/internal/file/pattern/PatternMatcher.java @@ -16,6 +16,9 @@ package org.gradle.api.internal.file.pattern; +import java.util.ArrayList; +import java.util.List; + public abstract class PatternMatcher { public static final PatternMatcher MATCH_ALL = new PatternMatcher() { @Override @@ -37,21 +40,11 @@ public PatternMatcher or(PatternMatcher other) { public abstract boolean test(String[] segments, boolean isFile); public PatternMatcher and(final PatternMatcher other) { - return new PatternMatcher() { - @Override - public boolean test(String[] segments, boolean isFile) { - return PatternMatcher.this.test(segments, isFile) && other.test(segments, isFile); - } - }; + return new And(PatternMatcher.this, other); } public PatternMatcher or(final PatternMatcher other) { - return new PatternMatcher() { - @Override - public boolean test(String[] segments, boolean isFile) { - return PatternMatcher.this.test(segments, isFile) || other.test(segments, isFile); - } - }; + return new Or(PatternMatcher.this, other); } public PatternMatcher negate() { @@ -62,4 +55,54 @@ public boolean test(String[] segments, boolean isFile) { } }; } + + private static final class Or extends PatternMatcher { + private final List parts = new ArrayList(); + + public Or(PatternMatcher patternMatcher, PatternMatcher other) { + parts.add(patternMatcher); + parts.add(other); + } + + @Override + public PatternMatcher or(PatternMatcher other) { + parts.add(other); + return this; + } + + @Override + public boolean test(String[] segments, boolean isFile) { + for (PatternMatcher part : parts) { + if (part.test(segments, isFile)) { + return true; + } + } + return false; + } + } + + private static final class And extends PatternMatcher { + private final List parts = new ArrayList(); + + public And(PatternMatcher patternMatcher, PatternMatcher other) { + parts.add(patternMatcher); + parts.add(other); + } + + @Override + public PatternMatcher and(PatternMatcher other) { + parts.add(other); + return this; + } + + @Override + public boolean test(String[] segments, boolean isFile) { + for (PatternMatcher part : parts) { + if (!part.test(segments, isFile)) { + return false; + } + } + return true; + } + } }