diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 422cb5af577b60..6b93d536ae1d1c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader; import com.google.devtools.build.lib.bazel.repository.downloader.UrlRewriter; +import com.google.devtools.build.lib.bazel.repository.downloader.UrlRewriterParseException; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryFunction; import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule; import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryFunction; @@ -225,7 +226,7 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) { } @Override - public void beforeCommand(CommandEnvironment env) { + public void beforeCommand(CommandEnvironment env) throws AbruptExitException { clientEnvironmentSupplier.set(env.getRepoEnv()); PackageOptions pkgOptions = env.getOptions().getOptions(PackageOptions.class); isFetch.set(pkgOptions != null && pkgOptions.fetch); @@ -284,10 +285,20 @@ public void beforeCommand(CommandEnvironment env) { } } - UrlRewriter rewriter = - UrlRewriter.getDownloaderUrlRewriter( - repoOptions == null ? null : repoOptions.downloaderConfig, env.getReporter()); - downloadManager.setUrlRewriter(rewriter); + try { + UrlRewriter rewriter = + UrlRewriter.getDownloaderUrlRewriter( + repoOptions == null ? null : repoOptions.downloaderConfig, env.getReporter()); + downloadManager.setUrlRewriter(rewriter); + } catch (UrlRewriterParseException e) { + // It's important that the build stops ASAP, because this config file may be required for + // security purposes, and the build must not proceed ignoring it. + throw new AbruptExitException( + detailedExitCode( + String.format( + "Failed to parse downloader config at %s: %s", e.getLocation(), e.getMessage()), + Code.BAD_DOWNLOADER_CONFIG)); + } if (repoOptions.experimentalDistdir != null) { downloadManager.setDistdir( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD index 88523be9861443..74507ef16ba5c7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD @@ -22,6 +22,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/net/starlark/java/syntax", "//third_party:flogger", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java index b3b37403fa559b..dfeee8866b3202 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java @@ -57,10 +57,11 @@ public class UrlRewriter { private final Consumer log; @VisibleForTesting - UrlRewriter(Consumer log, Reader reader) { + UrlRewriter(Consumer log, String filePathForErrorReporting, Reader reader) + throws UrlRewriterParseException { this.log = Preconditions.checkNotNull(log); Preconditions.checkNotNull(reader, "UrlRewriterConfig source must be set"); - this.config = new UrlRewriterConfig(reader); + this.config = new UrlRewriterConfig(filePathForErrorReporting, reader); this.rewriter = this::rewrite; } @@ -68,18 +69,19 @@ public class UrlRewriter { /** * Obtain a new {@code UrlRewriter} configured with the specified config file. * - * @param downloaderConfig Path to the config file to use. May be null. + * @param configPath Path to the config file to use. May be null. * @param reporter Used for logging when URLs are rewritten. */ - public static UrlRewriter getDownloaderUrlRewriter(String downloaderConfig, Reporter reporter) { + public static UrlRewriter getDownloaderUrlRewriter(String configPath, Reporter reporter) + throws UrlRewriterParseException { Consumer log = str -> reporter.handle(Event.info(str)); - if (Strings.isNullOrEmpty(downloaderConfig)) { - return new UrlRewriter(log, new StringReader("")); + if (Strings.isNullOrEmpty(configPath)) { + return new UrlRewriter(log, "", new StringReader("")); } - try (BufferedReader reader = Files.newBufferedReader(Paths.get(downloaderConfig))) { - return new UrlRewriter(log, reader); + try (BufferedReader reader = Files.newBufferedReader(Paths.get(configPath))) { + return new UrlRewriter(log, configPath, reader); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterConfig.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterConfig.java index f77f08e9749960..322cf11df46bd3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterConfig.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterConfig.java @@ -25,6 +25,7 @@ import java.util.Map; import java.util.Set; import java.util.regex.Pattern; +import net.starlark.java.syntax.Location; /** * Models the downloader config file. This file has a line-based format, with each line starting @@ -72,15 +73,18 @@ class UrlRewriterConfig { /** * Constructor to use. The {@code config} will be read to completion. * + * @throws UrlRewriterParseException If the file contents was invalid. * @throws UncheckedIOException If any processing problems occur. */ - public UrlRewriterConfig(Reader config) { + public UrlRewriterConfig(String filePathForErrorReporting, Reader config) + throws UrlRewriterParseException { ImmutableSet.Builder allowList = ImmutableSet.builder(); ImmutableSet.Builder blockList = ImmutableSet.builder(); ImmutableMultimap.Builder rewrites = ImmutableMultimap.builder(); try (BufferedReader reader = new BufferedReader(config)) { - for (String line = reader.readLine(); line != null; line = reader.readLine()) { + int lineNumber = 1; + for (String line = reader.readLine(); line != null; line = reader.readLine(), lineNumber++) { // Find the first word List parts = SPLITTER.splitToList(line); if (parts.isEmpty()) { @@ -92,34 +96,37 @@ public UrlRewriterConfig(Reader config) { continue; } + Location location = Location.fromFileLineColumn(filePathForErrorReporting, lineNumber, 0); + switch (parts.get(0)) { case "allow": if (parts.size() != 2) { - throw new IllegalStateException( - "Only the host name is allowed after `allow`: " + line); + throw new UrlRewriterParseException( + "Only the host name is allowed after `allow`: " + line, location); } allowList.add(parts.get(1)); break; case "block": if (parts.size() != 2) { - throw new IllegalStateException( - "Only the host name is allowed after `block`: " + line); + throw new UrlRewriterParseException( + "Only the host name is allowed after `block`: " + line, location); } blockList.add(parts.get(1)); break; case "rewrite": if (parts.size() != 3) { - throw new IllegalStateException( + throw new UrlRewriterParseException( "Only the matching pattern and rewrite pattern is allowed after `rewrite`: " - + line); + + line, + location); } rewrites.put(Pattern.compile(parts.get(1)), parts.get(2)); break; default: - throw new IllegalStateException("Unable to parse: " + line); + throw new UrlRewriterParseException("Unable to parse: " + line, location); } } } catch (IOException e) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterParseException.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterParseException.java new file mode 100644 index 00000000000000..57a95901d00dcb --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterParseException.java @@ -0,0 +1,30 @@ +// Copyright 2021 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. +package com.google.devtools.build.lib.bazel.repository.downloader; + +import net.starlark.java.syntax.Location; + +/** An {@link Exception} thrown when failed to parse {@link UrlRewriterConfig}. */ +public class UrlRewriterParseException extends Exception { + private final Location location; + + public UrlRewriterParseException(String message, Location location) { + super(message); + this.location = location; + } + + public Location getLocation() { + return location; + } +} diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index ed350af6e5721c..f3fd9dc1b4298d 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -240,6 +240,7 @@ message ExternalRepository { enum Code { EXTERNAL_REPOSITORY_UNKNOWN = 0 [(metadata) = { exit_code: 37 }]; OVERRIDE_DISALLOWED_MANAGED_DIRECTORIES = 1 [(metadata) = { exit_code: 2 }]; + BAD_DOWNLOADER_CONFIG = 2 [(metadata) = { exit_code: 2 }]; } Code code = 1; // Additional data could include external repository names. diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD index e50a41b3f1f5d0..79c0e6ce1b4f51 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/BUILD @@ -21,6 +21,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/net/starlark/java/syntax", "//src/test/java/com/google/devtools/build/lib/testutil", "//third_party:guava", "//third_party:jsr305", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterTest.java index 9a58156b87f85a..10ec84edbac2f6 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterTest.java @@ -14,12 +14,13 @@ package com.google.devtools.build.lib.bazel.repository.downloader; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; import com.google.common.collect.ImmutableList; import java.io.StringReader; -import java.net.MalformedURLException; import java.net.URL; import java.util.List; +import net.starlark.java.syntax.Location; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -29,8 +30,8 @@ public class UrlRewriterTest { @Test - public void byDefaultTheUrlRewriterDoesNothing() throws MalformedURLException { - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader("")); + public void byDefaultTheUrlRewriterDoesNothing() throws Exception { + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader("")); List urls = ImmutableList.of(new URL("http://example.com")); List amended = munger.amend(urls); @@ -39,9 +40,9 @@ public void byDefaultTheUrlRewriterDoesNothing() throws MalformedURLException { } @Test - public void shouldBeAbleToBlockParticularHostsRegardlessOfScheme() throws MalformedURLException { + public void shouldBeAbleToBlockParticularHostsRegardlessOfScheme() throws Exception { String config = "block example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List urls = ImmutableList.of( @@ -54,9 +55,9 @@ public void shouldBeAbleToBlockParticularHostsRegardlessOfScheme() throws Malfor } @Test - public void shouldAllowAUrlToBeRewritten() throws MalformedURLException { + public void shouldAllowAUrlToBeRewritten() throws Exception { String config = "rewrite example.com/foo/(.*) mycorp.com/$1/foo"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List urls = ImmutableList.of(new URL("https://example.com/foo/bar")); List amended = munger.amend(urls); @@ -65,11 +66,11 @@ public void shouldAllowAUrlToBeRewritten() throws MalformedURLException { } @Test - public void rewritesCanExpandToMoreThanOneUrl() throws MalformedURLException { + public void rewritesCanExpandToMoreThanOneUrl() throws Exception { String config = "rewrite example.com/foo/(.*) mycorp.com/$1/somewhere\n" + "rewrite example.com/foo/(.*) mycorp.com/$1/elsewhere"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List urls = ImmutableList.of(new URL("https://example.com/foo/bar")); List amended = munger.amend(urls); @@ -80,10 +81,10 @@ public void rewritesCanExpandToMoreThanOneUrl() throws MalformedURLException { } @Test - public void shouldBlockAllUrlsOtherThanSpecificOnes() throws MalformedURLException { + public void shouldBlockAllUrlsOtherThanSpecificOnes() throws Exception { String config = "" + "block *\n" + "allow example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List urls = ImmutableList.of( @@ -98,7 +99,7 @@ public void shouldBlockAllUrlsOtherThanSpecificOnes() throws MalformedURLExcepti } @Test - public void commentsArePrecededByTheHashCharacter() throws MalformedURLException { + public void commentsArePrecededByTheHashCharacter() throws Exception { String config = "" + "# Block everything\n" @@ -106,7 +107,7 @@ public void commentsArePrecededByTheHashCharacter() throws MalformedURLException + "# But allow example.com\n" + "allow example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List urls = ImmutableList.of(new URL("https://foo.com"), new URL("https://example.com")); List amended = munger.amend(urls); @@ -115,10 +116,10 @@ public void commentsArePrecededByTheHashCharacter() throws MalformedURLException } @Test - public void allowListAppliesToSubdomainsToo() throws MalformedURLException { + public void allowListAppliesToSubdomainsToo() throws Exception { String config = "" + "block *\n" + "allow example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com"))); @@ -126,10 +127,10 @@ public void allowListAppliesToSubdomainsToo() throws MalformedURLException { } @Test - public void blockListAppliesToSubdomainsToo() throws MalformedURLException { + public void blockListAppliesToSubdomainsToo() throws Exception { String config = "block example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com"))); @@ -137,10 +138,10 @@ public void blockListAppliesToSubdomainsToo() throws MalformedURLException { } @Test - public void emptyLinesAreFine() throws MalformedURLException { + public void emptyLinesAreFine() throws Exception { String config = "" + "\n" + " \n" + "block *\n" + "\t \n" + "allow example.com"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com"))); @@ -148,10 +149,10 @@ public void emptyLinesAreFine() throws MalformedURLException { } @Test - public void rewritingUrlsIsAppliedBeforeBlocking() throws MalformedURLException { + public void rewritingUrlsIsAppliedBeforeBlocking() throws Exception { String config = "" + "block bad.com\n" + "rewrite bad.com/foo/(.*) mycorp.com/$1"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List amended = munger.amend( @@ -161,11 +162,11 @@ public void rewritingUrlsIsAppliedBeforeBlocking() throws MalformedURLException } @Test - public void rewritingUrlsIsAppliedBeforeAllowing() throws MalformedURLException { + public void rewritingUrlsIsAppliedBeforeAllowing() throws Exception { String config = "" + "block *\n" + "allow mycorp.com\n" + "rewrite bad.com/foo/(.*) mycorp.com/$1"; - UrlRewriter munger = new UrlRewriter(str -> {}, new StringReader(config)); + UrlRewriter munger = new UrlRewriter(str -> {}, "/dev/null", new StringReader(config)); List amended = munger.amend( @@ -173,4 +174,15 @@ public void rewritingUrlsIsAppliedBeforeAllowing() throws MalformedURLException assertThat(amended).containsExactly(new URL("https://mycorp.com/bar")); } + + @Test + public void parseError() throws Exception { + String config = "#comment\nhello"; + try { + new UrlRewriterConfig("/some/file", new StringReader(config)); + fail(); + } catch (UrlRewriterParseException e) { + assertThat(e.getLocation()).isEqualTo(Location.fromFileLineColumn("/some/file", 2, 0)); + } + } }