Improve reliability/performance of Bazel downloads
1. We now retry on connection failures.
a. With exponential backoff.
b. While recovering quickly from ephemeral failures.
c. While still working if internet or web server is slow.
2. We now request gzip responses from web server.
Fixed #1760
Fixed #1910
RELNOTES: External downloads now retry with exponential backoff and support gzip content-encoding.
--
MOS_MIGRATED_REVID=139612882
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnection.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnection.java
deleted file mode 100644
index 84e6c47..0000000
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnection.java
+++ /dev/null
@@ -1,238 +0,0 @@
-// Copyright 2016 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 com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Optional;
-import com.google.common.io.ByteStreams;
-import com.google.common.net.MediaType;
-import java.io.Closeable;
-import java.io.IOException;
-import java.io.InputStream;
-import java.net.HttpURLConnection;
-import java.net.MalformedURLException;
-import java.net.Proxy;
-import java.net.SocketTimeoutException;
-import java.net.URL;
-import java.net.URLConnection;
-import java.nio.charset.Charset;
-import java.nio.charset.StandardCharsets;
-import java.util.Map;
-
-/**
- * Represents a connection over HTTP.
- */
-class HttpConnection implements Closeable {
- private static final int MAX_REDIRECTS = 20;
- private static final int TIMEOUT_MS = 60000;
- private final InputStream inputStream;
- private final int contentLength;
-
- private HttpConnection(InputStream inputStream, int contentLength) {
- this.inputStream = inputStream;
- this.contentLength = contentLength;
- }
-
- public InputStream getInputStream() {
- return inputStream;
- }
-
- /**
- * @return The length of the response, or -1 if unknown.
- */
- int getContentLength() {
- return contentLength;
- }
-
- @Override
- public void close() throws IOException {
- inputStream.close();
- }
-
- private static int parseContentLength(HttpURLConnection connection) {
- String length;
- try {
- length = connection.getHeaderField("Content-Length");
- if (length == null) {
- return -1;
- }
- return Integer.parseInt(length);
- } catch (NumberFormatException e) {
- return -1;
- }
- }
-
- /**
- * Connects to the given URL. Should not leave any connections open if anything goes wrong.
- */
- static HttpConnection createAndConnect(URL url, Map<String, String> clientEnv)
- throws IOException {
- Proxy proxy = ProxyHelper.createProxyIfNeeded(url.toString(), clientEnv);
- for (int i = 0; i < MAX_REDIRECTS; ++i) {
- URLConnection urlConnection = url.openConnection(proxy);
- if (!(urlConnection instanceof HttpURLConnection)) {
- return createFileConnection(urlConnection);
- }
-
- HttpURLConnection connection = (HttpURLConnection) urlConnection;
- int statusCode;
- try {
- statusCode = createAndConnectViaHttp(connection);
- } catch (IOException e) {
- connection.disconnect();
- throw e;
- }
-
- switch (statusCode) {
- case HttpURLConnection.HTTP_OK:
- try {
- return new HttpConnection(connection.getInputStream(), parseContentLength(connection));
- } catch (IOException e) {
- connection.disconnect();
- throw e;
- }
- case HttpURLConnection.HTTP_MOVED_PERM:
- case HttpURLConnection.HTTP_MOVED_TEMP:
- // Try again with the new URL. This is the only case that doesn't return/throw.
- url = tryGetLocation(statusCode, connection);
- connection.disconnect();
- break;
- case -1:
- throw new IOException("An HTTP error occurred");
- default:
- throw new IOException(
- String.format(
- "%s %s: %s",
- connection.getResponseCode(),
- connection.getResponseMessage(),
- readBody(connection)));
- }
- }
- throw new IOException("Maximum redirects (" + MAX_REDIRECTS + ") exceeded");
- }
-
- // For file:// URLs.
- private static HttpConnection createFileConnection(URLConnection connection)
- throws IOException {
- int contentLength = connection.getContentLength();
- // check for empty file. -1 is a valid contentLength, meaning the size of unknown. It's a
- // common return value for an FTP download request for example. Local files will always
- // have a valid contentLength value.
- if (contentLength == 0) {
- throw new IOException("Attempted to download an empty file");
- }
-
- return new HttpConnection(connection.getInputStream(), contentLength);
- }
-
- private static int createAndConnectViaHttp(HttpURLConnection connection) throws IOException {
- connection.setConnectTimeout(TIMEOUT_MS);
- connection.setReadTimeout(TIMEOUT_MS);
- try {
- connection.connect();
- } catch (SocketTimeoutException e) {
- throw new IOException(
- "Timed out connecting to " + connection.getURL() + " : " + e.getMessage(), e);
- } catch (IllegalArgumentException | IOException e) {
- throw new IOException(
- "Failed to connect to " + connection.getURL() + " : " + e.getMessage(), e);
- }
- return connection.getResponseCode();
- }
-
- private static URL tryGetLocation(int statusCode, HttpURLConnection connection)
- throws IOException {
- String newLocation = connection.getHeaderField("Location");
- if (newLocation == null) {
- throw new IOException(
- "Remote returned " + statusCode + " but did not return location header.");
- }
-
- URL newUrl;
- try {
- newUrl = new URL(newLocation);
- } catch (MalformedURLException e) {
- throw new IOException("Remote returned invalid location header: " + newLocation);
- }
-
- String newProtocol = newUrl.getProtocol();
- if (!("http".equals(newProtocol) || "https".equals(newProtocol))) {
- throw new IOException(
- "Remote returned invalid location header: " + newLocation);
- }
-
- return newUrl;
- }
-
- /**
- * Attempts to detect the encoding the HTTP reponse is using.
- *
- * <p>This attempts to read the Content-Encoding header, then the Content-Type header,
- * then just falls back to UTF-8.</p>
- *
- * @throws IOException If something goes wrong (the encoding isn't parsable or is, but isn't
- * supported by the system).
- */
- @VisibleForTesting
- static Charset getEncoding(HttpURLConnection connection) throws IOException {
- String encoding = connection.getContentEncoding();
- if (encoding != null) {
- if (Charset.availableCharsets().containsKey(encoding)) {
- try {
- return Charset.forName(encoding);
- } catch (IllegalArgumentException | UnsupportedOperationException e) {
- throw new IOException(
- "Got invalid encoding from " + connection.getURL() + ": " + encoding);
- }
- } else {
- throw new IOException(
- "Got unavailable encoding from " + connection.getURL() + ": " + encoding);
- }
- }
- encoding = connection.getContentType();
- if (encoding == null) {
- return StandardCharsets.UTF_8;
- }
- try {
- MediaType mediaType = MediaType.parse(encoding);
- if (mediaType == null) {
- return StandardCharsets.UTF_8;
- }
- Optional<Charset> charset = mediaType.charset();
- if (charset.isPresent()) {
- return charset.get();
- }
- } catch (IllegalArgumentException | IllegalStateException e) {
- throw new IOException(
- "Got invalid encoding from " + connection.getURL() + ": " + encoding);
- }
- return StandardCharsets.UTF_8;
- }
-
- private static String readBody(HttpURLConnection connection) throws IOException {
- InputStream errorStream = connection.getErrorStream();
- Charset encoding = getEncoding(connection);
- if (errorStream != null) {
- return new String(ByteStreams.toByteArray(errorStream), encoding);
- }
-
- InputStream responseStream = connection.getInputStream();
- if (responseStream != null) {
- return new String(ByteStreams.toByteArray(responseStream), encoding);
- }
-
- return null;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnector.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnector.java
new file mode 100644
index 0000000..616f0b0
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnector.java
@@ -0,0 +1,246 @@
+// Copyright 2016 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 static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Strings.nullToEmpty;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Ascii;
+import com.google.common.math.IntMath;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
+import java.net.Proxy;
+import java.net.SocketTimeoutException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
+import java.net.UnknownHostException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.zip.GZIPInputStream;
+import javax.annotation.Nullable;
+
+/** Utility class for connecting to HTTP servers for downloading files. */
+final class HttpConnector {
+
+ private static final int MAX_RETRIES = 8;
+ private static final int MAX_REDIRECTS = 20;
+ private static final int MIN_RETRY_DELAY_MS = 100;
+ private static final int CONNECT_TIMEOUT_MS = 1000;
+ private static final int MAX_CONNECT_TIMEOUT_MS = 10000;
+ private static final int READ_TIMEOUT_MS = 20000;
+
+ /**
+ * Connects to HTTP (or file) URL with GET request and lazily returns payload.
+ *
+ * <p>This routine supports gzip, redirects, retries, and exponential backoff. It's designed to
+ * recover fast from transient errors. However please note that this this reliability magic only
+ * applies to the connection and header reading phase.
+ *
+ * @param url URL to download, which can be file, http, or https
+ * @param proxy HTTP proxy to use or {@link Proxy#NO_PROXY} if none is desired
+ * @param eventHandler Bazel event handler for reporting real-time progress on retries
+ * @throws IOException if response returned ≥400 after max retries or ≥300 after max redirects
+ * @throws InterruptedException if thread is being cast into oblivion
+ */
+ static InputStream connect(
+ URL url, Proxy proxy, EventHandler eventHandler)
+ throws IOException, InterruptedException {
+ checkNotNull(proxy);
+ checkNotNull(eventHandler);
+ if (isProtocol(url, "file")) {
+ return url.openConnection().getInputStream();
+ }
+ if (!isHttp(url)) {
+ throw new IOException("Protocol must be http, https, or file");
+ }
+ List<Throwable> suppressions = new ArrayList<>();
+ int retries = 0;
+ int redirects = 0;
+ int connectTimeout = CONNECT_TIMEOUT_MS;
+ while (true) {
+ HttpURLConnection connection = null;
+ try {
+ connection = (HttpURLConnection) url.openConnection(proxy);
+ connection.setRequestProperty("Accept-Encoding", "gzip");
+ connection.setConnectTimeout(connectTimeout);
+ connection.setReadTimeout(READ_TIMEOUT_MS);
+ int code;
+ try {
+ connection.connect();
+ code = connection.getResponseCode();
+ } catch (FileNotFoundException ignored) {
+ code = connection.getResponseCode();
+ } catch (IOException e) {
+ if (!e.getMessage().startsWith("Server returned")) {
+ throw e;
+ }
+ code = connection.getResponseCode();
+ }
+ if (code == 200) {
+ return getInputStream(connection);
+ } else if (code == 301 || code == 302) {
+ readAllBytesAndClose(connection.getInputStream());
+ if (++redirects == MAX_REDIRECTS) {
+ throw new UnrecoverableHttpException("Redirect loop detected");
+ }
+ url = getLocation(connection);
+ } else if (code < 500) {
+ readAllBytesAndClose(connection.getErrorStream());
+ throw new UnrecoverableHttpException(describeHttpResponse(connection));
+ } else {
+ throw new IOException(describeHttpResponse(connection));
+ }
+ } catch (InterruptedIOException e) {
+ throw new InterruptedException();
+ } catch (UnrecoverableHttpException e) {
+ throw e;
+ } catch (UnknownHostException e) {
+ throw new IOException("Unknown host: " + e.getMessage());
+ } catch (IOException e) {
+ if (connection != null) {
+ connection.disconnect();
+ }
+ if (e instanceof SocketTimeoutException) {
+ connectTimeout = Math.min(connectTimeout * 2, MAX_CONNECT_TIMEOUT_MS);
+ }
+ if (++retries == MAX_RETRIES) {
+ for (Throwable suppressed : suppressions) {
+ e.addSuppressed(suppressed);
+ }
+ throw e;
+ }
+ suppressions.add(e);
+ int timeout = IntMath.pow(2, retries) * MIN_RETRY_DELAY_MS;
+ eventHandler.handle(Event.progress(
+ String.format("Failed to connect to %s trying again in %,dms: %s",
+ url, timeout, e)));
+ TimeUnit.MILLISECONDS.sleep(timeout);
+ } catch (RuntimeException e) {
+ if (connection != null) {
+ connection.disconnect();
+ }
+ throw e;
+ }
+ }
+ }
+
+ private static String describeHttpResponse(HttpURLConnection connection) throws IOException {
+ return String.format(
+ "%s returned %s %s",
+ connection.getRequestMethod(),
+ connection.getResponseCode(),
+ nullToEmpty(connection.getResponseMessage()));
+ }
+
+ private static void readAllBytesAndClose(@Nullable InputStream stream) throws IOException {
+ if (stream != null) {
+ // TODO: Replace with ByteStreams#exhaust when Guava 20 comes out.
+ byte[] buf = new byte[8192];
+ while (stream.read(buf) != -1) {}
+ stream.close();
+ }
+ }
+
+ private static InputStream getInputStream(HttpURLConnection connection) throws IOException {
+ // See RFC2616 § 3.5 and § 14.11
+ switch (firstNonNull(connection.getContentEncoding(), "identity")) {
+ case "identity":
+ return connection.getInputStream();
+ case "gzip":
+ case "x-gzip":
+ return new GZIPInputStream(connection.getInputStream());
+ default:
+ throw new UnrecoverableHttpException(
+ "Unsupported and unrequested Content-Encoding: " + connection.getContentEncoding());
+ }
+ }
+
+ @VisibleForTesting
+ static URL getLocation(HttpURLConnection connection) throws IOException {
+ String newLocation = connection.getHeaderField("Location");
+ if (newLocation == null) {
+ throw new IOException("Remote redirect missing Location.");
+ }
+ URL result = mergeUrls(URI.create(newLocation), connection.getURL());
+ if (!isHttp(result)) {
+ throw new IOException("Bad Location: " + newLocation);
+ }
+ return result;
+ }
+
+ private static URL mergeUrls(URI preferred, URL original) throws IOException {
+ // If the Location value provided in a 3xx (Redirection) response does not have a fragment
+ // component, a user agent MUST process the redirection as if the value inherits the fragment
+ // component of the URI reference used to generate the request target (i.e., the redirection
+ // inherits the original reference's fragment, if any). Quoth RFC7231 § 7.1.2
+ String protocol = firstNonNull(preferred.getScheme(), original.getProtocol());
+ String userInfo = preferred.getUserInfo();
+ String host = preferred.getHost();
+ int port;
+ if (host == null) {
+ host = original.getHost();
+ port = original.getPort();
+ userInfo = original.getUserInfo();
+ } else {
+ port = preferred.getPort();
+ if (userInfo == null
+ && host.equals(original.getHost())
+ && port == original.getPort()) {
+ userInfo = original.getUserInfo();
+ }
+ }
+ String path = preferred.getPath();
+ String query = preferred.getQuery();
+ String fragment = preferred.getFragment();
+ if (fragment == null) {
+ fragment = original.getRef();
+ }
+ URL result;
+ try {
+ result = new URI(protocol, userInfo, host, port, path, query, fragment).toURL();
+ } catch (URISyntaxException | MalformedURLException e) {
+ throw new IOException("Could not merge " + preferred + " into " + original);
+ }
+ return result;
+ }
+
+ private static boolean isHttp(URL url) {
+ return isProtocol(url, "http") || isProtocol(url, "https");
+ }
+
+ private static boolean isProtocol(URL url, String protocol) {
+ // An implementation should accept uppercase letters as equivalent to lowercase in scheme names
+ // (e.g., allow "HTTP" as well as "http") for the sake of robustness. Quoth RFC3986 § 3.1
+ return Ascii.equalsIgnoreCase(protocol, url.getProtocol());
+ }
+
+ private static final class UnrecoverableHttpException extends IOException {
+ UnrecoverableHttpException(String message) {
+ super(message);
+ }
+ }
+
+ private HttpConnector() {}
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
index 400f9e2..ea4cc81 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpDownloader.java
@@ -31,6 +31,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
+import java.net.Proxy;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
@@ -82,10 +83,7 @@
try {
return download(url, sha256, type, outputDirectory, eventHandler, clientEnv);
} catch (IOException e) {
- throw new RepositoryFunctionException(
- new IOException(
- "Error downloading from " + url + " to " + outputDirectory + ": " + e.getMessage()),
- SkyFunctionException.Transience.TRANSIENT);
+ throw new RepositoryFunctionException(e, SkyFunctionException.Transience.TRANSIENT);
}
}
@@ -130,10 +128,10 @@
AtomicInteger totalBytes = new AtomicInteger(0);
final ScheduledFuture<?> loggerHandle = getLoggerHandle(totalBytes, eventHandler, urlString);
final URL url = new URL(urlString);
+ Proxy proxy = ProxyHelper.createProxyIfNeeded(url.toString(), clientEnv);
try (OutputStream out = destination.getOutputStream();
- HttpConnection connection = HttpConnection.createAndConnect(url, clientEnv)) {
- InputStream inputStream = connection.getInputStream();
+ InputStream inputStream = HttpConnector.connect(url, proxy, eventHandler)) {
int read;
byte[] buf = new byte[BUFFER_SIZE];
while ((read = inputStream.read(buf)) > 0) {
@@ -143,11 +141,6 @@
throw new InterruptedException("Download interrupted");
}
}
- if (connection.getContentLength() != -1
- && totalBytes.get() != connection.getContentLength()) {
- throw new IOException("Expected " + formatSize(connection.getContentLength()) + ", got "
- + formatSize(totalBytes.get()));
- }
} catch (IOException e) {
throw new IOException(
"Error downloading " + url + " to " + destination + ": " + e.getMessage());
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
index 1256c54..7e9d9c9 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpArchiveRule.java
@@ -39,7 +39,7 @@
A URL referencing an archive file containing a Bazel repository.
<p>Archives of type .zip, .jar, .war, .tar.gz or .tgz are supported. There is no support
- for authentication. Redirections are followed, but not from HTTP to HTTPS.</p>
+ for authentication.</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("url", STRING).mandatory())
/* <!-- #BLAZE_RULE(http_archive).ATTRIBUTE(sha256) -->
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
index b4e3dc9..72e1638 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpFileRule.java
@@ -39,8 +39,7 @@
/* <!-- #BLAZE_RULE(http_file).ATTRIBUTE(url) -->
A URL to a file that will be made available to Bazel.
- <p>This must be an http or https URL. Authentication is not support.
- Redirections are followed, but not from HTTP to HTTPS.</p>
+ <p>This must be an http or https URL. Authentication is not supported.</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("url", STRING).mandatory())
/* <!-- #BLAZE_RULE(http_file).ATTRIBUTE(sha256) -->
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
index 06769ce..fdce57e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/HttpJarRule.java
@@ -38,8 +38,7 @@
/* <!-- #BLAZE_RULE(http_jar).ATTRIBUTE(url) -->
A URL to an archive file containing a Bazel repository.
- <p>This must be an http or https URL that ends with .jar. Redirections are followed, but
- not from HTTP to HTTPS.</p>
+ <p>This must be an http or https URL that ends with .jar.</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("url", STRING).mandatory())
/* <!-- #BLAZE_RULE(http_jar).ATTRIBUTE(sha256) -->
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
index b86f0f0..2ccbef8 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/workspace/NewHttpArchiveRule.java
@@ -36,7 +36,7 @@
A URL referencing an archive file containing a Bazel repository.
<p>Archives of type .zip, .jar, .war, .tar.gz or .tgz are supported. There is no support
- for authentication. Redirections are followed, but not from HTTP to HTTPS.</p>
+ for authentication.</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("url", STRING).mandatory())
/* <!-- #BLAZE_RULE(new_http_archive).ATTRIBUTE(sha256) -->
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 f29ceb4..988efb8 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
@@ -5,11 +5,11 @@
)
java_test(
- name = "DownloaderTests",
+ name = "DownloaderTestSuite",
srcs = glob(["*.java"]),
tags = ["rules"],
- test_class = "com.google.devtools.build.lib.AllTests",
deps = [
+ "//src/main/java/com/google/devtools/build/lib:events",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/test/java/com/google/devtools/build/lib:foundations_testutil",
"//src/test/java/com/google/devtools/build/lib:test_runner",
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java
new file mode 100644
index 0000000..1a48a1c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/DownloaderTestSuite.java
@@ -0,0 +1,27 @@
+// Copyright 2016 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 org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+import org.junit.runners.Suite.SuiteClasses;
+
+/** Test suite for downloader package. */
+@RunWith(Suite.class)
+@SuiteClasses({
+ HttpConnectorTest.class,
+ ProxyHelperTest.class,
+})
+class DownloaderTestSuite {}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectionTest.java
deleted file mode 100644
index 93d9f61..0000000
--- a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectionTest.java
+++ /dev/null
@@ -1,138 +0,0 @@
-// Copyright 2016 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 static com.google.common.truth.Truth.assertThat;
-import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
-import static org.mockito.Mockito.when;
-
-import com.google.common.collect.ImmutableMap;
-import com.google.common.io.ByteStreams;
-import com.google.common.net.MediaType;
-import java.io.File;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.nio.charset.Charset;
-import java.util.Map;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-import org.mockito.Mockito;
-
-/**
- * Tests for @{link HttpConnection}.
- */
-@RunWith(JUnit4.class)
-public class HttpConnectionTest {
-
- @Test
- public void testEncodingSet() throws Exception {
- Map<String, Charset> charsets = Charset.availableCharsets();
- assertThat(charsets).isNotEmpty();
- Map.Entry<String, Charset> entry = charsets.entrySet().iterator().next();
-
- String availableEncoding = entry.getKey();
- Charset availableCharset = entry.getValue();
-
- HttpURLConnection connection = Mockito.mock(HttpURLConnection.class);
- when(connection.getContentEncoding()).thenReturn(availableEncoding);
- Charset charset = HttpConnection.getEncoding(connection);
- assertEquals(availableCharset, charset);
- }
-
- @Test
- public void testInvalidEncoding() throws Exception {
- HttpURLConnection connection = Mockito.mock(HttpURLConnection.class);
- when(connection.getContentEncoding()).thenReturn("This-isn't-a-valid-content-encoding");
- try {
- HttpConnection.getEncoding(connection);
- fail("Expected exception");
- } catch (IOException e) {
- assertThat(e.getMessage()).contains("Got unavailable encoding");
- }
- }
-
- @Test
- public void testContentType() throws Exception {
- HttpURLConnection connection = Mockito.mock(HttpURLConnection.class);
- when(connection.getContentType()).thenReturn(MediaType.HTML_UTF_8.toString());
- Charset charset = HttpConnection.getEncoding(connection);
- assertEquals(UTF_8, charset);
- }
-
- @Test
- public void testInvalidContentType() throws Exception {
- HttpURLConnection connection = Mockito.mock(HttpURLConnection.class);
- when(connection.getContentType()).thenReturn("This-isn't-a-valid-content-type");
- try {
- HttpConnection.getEncoding(connection);
- fail("Expected exception");
- } catch (IOException e) {
- assertThat(e.getMessage()).contains("Got invalid encoding");
- }
- }
-
- @Test
- public void testNoEncodingNorContentType() throws Exception {
- HttpURLConnection connection = Mockito.mock(HttpURLConnection.class);
- Charset charset = HttpConnection.getEncoding(connection);
- assertEquals(UTF_8, charset);
- }
-
- /**
- * Creates a temporary file with the specified {@code fileContents}. The file will be
- * automatically deleted when the JVM exits.
- *
- * @param fileContents the contents of the file
- * @return the {@link File} object representing the temporary file
- */
- private static File createTempFile(byte[] fileContents) throws IOException {
- File temp = File.createTempFile("httpConnectionTest", ".tmp");
- temp.deleteOnExit();
- try (FileOutputStream outputStream = new FileOutputStream(temp)) {
- outputStream.write(fileContents);
- }
- return temp;
- }
-
- @Test
- public void testLocalFileDownload() throws Exception {
- byte[] fileContents = "this is a test".getBytes(UTF_8);
- File temp = createTempFile(fileContents);
- HttpConnection httpConnection =
- HttpConnection.createAndConnect(temp.toURI().toURL(), ImmutableMap.<String, String>of());
-
- assertThat(httpConnection.getContentLength()).isEqualTo(fileContents.length);
-
- byte[] readContents = ByteStreams.toByteArray(httpConnection.getInputStream());
- assertThat(readContents).isEqualTo(fileContents);
- }
-
- @Test
- public void testLocalEmptyFileDownload() throws Exception {
- byte[] fileContents = new byte[0];
- // create a temp file
- File temp = createTempFile(fileContents);
- try {
- HttpConnection.createAndConnect(temp.toURI().toURL(), ImmutableMap.<String, String>of());
- fail("Expected exception");
- } catch (IOException ex) {
- // expected
- }
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorTest.java
new file mode 100644
index 0000000..eae5ad9
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnectorTest.java
@@ -0,0 +1,154 @@
+// Copyright 2016 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 static com.google.common.truth.Truth.assertThat;
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.common.io.ByteStreams;
+import com.google.devtools.build.lib.events.EventHandler;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.net.Proxy;
+import java.net.URL;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/**
+ * Unit tests for {@link HttpConnector}.
+ */
+@RunWith(JUnit4.class)
+public class HttpConnectorTest {
+
+ @Rule
+ public final ExpectedException thrown = ExpectedException.none();
+
+ @Rule
+ public TemporaryFolder testFolder = new TemporaryFolder();
+
+ private final HttpURLConnection connection = mock(HttpURLConnection.class);
+ private final EventHandler eventHandler = mock(EventHandler.class);
+
+ @Test
+ public void testLocalFileDownload() throws Exception {
+ byte[] fileContents = "this is a test".getBytes(UTF_8);
+ assertThat(
+ ByteStreams.toByteArray(
+ HttpConnector.connect(
+ createTempFile(fileContents).toURI().toURL(),
+ Proxy.NO_PROXY,
+ eventHandler)))
+ .isEqualTo(fileContents);
+ }
+
+ @Test
+ public void missingLocationInRedirect_throwsIOException() throws Exception {
+ thrown.expect(IOException.class);
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ HttpConnector.getLocation(connection);
+ }
+
+ @Test
+ public void absoluteLocationInRedirect_returnsNewUrl() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("http://new.example/hi");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("http://new.example/hi"));
+ }
+
+ @Test
+ public void redirectOnlyHasPath_mergesHostFromOriginalUrl() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("/hi");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("http://lol.example/hi"));
+ }
+
+ @Test
+ public void locationOnlyHasPathWithoutSlash_failsToMerge() throws Exception {
+ thrown.expect(IOException.class);
+ thrown.expectMessage("Could not merge");
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("omg");
+ HttpConnector.getLocation(connection);
+ }
+
+ @Test
+ public void locationHasFragment_prefersNewFragment() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://lol.example#a"));
+ when(connection.getHeaderField("Location")).thenReturn("http://new.example/hi#b");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("http://new.example/hi#b"));
+ }
+
+ @Test
+ public void locationHasNoFragmentButOriginalDoes_mergesOldFragment() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://lol.example#a"));
+ when(connection.getHeaderField("Location")).thenReturn("http://new.example/hi");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("http://new.example/hi#a"));
+ }
+
+ @Test
+ public void oldUrlHasPasswordRedirectingToSameDomain_mergesPassword() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://a:b@lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("http://lol.example/hi");
+ assertThat(HttpConnector.getLocation(connection))
+ .isEqualTo(new URL("http://a:b@lol.example/hi"));
+ when(connection.getURL()).thenReturn(new URL("http://a:b@lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("/hi");
+ assertThat(HttpConnector.getLocation(connection))
+ .isEqualTo(new URL("http://a:b@lol.example/hi"));
+ }
+
+ @Test
+ public void oldUrlHasPasswordRedirectingToNewServer_doesntMergePassword() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://a:b@lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("http://new.example/hi");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("http://new.example/hi"));
+ when(connection.getURL()).thenReturn(new URL("http://a:b@lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("http://lol.example:81/hi");
+ assertThat(HttpConnector.getLocation(connection))
+ .isEqualTo(new URL("http://lol.example:81/hi"));
+ }
+
+ @Test
+ public void redirectToFtp_throwsIOException() throws Exception {
+ thrown.expect(IOException.class);
+ thrown.expectMessage("Bad Location");
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("ftp://lol.example");
+ HttpConnector.getLocation(connection);
+ }
+
+ @Test
+ public void redirectToHttps_works() throws Exception {
+ when(connection.getURL()).thenReturn(new URL("http://lol.example"));
+ when(connection.getHeaderField("Location")).thenReturn("https://lol.example");
+ assertThat(HttpConnector.getLocation(connection)).isEqualTo(new URL("https://lol.example"));
+ }
+
+ private File createTempFile(byte[] fileContents) throws IOException {
+ File temp = testFolder.newFile();
+ try (FileOutputStream outputStream = new FileOutputStream(temp)) {
+ outputStream.write(fileContents);
+ }
+ return temp;
+ }
+}
