Downloader rewriter config has all_blocked_message

illicitonion · copybara-github · commit 63bc1c7d0853 · 2021-02-26T04:23:37.000-08:00
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
@@ -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 @@ public void setDisableDownload(boolean disableDownload) {
    * @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 @@ public Path download(
       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 @@ public Path download(
         }
       }
 
-      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 @@ public Path download(
           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 @@ public Path download(
           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
@@ -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 @@ private ImmutableList<URL> applyRewriteRules(URL url) {
             })
         .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
@@ -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,13 +67,16 @@ class UrlRewriterConfig {
 
   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;
   // A set of domain names that should be blocked.
   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 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
     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 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
             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 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
     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 Set<String> getBlockList() {
   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
@@ -185,4 +185,31 @@ public void parseError() throws Exception {
       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));
+    }
+  }
 }
