Skip to content

Commit 63bc1c7

Browse files
illicitonioncopybara-github
authored andcommitted
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
1 parent 2f00d4f commit 63bc1c7

File tree

4 files changed

+100
-8
lines changed

4 files changed

+100
-8
lines changed

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloadManager.java

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.net.URL;
3434
import java.util.List;
3535
import java.util.Map;
36+
import javax.annotation.Nullable;
3637

3738
/**
3839
* Bazel file downloader.
@@ -85,7 +86,7 @@ public void setDisableDownload(boolean disableDownload) {
8586
* @throws InterruptedException if this thread is being cast into oblivion
8687
*/
8788
public Path download(
88-
List<URL> urls,
89+
List<URL> originalUrls,
8990
Map<URI, Map<String, String>> authHeaders,
9091
Optional<Checksum> checksum,
9192
String canonicalId,
@@ -99,20 +100,21 @@ public Path download(
99100
throw new InterruptedException();
100101
}
101102

103+
List<URL> rewrittenUrls = originalUrls;
102104
if (rewriter != null) {
103-
urls = rewriter.amend(urls);
105+
rewrittenUrls = rewriter.amend(originalUrls);
104106
}
105107

106108
URL mainUrl; // The "main" URL for this request
107109
// Used for reporting only and determining the file name only.
108-
if (urls.isEmpty()) {
110+
if (rewrittenUrls.isEmpty()) {
109111
if (type.isPresent() && !Strings.isNullOrEmpty(type.get())) {
110112
mainUrl = new URL("http://nonexistent.example.org/cacheprobe." + type.get());
111113
} else {
112114
mainUrl = new URL("http://nonexistent.example.org/cacheprobe");
113115
}
114116
} else {
115-
mainUrl = urls.get(0);
117+
mainUrl = rewrittenUrls.get(0);
116118
}
117119
Path destination = getDownloadDestination(mainUrl, type, output);
118120
ImmutableSet<String> candidateFileNames = getCandidateFileNames(mainUrl, destination);
@@ -153,8 +155,13 @@ public Path download(
153155
}
154156
}
155157

156-
if (urls.isEmpty()) {
157-
throw new IOException("Cache miss and no url specified");
158+
if (rewrittenUrls.isEmpty()) {
159+
StringBuilder message = new StringBuilder("Cache miss and no url specified");
160+
if (!originalUrls.isEmpty()) {
161+
message.append(" - ");
162+
message.append(getRewriterBlockedAllUrlsMessage(originalUrls));
163+
}
164+
throw new IOException(message.toString());
158165
}
159166

160167
for (Path dir : distdir) {
@@ -204,9 +211,20 @@ public Path download(
204211
String.format("Failed to download repo %s: download is disabled.", repo));
205212
}
206213

214+
if (rewrittenUrls.isEmpty() && !originalUrls.isEmpty()) {
215+
throw new IOException(getRewriterBlockedAllUrlsMessage(originalUrls));
216+
}
217+
207218
try {
208219
downloader.download(
209-
urls, authHeaders, checksum, canonicalId, destination, eventHandler, clientEnv, type);
220+
rewrittenUrls,
221+
authHeaders,
222+
checksum,
223+
canonicalId,
224+
destination,
225+
eventHandler,
226+
clientEnv,
227+
type);
210228
} catch (InterruptedIOException e) {
211229
throw new InterruptedException(e.getMessage());
212230
}
@@ -216,12 +234,26 @@ public Path download(
216234
checksum.get().toString(), destination, checksum.get().getKeyType(), canonicalId);
217235
} else if (repositoryCache.isEnabled()) {
218236
String newSha256 = repositoryCache.put(destination, KeyType.SHA256, canonicalId);
219-
eventHandler.handle(Event.info("SHA256 (" + urls.get(0) + ") = " + newSha256));
237+
eventHandler.handle(Event.info("SHA256 (" + rewrittenUrls.get(0) + ") = " + newSha256));
220238
}
221239

222240
return destination;
223241
}
224242

