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

4 files changed, 183 insertions, 140 deletions

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 f29ceb4e7a..988efb8132 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 @@ filegroup(
 )
 
 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 0000000000..1a48a1ceb5
--- /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 93d9f61ddd..0000000000
--- 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 0000000000..eae5ad96e1
--- /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;
+  }
+}