Downloader rewriter config has all_blocked_message
This allows for the config author to specify a useful error message to
adorn the user-facing error, so that if the user is unaware or forgets
about the config, they get a more clear error message than:
```
ERROR: java.io.IOException: Error downloading [] to /some/path
```
or
```
Error in download: java.io.IOException: Cache miss and no url specified
```
Currently there is a log line like:
```
INFO: Rewritten [https://doesnotexist.com/beep] as []
```
printed, but this is often quite far from the error - this puts all the
information in one place.
If you don't configure an all_blocked_message, this now looks like:
```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep]
```
And if you do, it will look like something like:
```
ERROR: java.io.IOException: Cache miss and no url specified - Configured URL rewriter blocked all URLs: [https://doesnotexist.com/beep] - See https://mycorp.com/bazel-rewriter-issue for more details.
```
Closes #12997.
PiperOrigin-RevId: 359730842
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java
index a117b7d..a8d0b1f 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java
@@ -33,6 +33,7 @@
import java.net.URL;
import java.util.List;
import java.util.Map;
+import javax.annotation.Nullable;
/**
* Bazel file downloader.
@@ -85,7 +86,7 @@
* @throws InterruptedException if this thread is being cast into oblivion
*/
public Path download(
- List<URL> urls,
+ List<URL> originalUrls,
Map<URI, Map<String, String>> authHeaders,
Optional<Checksum> checksum,
String canonicalId,
@@ -99,20 +100,21 @@
throw new InterruptedException();
}
+ List<URL> rewrittenUrls = originalUrls;
if (rewriter != null) {
- urls = rewriter.amend(urls);
+ rewrittenUrls = rewriter.amend(originalUrls);
}
URL mainUrl; // The "main" URL for this request
// Used for reporting only and determining the file name only.
- if (urls.isEmpty()) {
+ if (rewrittenUrls.isEmpty()) {
if (type.isPresent() && !Strings.isNullOrEmpty(type.get())) {
mainUrl = new URL("http://nonexistent.example.org/cacheprobe." + type.get());
} else {
mainUrl = new URL("http://nonexistent.example.org/cacheprobe");
}
} else {
- mainUrl = urls.get(0);
+ mainUrl = rewrittenUrls.get(0);
}
Path destination = getDownloadDestination(mainUrl, type, output);
ImmutableSet<String> candidateFileNames = getCandidateFileNames(mainUrl, destination);
@@ -153,8 +155,13 @@
}
}
- if (urls.isEmpty()) {
- throw new IOException("Cache miss and no url specified");
+ if (rewrittenUrls.isEmpty()) {
+ StringBuilder message = new StringBuilder("Cache miss and no url specified");
+ if (!originalUrls.isEmpty()) {
+ message.append(" - ");
+ message.append(getRewriterBlockedAllUrlsMessage(originalUrls));
+ }
+ throw new IOException(message.toString());
}
for (Path dir : distdir) {
@@ -204,9 +211,20 @@
String.format("Failed to download repo %s: download is disabled.", repo));
}
+ if (rewrittenUrls.isEmpty() && !originalUrls.isEmpty()) {
+ throw new IOException(getRewriterBlockedAllUrlsMessage(originalUrls));
+ }
+
try {
downloader.download(
- urls, authHeaders, checksum, canonicalId, destination, eventHandler, clientEnv, type);
+ rewrittenUrls,
+ authHeaders,
+ checksum,
+ canonicalId,
+ destination,
+ eventHandler,
+ clientEnv,
+ type);
} catch (InterruptedIOException e) {
throw new InterruptedException(e.getMessage());
}
@@ -216,12 +234,26 @@
checksum.get().toString(), destination, checksum.get().getKeyType(), canonicalId);
} else if (repositoryCache.isEnabled()) {
String newSha256 = repositoryCache.put(destination, KeyType.SHA256, canonicalId);
- eventHandler.handle(Event.info("SHA256 (" + urls.get(0) + ") = " + newSha256));
+ eventHandler.handle(Event.info("SHA256 (" + rewrittenUrls.get(0) + ") = " + newSha256));
}
return destination;
}
+ @Nullable
+ private String getRewriterBlockedAllUrlsMessage(List<URL> originalUrls) {
+ if (rewriter == null) {
+ return null;
+ }
+ StringBuilder message = new StringBuilder("Configured URL rewriter blocked all URLs: ");
+ message.append(originalUrls);
+ String rewriterMessage = rewriter.getAllBlockedMessage();
+ if (rewriterMessage != null) {
+ message.append(" - ").append(rewriterMessage);
+ }
+ return message.toString();
+ }
+
private Path getDownloadDestination(URL url, Optional<String> type, Path output) {
if (!type.isPresent()) {
return output;
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 dfeee88..a081f43 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
@@ -40,6 +40,7 @@
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import javax.annotation.Nullable;
/**
* Helper class for taking URLs and converting them according to an optional config specified by
@@ -196,4 +197,9 @@
})
.collect(toImmutableList());
}
+
+ @Nullable
+ public String getAllBlockedMessage() {
+ return config.getAllBlockedMessage();
+ }
}
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 322cf11..ad3b170 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 javax.annotation.Nullable;
import net.starlark.java.syntax.Location;
/**
@@ -36,11 +37,15 @@
* <li>{@code block hostName} Will block access to the given host and subdomains
* <li>{@code rewrite pattern pattern} Rewrite a URL using the given pattern. Back references are
* numbered from `$1`
+ * <li>{@code all_blocked_message message which may contain spaces} If the rewriter causes all
+ * URLs for a particular resource to be blocked, this informational message will be rendered
+ * to the user. This directive may only be present at most once.
* </ul>
*
* The directives are applied in the order `rewrite, allow, block'. An example config may look like:
*
* <pre>
+ * all_blocked_message See mycorp.com/blocked-bazel-fetches for more information.
* block mvnrepository.com
* block maven-central.storage.googleapis.com
* block gitblit.github.io
@@ -62,6 +67,7 @@
private static final Splitter SPLITTER =
Splitter.onPattern("\\s+").omitEmptyStrings().trimResults();
+ private static final String ALL_BLOCKED_MESSAGE_DIRECTIVE = "all_blocked_message";
// A set of domain names that should be accessible.
private final Set<String> allowList;
@@ -69,6 +75,8 @@
private final Set<String> blockList;
// A set of patterns matching "everything in the url after the scheme" to rewrite rules.
private final ImmutableMultimap<Pattern, String> rewrites;
+ // Message to display if the rewriter caused all URLs to be blocked.
+ @Nullable private final String allBlockedMessage;
/**
* Constructor to use. The {@code config} will be read to completion.
@@ -81,6 +89,7 @@
ImmutableSet.Builder<String> allowList = ImmutableSet.builder();
ImmutableSet.Builder<String> blockList = ImmutableSet.builder();
ImmutableMultimap.Builder<Pattern, String> rewrites = ImmutableMultimap.builder();
+ String allBlockedMessage = null;
try (BufferedReader reader = new BufferedReader(config)) {
int lineNumber = 1;
@@ -125,6 +134,18 @@
rewrites.put(Pattern.compile(parts.get(1)), parts.get(2));
break;
+ case ALL_BLOCKED_MESSAGE_DIRECTIVE:
+ if (parts.size() == 1) {
+ throw new UrlRewriterParseException(
+ "all_blocked_message must be followed by a message", location);
+ }
+ if (allBlockedMessage != null) {
+ throw new UrlRewriterParseException(
+ "At most one all_blocked_message directive is allowed", location);
+ }
+ allBlockedMessage = line.substring(ALL_BLOCKED_MESSAGE_DIRECTIVE.length() + 1);
+ break;
+
default:
throw new UrlRewriterParseException("Unable to parse: " + line, location);
}
@@ -136,6 +157,7 @@
this.allowList = allowList.build();
this.blockList = blockList.build();
this.rewrites = rewrites.build();
+ this.allBlockedMessage = allBlockedMessage;
}
/** Returns all {@code allow} directives. */
@@ -155,4 +177,9 @@
public Map<Pattern, Collection<String>> getRewrites() {
return rewrites.asMap();
}
+
+ @Nullable
+ public String getAllBlockedMessage() {
+ return allBlockedMessage;
+ }
}
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 10ec84e..805cf01 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
@@ -185,4 +185,31 @@
assertThat(e.getLocation()).isEqualTo(Location.fromFileLineColumn("/some/file", 2, 0));
}
}
+
+ @Test
+ public void noAllBlockedMessage() throws Exception {
+ String config = "";
+ UrlRewriterConfig munger = new UrlRewriterConfig("/some/file", new StringReader(config));
+ assertThat(munger.getAllBlockedMessage()).isNull();
+ }
+
+ @Test
+ public void singleAllBlockedMessage() throws Exception {
+ String config =
+ "all_blocked_message I'm sorry Dave, I'm afraid I can't do that.\n" + "allow *\n";
+ UrlRewriterConfig munger = new UrlRewriterConfig("/some/file", new StringReader(config));
+ assertThat(munger.getAllBlockedMessage())
+ .isEqualTo("I'm sorry Dave, I'm afraid I can't do that.");
+ }
+
+ @Test
+ public void multipleAllBlockedMessage() throws Exception {
+ String config = "all_blocked_message one\n" + "block *\n" + "all_blocked_message two\n";
+ try {
+ new UrlRewriterConfig("/some/file", new StringReader(config));
+ fail();
+ } catch (UrlRewriterParseException e) {
+ assertThat(e.getLocation()).isEqualTo(Location.fromFileLineColumn("/some/file", 3, 0));
+ }
+ }
}
