diff options
3 files changed, 71 insertions, 37 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java index 77cc3d1601..5b0f8c4db2 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelValidator.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.cmdline; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.CharMatcher; import java.util.Objects; @@ -49,8 +50,13 @@ public final class LabelValidator { .or(PUNCTUATION_REQUIRING_QUOTING) .or(PUNCTUATION_NOT_REQUIRING_QUOTING); - private static final String PACKAGE_NAME_ERROR = - "package names may contain only A-Z, a-z, 0-9, '/', '-' and '_'"; + @VisibleForTesting + static final String PACKAGE_NAME_ERROR = + "package names may contain only A-Z, a-z, 0-9, '/', '-', '.' and '_'"; + + @VisibleForTesting + static final String PACKAGE_NAME_DOT_ERROR = + "package name component contains only '.' characters"; /** * Performs validity checking of the specified package name. Returns null on success or an error @@ -72,28 +78,43 @@ public final class LabelValidator { return "package names may not start with '/'"; } - // Fast path for packages with '.' in their name - if (packageName.lastIndexOf('.') != -1) { - return PACKAGE_NAME_ERROR; - } - - // Check for any character outside of [/0-9A-Za-z_-]. Try to evaluate the + // Check for any character outside of [/0-9.A-Za-z_-]. Try to evaluate the // conditional quickly (by looking in decreasing order of character class - // likelihood). - for (int i = len - 1; i >= 0; --i) { - char c = packageName.charAt(i); - if ((c < 'a' || c > 'z') && c != '/' && c != '_' && c != '-' && - (c < '0' || c > '9') && (c < 'A' || c > 'Z')) { + // likelihood). To deal with . and .. pretend that the name is surrounded by '/' + // on both sides. + boolean nonDot = false; + int lastSlash = len; + for (int i = len - 1; i >= -1; --i) { + char c = (i >= 0) ? packageName.charAt(i) : '/'; + if ((c < 'a' || c > 'z') + && c != '/' + && c != '_' + && c != '-' + && c != '.' + && (c < '0' || c > '9') + && (c < 'A' || c > 'Z')) { return PACKAGE_NAME_ERROR; } - } - if (packageName.contains("//")) { - return "package names may not contain '//' path separators"; - } - if (packageName.endsWith("/")) { - return "package names may not end with '/'"; + if (c == '/') { + if (lastSlash == i + 1) { + return lastSlash == len + ? "package names may not end with '/'" + : "package names may not contain '//' path separators"; + } + + if (!nonDot) { + return PACKAGE_NAME_DOT_ERROR; + } + nonDot = false; + lastSlash = i; + } else { + if (c != '.') { + nonDot = true; + } + } } + return null; // ok } diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java index daf5853534..feceda346d 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelValidatorTest.java @@ -33,9 +33,6 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class LabelValidatorTest { - private static final String BAD_PACKAGE_CHARS = - "package names may contain only A-Z, a-z, 0-9, '/', '-' and '_'"; - private PackageAndTarget newFooTarget() { return new PackageAndTarget("foo", "foo"); } @@ -54,22 +51,38 @@ public class LabelValidatorTest { assertNull(LabelValidator.validatePackageName("foo-bar")); assertNull(LabelValidator.validatePackageName("Foo-Bar")); assertNull(LabelValidator.validatePackageName("FOO-BAR")); + assertNull(LabelValidator.validatePackageName("bar.baz")); + assertNull(LabelValidator.validatePackageName("a/..b")); + assertNull(LabelValidator.validatePackageName("a/.b")); + assertNull(LabelValidator.validatePackageName("a/b.")); + assertNull(LabelValidator.validatePackageName("a/b..")); // Bad: - assertEquals("package names may not start with '/'", - LabelValidator.validatePackageName("/foo")); - assertEquals("package names may not end with '/'", - LabelValidator.validatePackageName("foo/")); - assertEquals(BAD_PACKAGE_CHARS, - LabelValidator.validatePackageName("bar baz")); - assertEquals(BAD_PACKAGE_CHARS, - LabelValidator.validatePackageName("foo:bar")); - assertEquals(BAD_PACKAGE_CHARS, - LabelValidator.validatePackageName("baz@12345")); - assertEquals(BAD_PACKAGE_CHARS, - LabelValidator.validatePackageName("baz(foo)")); - assertEquals(BAD_PACKAGE_CHARS, - LabelValidator.validatePackageName("bazfoo)")); + assertEquals( + "package names may not start with '/'", LabelValidator.validatePackageName("/foo")); + assertEquals("package names may not end with '/'", LabelValidator.validatePackageName("foo/")); + assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("bar baz")); + assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("foo:bar")); + assertEquals( + LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("baz@12345")); + assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("baz(foo)")); + assertEquals(LabelValidator.PACKAGE_NAME_ERROR, LabelValidator.validatePackageName("bazfoo)")); + + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/../baz")); + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/..")); + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("../bar")); + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/...")); + + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/./baz")); + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("bar/.")); + assertEquals( + LabelValidator.PACKAGE_NAME_DOT_ERROR, LabelValidator.validatePackageName("./bar")); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java index 8efb04086a..dd62d999bd 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java @@ -57,7 +57,7 @@ public class TargetPatternTest { @Test public void testInvalidPatterns() throws TargetParsingException { try { - parse("Bar.java"); + parse("Bar&&&java"); fail(); } catch (TargetParsingException expected) { } |