Skip to content

Commit

Permalink
Report errors parsing rewriter config file
Browse files Browse the repository at this point in the history
Before this change, an invalid config file results in a silent server
crash.

Closes #12989.

PiperOrigin-RevId: 359251417
  • Loading branch information
illicitonion authored and copybara-github committed Feb 24, 2021
1 parent 17afbe4 commit b243584
Show file tree
Hide file tree
Showing 8 changed files with 110 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,29 +57,31 @@ public class UrlRewriter {
private final Consumer<String> log;

@VisibleForTesting
UrlRewriter(Consumer<String> log, Reader reader) {
UrlRewriter(Consumer<String> 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;
}

/**
* 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<String> 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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<String> allowList = ImmutableSet.builder();
ImmutableSet.Builder<String> blockList = ImmutableSet.builder();
ImmutableMultimap.Builder<Pattern, String> 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<String> parts = SPLITTER.splitToList(line);
if (parts.isEmpty()) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<URL> urls = ImmutableList.of(new URL("http://example.com"));
List<URL> amended = munger.amend(urls);
Expand All @@ -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<URL> urls =
ImmutableList.of(
Expand All @@ -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<URL> urls = ImmutableList.of(new URL("https://example.com/foo/bar"));
List<URL> amended = munger.amend(urls);
Expand All @@ -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<URL> urls = ImmutableList.of(new URL("https://example.com/foo/bar"));
List<URL> amended = munger.amend(urls);
Expand All @@ -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<URL> urls =
ImmutableList.of(
Expand All @@ -98,15 +99,15 @@ public void shouldBlockAllUrlsOtherThanSpecificOnes() throws MalformedURLExcepti
}

@Test
public void commentsArePrecededByTheHashCharacter() throws MalformedURLException {
public void commentsArePrecededByTheHashCharacter() throws Exception {
String config =
""
+ "# Block everything\n"
+ "block *\n"
+ "# 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<URL> urls = ImmutableList.of(new URL("https://foo.com"), new URL("https://example.com"));
List<URL> amended = munger.amend(urls);
Expand All @@ -115,43 +116,43 @@ 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<URL> amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com")));

assertThat(amended).containsExactly(new URL("https://subdomain.example.com"));
}

@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<URL> amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com")));

assertThat(amended).isEmpty();
}

@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<URL> amended = munger.amend(ImmutableList.of(new URL("https://subdomain.example.com")));

assertThat(amended).containsExactly(new URL("https://subdomain.example.com"));
}

@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<URL> amended =
munger.amend(
Expand All @@ -161,16 +162,27 @@ 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<URL> amended =
munger.amend(
ImmutableList.of(new URL("https://www.bad.com"), new URL("https://bad.com/foo/bar")));

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

0 comments on commit b243584

Please sign in to comment.