243+
@Nullable
244+
private String getRewriterBlockedAllUrlsMessage(List<URL> originalUrls) {
245+
if (rewriter == null) {
246+
return null;
247+
}
248+
StringBuilder message = new StringBuilder("Configured URL rewriter blocked all URLs: ");
249+
message.append(originalUrls);
250+
String rewriterMessage = rewriter.getAllBlockedMessage();
251+
if (rewriterMessage != null) {
252+
message.append(" - ").append(rewriterMessage);
253+
}
254+
return message.toString();
255+
}
256+
225257
private Path getDownloadDestination(URL url, Optional<String> type, Path output) {
226258
if (!type.isPresent()) {
227259
return output;

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriter.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.function.Function;
4141
import java.util.regex.Matcher;
4242
import java.util.regex.Pattern;
43+
import javax.annotation.Nullable;
4344

4445
/**
4546
* 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) {
196197
})
197198
.collect(toImmutableList());
198199
}
200+
201+
@Nullable
202+
public String getAllBlockedMessage() {
203+
return config.getAllBlockedMessage();
204+
}
199205
}

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterConfig.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.Map;
2626
import java.util.Set;
2727
import java.util.regex.Pattern;
28+
import javax.annotation.Nullable;
2829
import net.starlark.java.syntax.Location;
2930

3031
/**
@@ -36,11 +37,15 @@
3637
* <li>{@code block hostName} Will block access to the given host and subdomains
3738
* <li>{@code rewrite pattern pattern} Rewrite a URL using the given pattern. Back references are
3839
* numbered from `$1`
40+
* <li>{@code all_blocked_message message which may contain spaces} If the rewriter causes all
41+
* URLs for a particular resource to be blocked, this informational message will be rendered
42+
* to the user. This directive may only be present at most once.
3943
* </ul>
4044
*
4145
* The directives are applied in the order `rewrite, allow, block'. An example config may look like:
4246
*
4347
* <pre>
48+
* all_blocked_message See mycorp.com/blocked-bazel-fetches for more information.
4449
* block mvnrepository.com
4550
* block maven-central.storage.googleapis.com
4651
* block gitblit.github.io
@@ -62,13 +67,16 @@ class UrlRewriterConfig {
6267

6368
private static final Splitter SPLITTER =
6469
Splitter.onPattern("\\s+").omitEmptyStrings().trimResults();
70+
private static final String ALL_BLOCKED_MESSAGE_DIRECTIVE = "all_blocked_message";
6571

6672
// A set of domain names that should be accessible.
6773
private final Set<String> allowList;
6874
// A set of domain names that should be blocked.
6975
private final Set<String> blockList;
7076
// A set of patterns matching "everything in the url after the scheme" to rewrite rules.
7177
private final ImmutableMultimap<Pattern, String> rewrites;
78+
// Message to display if the rewriter caused all URLs to be blocked.
79+
@Nullable private final String allBlockedMessage;
7280

7381
/**
7482
* Constructor to use. The {@code config} will be read to completion.
@@ -81,6 +89,7 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
8189
ImmutableSet.Builder<String> allowList = ImmutableSet.builder();
8290
ImmutableSet.Builder<String> blockList = ImmutableSet.builder();
8391
ImmutableMultimap.Builder<Pattern, String> rewrites = ImmutableMultimap.builder();
92+
String allBlockedMessage = null;
8493

8594
try (BufferedReader reader = new BufferedReader(config)) {
8695
int lineNumber = 1;
@@ -125,6 +134,18 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
125134
rewrites.put(Pattern.compile(parts.get(1)), parts.get(2));
126135
break;
127136

137+
case ALL_BLOCKED_MESSAGE_DIRECTIVE:
138+
if (parts.size() == 1) {
139+
throw new UrlRewriterParseException(
140+
"all_blocked_message must be followed by a message", location);
141+
}
142+
if (allBlockedMessage != null) {
143+
throw new UrlRewriterParseException(
144+
"At most one all_blocked_message directive is allowed", location);
145+
}
146+
allBlockedMessage = line.substring(ALL_BLOCKED_MESSAGE_DIRECTIVE.length() + 1);
147+
break;
148+
128149
default:
129150
throw new UrlRewriterParseException("Unable to parse: " + line, location);
130151
}
@@ -136,6 +157,7 @@ public UrlRewriterConfig(String filePathForErrorReporting, Reader config)
136157
this.allowList = allowList.build();
137158
this.blockList = blockList.build();
138159
this.rewrites = rewrites.build();
160+
this.allBlockedMessage = allBlockedMessage;
139161
}
140162

141163
/** Returns all {@code allow} directives. */
@@ -155,4 +177,9 @@ public Set<String> getBlockList() {
155177
public Map<Pattern, Collection<String>> getRewrites() {
156178
return rewrites.asMap();
157179
}
180+
181+
@Nullable
182+
public String getAllBlockedMessage() {
183+
return allBlockedMessage;
184+
}
158185
}

src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/UrlRewriterTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,4 +185,31 @@ public void parseError() throws Exception {
185185
assertThat(e.getLocation()).isEqualTo(Location.fromFileLineColumn("/some/file", 2, 0));
186186
}
187187
}
188+
189+
@Test
190+
public void noAllBlockedMessage() throws Exception {
191+
String config = "";
192+
UrlRewriterConfig munger = new UrlRewriterConfig("/some/file", new StringReader(config));
193+
assertThat(munger.getAllBlockedMessage()).isNull();
194+
}
195+
196+
@Test
197+
public void singleAllBlockedMessage() throws Exception {
198+
String config =
199+
"all_blocked_message I'm sorry Dave, I'm afraid I can't do that.\n" + "allow *\n";
200+
UrlRewriterConfig munger = new UrlRewriterConfig("/some/file", new StringReader(config));
201+
assertThat(munger.getAllBlockedMessage())
202+
.isEqualTo("I'm sorry Dave, I'm afraid I can't do that.");
203+
}
204+
205+
@Test
206+
public void multipleAllBlockedMessage() throws Exception {
207+
String config = "all_blocked_message one\n" + "block *\n" + "all_blocked_message two\n";
208+
try {
209+
new UrlRewriterConfig("/some/file", new StringReader(config));
210+
fail();
211+
} catch (UrlRewriterParseException e) {
212+
assertThat(e.getLocation()).isEqualTo(Location.fromFileLineColumn("/some/file", 3, 0));
213+
}
214+
}
188215
}

0 commit comments

Comments
 (0)