diff options
9 files changed, 98 insertions, 35 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 035bfbbe4d..a2f419eb23 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 @@ -25,7 +25,7 @@ import javax.annotation.Nullable; public final class LabelValidator { /** Matches punctuation in target names which requires quoting in a blaze query. */ - private static final CharMatcher PUNCTUATION_REQUIRING_QUOTING = CharMatcher.anyOf("+,=~#"); + private static final CharMatcher PUNCTUATION_REQUIRING_QUOTING = CharMatcher.anyOf("+,=~# ()$"); /** * Matches punctuation in target names which doesn't require quoting in a blaze query. @@ -36,13 +36,13 @@ public final class LabelValidator { private static final CharMatcher PUNCTUATION_NOT_REQUIRING_QUOTING = CharMatcher.anyOf("_@-"); /** - * Matches characters allowed in package name (allowed are A-Z, a-z, 0-9, '/', '-', '.' and '_') + * Matches characters allowed in package name. */ private static final CharMatcher ALLOWED_CHARACTERS_IN_PACKAGE_NAME = CharMatcher.inRange('0', '9') .or(CharMatcher.inRange('a', 'z')) .or(CharMatcher.inRange('A', 'Z')) - .or(CharMatcher.anyOf("/-._")) + .or(CharMatcher.anyOf("/-._ $()")) .precomputed(); /** @@ -59,7 +59,7 @@ public final class LabelValidator { @VisibleForTesting static final String PACKAGE_NAME_ERROR = - "package names may contain only A-Z, a-z, 0-9, '/', '-', '.' and '_'"; + "package names may contain only A-Z, a-z, 0-9, '/', '-', '.', ' ', '$', '(', ')' and '_'"; @VisibleForTesting static final String PACKAGE_NAME_DOT_ERROR = diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java index 3766a76755..69805058d7 100644 --- a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java +++ b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java @@ -279,8 +279,6 @@ public class LabelTest { @Test public void testBadCharacters() throws Exception { - assertSyntaxError("package names may contain only", - "//foo/bar baz"); assertSyntaxError("target names may not contain ':'", "//foo:bar:baz"); assertSyntaxError("target names may not contain ':'", @@ -289,12 +287,6 @@ public class LabelTest { "//foo/bar::"); assertSyntaxError("target names may not contain '&'", "//foo:bar&"); - assertSyntaxError("target names may not contain '$'", - "//foo/bar:baz$a"); - assertSyntaxError("target names may not contain '('", - "//foo/bar:baz(foo)"); - assertSyntaxError("target names may not contain ')'", - "//foo/bar:bazfoo)"); // Warning: if these assertions are false, tools that assume that they can safely quote labels // may need to be fixed. Please consult with bazel-dev before loosening these restrictions. assertSyntaxError("target names may not contain '''", "//foo/bar:baz'foo"); @@ -357,6 +349,7 @@ public class LabelTest { Label.parseAbsolute("//package:foo.bar"); Label.parseAbsolute("//package:foo@bar"); Label.parseAbsolute("//package:foo~bar"); + Label.parseAbsolute("//$( ):$( )"); } /** 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 fcf887a5e2..17e6ba5a12 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 @@ -53,17 +53,15 @@ public class LabelValidatorTest { 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(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")); @@ -97,20 +95,12 @@ public class LabelValidatorTest { assertEquals("target names may not end with '/'", LabelValidator.validateTargetName("foo/")); - assertEquals("target names may not contain ' '", - LabelValidator.validateTargetName("bar baz")); assertEquals("target names may not contain ':'", LabelValidator.validateTargetName("bar:baz")); assertEquals("target names may not contain ':'", LabelValidator.validateTargetName("bar:")); assertEquals("target names may not contain '&'", LabelValidator.validateTargetName("bar&")); - assertEquals("target names may not contain '$'", - LabelValidator.validateTargetName("baz$a")); - assertEquals("target names may not contain '('", - LabelValidator.validateTargetName("baz(foo)")); - assertEquals("target names may not contain ')'", - LabelValidator.validateTargetName("bazfoo)")); } @Test @@ -122,6 +112,13 @@ public class LabelValidatorTest { LabelValidator.validateAbsoluteLabel("@repo//foo:bar")); assertEquals(new PackageAndTarget("foo", "bar"), LabelValidator.validateAbsoluteLabel("@//foo:bar")); + emptyPackage = new PackageAndTarget("", "b$() ar"); + assertEquals(emptyPackage, LabelValidator.validateAbsoluteLabel("//:b$() ar")); + assertEquals(emptyPackage, LabelValidator.validateAbsoluteLabel("@repo//:b$() ar")); + assertEquals(new PackageAndTarget("f$( )oo", "b$() ar"), + LabelValidator.validateAbsoluteLabel("@repo//f$( )oo:b$() ar")); + assertEquals(new PackageAndTarget("f$( )oo", "b$() ar"), + LabelValidator.validateAbsoluteLabel("@//f$( )oo:b$() ar")); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java index 978f203e86..c5b0d31544 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/BuildTypeTest.java @@ -294,14 +294,13 @@ public class BuildTypeTest { @Test public void testSelectorWrongType() throws Exception { ImmutableMap<String, String> input = ImmutableMap.of( - "//conditions:a", "not a label", - "//conditions:b", "also not a label", + "//conditions:a", "not a/../label", "//conditions:b", "also not a/../label", BuildType.Selector.DEFAULT_CONDITION_KEY, "whatever"); try { new Selector<>(input, null, currentRule, BuildType.LABEL); fail("Expected Selector instantiation to fail since the input isn't a selection of labels"); } catch (ConversionException e) { - assertThat(e.getMessage()).contains("invalid label 'not a label'"); + assertThat(e.getMessage()).contains("invalid label 'not a/../label'"); } } @@ -311,13 +310,13 @@ public class BuildTypeTest { @Test public void testSelectorKeyIsNotALabel() throws Exception { ImmutableMap<String, String> input = ImmutableMap.of( - "not a label", "//a:a", + "not a/../label", "//a:a", BuildType.Selector.DEFAULT_CONDITION_KEY, "whatever"); try { new Selector<>(input, null, currentRule, BuildType.LABEL); fail("Expected Selector instantiation to fail since the key isn't a label"); } catch (ConversionException e) { - assertThat(e.getMessage()).contains("invalid label 'not a label'"); + assertThat(e.getMessage()).contains("invalid label 'not a/../label'"); } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java index 836ed6c8da..3573827c81 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java @@ -114,13 +114,16 @@ public class PackageFactoryTest extends PackageFactoryTestBase { @Test public void testBadPackageName() throws Exception { try { - packages.createPackage("not even a legal label", emptyBuildFile("not even a legal label")); + // PathFragment parsing de-double slashes, and normalization of the path fragment removes + // up reference (/../), so use triple dot /.../ that will always be a forbidden package name. + packages.createPackage("not even a legal/.../label", + emptyBuildFile("not even a legal/.../label")); fail(); } catch (NoSuchPackageException e) { assertThat(e.getMessage()) .contains( - "no such package 'not even a legal label': " - + "illegal package name: 'not even a legal label' "); + "no such package 'not even a legal/.../label': " + + "illegal package name: 'not even a legal/.../label' "); } } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java index 2966fc8fd7..f8b3d16621 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java @@ -801,8 +801,8 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase { @Test public void testLabelGetRelativeSyntaxError() throws Exception { checkErrorContains( - "invalid target name 'bad syntax': target names may not contain ' '", - "Label('//foo:bar').relative('bad syntax')"); + "invalid target name 'bad//syntax': target names may not contain '//' path separators", + "Label('//foo:bar').relative('bad//syntax')"); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java index a4e42c7a32..26d9a3955b 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/TypeTest.java @@ -256,10 +256,10 @@ public class TypeTest { @Test public void testInvalidLabel() throws Exception { try { - BuildType.LABEL.convert("not a label", null, currentRule); + BuildType.LABEL.convert("not//a label", null, currentRule); fail(); } catch (Type.ConversionException e) { - MoreAsserts.assertContainsWordsWithQuotes(e.getMessage(), "not a label"); + MoreAsserts.assertContainsWordsWithQuotes(e.getMessage(), "not//a label"); } } diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 0e7355274c..38029e025c 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -82,6 +82,14 @@ sh_test( ) sh_test( + name = "bazel_random_characters_test", + size = "large", + srcs = ["bazel_random_characters_test.sh"], + data = [":test-deps"], + tags = ["nowindows"], +) + +sh_test( name = "bazel_java_test", size = "large", srcs = ["bazel_java_test.sh"], diff --git a/src/test/shell/bazel/bazel_random_characters_test.sh b/src/test/shell/bazel/bazel_random_characters_test.sh new file mode 100755 index 0000000000..59f4b89138 --- /dev/null +++ b/src/test/shell/bazel/bazel_random_characters_test.sh @@ -0,0 +1,63 @@ +#!/bin/bash +# +# Copyright 2016 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the examples provided in Bazel +# + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +function basic_glob_scenario_test_template() { + local chars="$1" + local pkg="pkg${chars}" + echo "chars = ${chars}, pkg = ${pkg}" + mkdir -p "${pkg}/resources" + cat >"${pkg}/BUILD" <<EOF +java_library(name = 'main', + resources = glob(["resources/**"]), + srcs = ['Main.java']) +EOF + + for i in $(seq 1 10); do + cat >"${pkg}/resources/file${chars}$i" <<EOF +file${chars}$i +EOF + done + + cat >"$pkg/Main.java" <<'EOF' +public class Main { + public static void main(String[] args) { + System.out.println("Hello, World!"); + } +} +EOF + + bazel build "//${pkg}:main" &>"${TEST_log}" \ + || fail "Failed to build java target" + + nb_files="$(unzip -l "bazel-bin/${pkg}/libmain.jar" \ + | grep -F "file${chars}" | tee "${TEST_log}" | wc -l | xargs echo)" + [ "10" = "${nb_files}" ] || fail "Expected 10 files, got ${nb_files}" +} + +function test_space_dollar_and_parentheses() { + basic_glob_scenario_test_template '$( )' +} + +run_suite "Integration tests for handling of special characters" + |