From e1ea045cd4ec02a695bb137cfb33e8bed59d58af Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Fri, 1 Nov 2019 16:04:39 -0400
Subject: [PATCH 1/6] Rename Connection to FailoverHttpClient

---
 ...onnection.java => FailoverHttpClient.java} |  10 +-
 .../AuthenticationMethodRetriever.java        |   6 +-
 .../jib/registry/RegistryAuthenticator.java   |   8 +-
 .../tools/jib/registry/RegistryClient.java    |   6 +-
 .../jib/registry/RegistryEndpointCaller.java  |   6 +-
 ...iloverHttpClientProxyCredentialsTest.java} |  17 +-
 ...nTest.java => FailoverHttpClientTest.java} | 159 +++++++++---------
 ... => WithServerFailoverHttpClientTest.java} |  17 +-
 .../AuthenticationMethodRetrieverTest.java    |   4 +-
 .../registry/RegistryAuthenticatorTest.java   |   6 +-
 .../registry/RegistryEndpointCallerTest.java  |   4 +-
 11 files changed, 123 insertions(+), 120 deletions(-)
 rename jib-core/src/main/java/com/google/cloud/tools/jib/http/{Connection.java => FailoverHttpClient.java} (98%)
 rename jib-core/src/test/java/com/google/cloud/tools/jib/http/{ConnectionWithProxyCredentialsTest.java => FailoverHttpClientProxyCredentialsTest.java} (92%)
 rename jib-core/src/test/java/com/google/cloud/tools/jib/http/{ConnectionTest.java => FailoverHttpClientTest.java} (91%)
 rename jib-core/src/test/java/com/google/cloud/tools/jib/http/{WithServerConnectionTest.java => WithServerFailoverHttpClientTest.java} (89%)

diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java b/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
similarity index 98%
rename from jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java
rename to jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
index f3671a6188..46bf2615ee 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
@@ -68,7 +68,7 @@
  * This failover behavior is similar to how the Docker client works:
  * https://docs.docker.com/registry/insecure/#deploy-a-plain-http-registry
  */
-public class Connection { // TODO: rename to TlsFailoverHttpClient
+public class FailoverHttpClient {
 
   private static boolean isHttpsProtocol(URL url) {
     return "https".equals(url.getProtocol());
@@ -141,7 +141,7 @@ private static void addProxyCredentials(ApacheHttpTransport transport, String pr
   private final Supplier<HttpTransport> secureHttpTransportFactory;
   private final Supplier<HttpTransport> insecureHttpTransportFactory;
 
-  public Connection(
+  public FailoverHttpClient(
       boolean enableHttpAndInsecureFailover,
       boolean sendAuthorizationOverHttp,
       Consumer<LogEvent> logger) {
@@ -149,12 +149,12 @@ public Connection(
         enableHttpAndInsecureFailover,
         sendAuthorizationOverHttp,
         logger,
-        Connection::getSecureHttpTransport,
-        Connection::getInsecureHttpTransport);
+        FailoverHttpClient::getSecureHttpTransport,
+        FailoverHttpClient::getInsecureHttpTransport);
   }
 
   @VisibleForTesting
-  Connection(
+  FailoverHttpClient(
       boolean enableHttpAndInsecureFailover,
       boolean sendAuthorizationOverHttp,
       Consumer<LogEvent> logger,
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java
index 5176cec504..fa2adfb432 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetriever.java
@@ -20,7 +20,7 @@
 import com.google.api.client.http.HttpStatusCodes;
 import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.http.ResponseException;
 import java.net.MalformedURLException;
@@ -36,12 +36,12 @@ class AuthenticationMethodRetriever
 
   private final RegistryEndpointRequestProperties registryEndpointRequestProperties;
   private final String userAgent;
-  private final Connection httpClient;
+  private final FailoverHttpClient httpClient;
 
   AuthenticationMethodRetriever(
       RegistryEndpointRequestProperties registryEndpointRequestProperties,
       String userAgent,
-      Connection httpClient) {
+      FailoverHttpClient httpClient) {
     this.registryEndpointRequestProperties = registryEndpointRequestProperties;
     this.userAgent = userAgent;
     this.httpClient = httpClient;
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java
index 8cd76441ab..7640f866ef 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryAuthenticator.java
@@ -25,7 +25,7 @@
 import com.google.cloud.tools.jib.global.JibSystemProperties;
 import com.google.cloud.tools.jib.http.Authorization;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Request;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.http.ResponseException;
@@ -70,7 +70,7 @@ static Optional<RegistryAuthenticator> fromAuthenticationMethod(
       String authenticationMethod,
       RegistryEndpointRequestProperties registryEndpointRequestProperties,
       String userAgent,
-      Connection httpClient)
+      FailoverHttpClient httpClient)
       throws RegistryAuthenticationFailedException {
     // If the authentication method starts with 'basic ' (case insensitive), no registry
     // authentication is needed.
@@ -143,14 +143,14 @@ private String getToken() {
   private final String realm;
   private final String service;
   private final String userAgent;
-  private final Connection httpClient;
+  private final FailoverHttpClient httpClient;
 
   private RegistryAuthenticator(
       String realm,
       String service,
       RegistryEndpointRequestProperties registryEndpointRequestProperties,
       String userAgent,
-      Connection httpClient) {
+      FailoverHttpClient httpClient) {
     this.realm = realm;
     this.service = service;
     this.registryEndpointRequestProperties = registryEndpointRequestProperties;
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java
index 1214f6aa3b..cdcda32702 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryClient.java
@@ -28,7 +28,7 @@
 import com.google.cloud.tools.jib.event.EventHandlers;
 import com.google.cloud.tools.jib.global.JibSystemProperties;
 import com.google.cloud.tools.jib.http.Authorization;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate;
 import com.google.cloud.tools.jib.image.json.ManifestTemplate;
@@ -244,7 +244,7 @@ static Multimap<String, String> decodeTokenRepositoryGrants(String token) {
   @Nullable private final Authorization authorization;
   private final RegistryEndpointRequestProperties registryEndpointRequestProperties;
   private final String userAgent;
-  private final Connection httpClient;
+  private final FailoverHttpClient httpClient;
 
   /**
    * Instantiate with {@link #factory}.
@@ -266,7 +266,7 @@ private RegistryClient(
     this.registryEndpointRequestProperties = registryEndpointRequestProperties;
     this.userAgent = userAgent;
     this.httpClient =
-        new Connection(
+        new FailoverHttpClient(
             allowInsecureRegistries,
             JibSystemProperties.sendCredentialsOverHttp(),
             eventHandlers::dispatch);
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
index 49cc444716..eb5d0fd98a 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
@@ -24,7 +24,7 @@
 import com.google.cloud.tools.jib.event.EventHandlers;
 import com.google.cloud.tools.jib.global.JibSystemProperties;
 import com.google.cloud.tools.jib.http.Authorization;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Request;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.http.ResponseException;
@@ -74,7 +74,7 @@ static boolean isBrokenPipe(IOException original) {
   private final RegistryEndpointProvider<T> registryEndpointProvider;
   @Nullable private final Authorization authorization;
   private final RegistryEndpointRequestProperties registryEndpointRequestProperties;
-  private final Connection httpClient;
+  private final FailoverHttpClient httpClient;
 
   /**
    * Constructs with parameters for making the request.
@@ -93,7 +93,7 @@ static boolean isBrokenPipe(IOException original) {
       RegistryEndpointProvider<T> registryEndpointProvider,
       @Nullable Authorization authorization,
       RegistryEndpointRequestProperties registryEndpointRequestProperties,
-      Connection httpClient) {
+      FailoverHttpClient httpClient) {
     this.eventHandlers = eventHandlers;
     this.userAgent = userAgent;
     this.registryEndpointProvider = registryEndpointProvider;
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java
similarity index 92%
rename from jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java
rename to jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java
index 385a05b21b..615f3f0cfb 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java
@@ -27,9 +27,8 @@
 import org.junit.Test;
 import org.junit.contrib.java.lang.system.RestoreSystemProperties;
 
-/** Tests for {@link Connection} with setting proxy credentials. */
-// TODO: rename to TlsFailoverHttpClientProxyCredentialsTest
-public class ConnectionWithProxyCredentialsTest {
+/** Tests for {@link FailoverHttpClient} with setting proxy credentials. */
+public class FailoverHttpClientProxyCredentialsTest {
 
   private static final ImmutableList<String> proxyProperties =
       ImmutableList.of(
@@ -53,7 +52,7 @@ public void setUp() {
 
   @Test
   public void testAddProxyCredentials_undefined() {
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
     Assert.assertNull(credentials);
@@ -71,7 +70,7 @@ public void testAddProxyCredentials() {
     System.setProperty("https.proxyUser", "s-user");
     System.setProperty("https.proxyPassword", "s-pass");
 
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials httpCredentials =
         httpClient.getCredentialsProvider().getCredentials(new AuthScope("http://localhost", 1080));
@@ -94,7 +93,7 @@ public void testAddProxyCredentials_defaultPorts() {
     System.setProperty("https.proxyUser", "s-user");
     System.setProperty("https.proxyPassword", "s-pass");
 
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials httpCredentials =
         httpClient.getCredentialsProvider().getCredentials(new AuthScope("http://localhost", 80));
@@ -115,7 +114,7 @@ public void testAddProxyCredentials_hostUndefined() {
     System.setProperty("https.proxyUser", "s-user");
     System.setProperty("https.proxyPassword", "s-pass");
 
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
     Assert.assertNull(credentials);
@@ -129,7 +128,7 @@ public void testAddProxyCredentials_userUndefined() {
     System.setProperty("https.proxyHost", "https://host.com");
     System.setProperty("https.proxyPassword", "s-pass");
 
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
     Assert.assertNull(credentials);
@@ -143,7 +142,7 @@ public void testAddProxyCredentials_passwordUndefined() {
     System.setProperty("https.proxyHost", "https://host.com");
     System.setProperty("https.proxyUser", "s-user");
 
-    Connection.addProxyCredentials(transport);
+    FailoverHttpClient.addProxyCredentials(transport);
     DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
     Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
     Assert.assertNull(credentials);
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientTest.java
similarity index 91%
rename from jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java
rename to jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientTest.java
index 6049f28bf8..f796dd6633 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientTest.java
@@ -47,14 +47,14 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
-/** Tests for {@link Connection}. */
+/** Tests for {@link FailoverHttpClient}. */
 @RunWith(MockitoJUnitRunner.class)
-public class ConnectionTest { // TODO: rename to TlsFailoverHttpClient
+public class FailoverHttpClientTest {
 
   @FunctionalInterface
   private interface CallFunction {
 
-    Response call(Connection httpClient, URL url, Request request) throws IOException;
+    Response call(FailoverHttpClient httpClient, URL url, Request request) throws IOException;
   }
 
   @Mock private HttpTransport mockHttpTransport;
@@ -79,29 +79,19 @@ public void setUp() throws IOException {
     Mockito.when(mockHttpResponse.getContent()).thenReturn(inStream);
   }
 
-  private Connection newHttpClient(boolean insecure, boolean authOverHttp) throws IOException {
-    setUpMocks(mockHttpTransport, mockHttpRequestFactory, mockHttpRequest);
-    if (insecure) {
-      setUpMocks(
-          mockInsecureHttpTransport, mockInsecureHttpRequestFactory, mockInsecureHttpRequest);
-    }
-    return new Connection(
-        insecure, authOverHttp, logger, () -> mockHttpTransport, () -> mockInsecureHttpTransport);
-  }
-
   @Test
   public void testGet() throws IOException {
-    verifyCall(HttpMethods.GET, Connection::get);
+    verifyCall(HttpMethods.GET, FailoverHttpClient::get);
   }
 
   @Test
   public void testPost() throws IOException {
-    verifyCall(HttpMethods.POST, Connection::post);
+    verifyCall(HttpMethods.POST, FailoverHttpClient::post);
   }
 
   @Test
   public void testPut() throws IOException {
-    verifyCall(HttpMethods.PUT, Connection::put);
+    verifyCall(HttpMethods.PUT, FailoverHttpClient::put);
   }
 
   @Test
@@ -114,68 +104,17 @@ public void testHttpTimeout_doNotSetByDefault() throws IOException {
 
   @Test
   public void testHttpTimeout() throws IOException {
-    Connection httpClient = newHttpClient(false, false);
+    FailoverHttpClient httpClient = newHttpClient(false, false);
     try (Response ignored = httpClient.get(fakeUrl.toURL(), fakeRequest(5982))) {}
 
     Mockito.verify(mockHttpRequest).setConnectTimeout(5982);
     Mockito.verify(mockHttpRequest).setReadTimeout(5982);
   }
 
-  private Request fakeRequest(Integer httpTimeout) {
-    return Request.builder()
-        .setAccept(Arrays.asList("fake.accept", "another.fake.accept"))
-        .setUserAgent("fake user agent")
-        .setBody(
-            new BlobHttpContent(Blobs.from("crepecake"), "fake.content.type", totalByteCount::add))
-        .setAuthorization(Authorization.fromBasicCredentials("fake-username", "fake-secret"))
-        .setHttpTimeout(httpTimeout)
-        .build();
-  }
-
-  private void setUpMocks(
-      HttpTransport mockHttpTransport,
-      HttpRequestFactory mockHttpRequestFactory,
-      HttpRequest mockHttpRequest)
-      throws IOException {
-    Mockito.when(mockHttpTransport.createRequestFactory()).thenReturn(mockHttpRequestFactory);
-    Mockito.when(
-            mockHttpRequestFactory.buildRequest(Mockito.any(), urlCaptor.capture(), Mockito.any()))
-        .thenReturn(mockHttpRequest);
-
-    Mockito.when(mockHttpRequest.setHeaders(httpHeadersCaptor.capture()))
-        .thenReturn(mockHttpRequest);
-    Mockito.when(mockHttpRequest.setConnectTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
-    Mockito.when(mockHttpRequest.setReadTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
-    Mockito.when(mockHttpRequest.execute()).thenReturn(mockHttpResponse);
-  }
-
-  private void verifyCall(String httpMethod, CallFunction callFunction) throws IOException {
-    Connection httpClient = newHttpClient(false, false);
-    try (Response ignored = callFunction.call(httpClient, fakeUrl.toURL(), fakeRequest(null))) {}
-
-    Assert.assertEquals(
-        "fake.accept,another.fake.accept", httpHeadersCaptor.getValue().getAccept());
-    Assert.assertEquals("fake user agent", httpHeadersCaptor.getValue().getUserAgent());
-    // Base64 representation of "fake-username:fake-secret"
-    Assert.assertEquals(
-        "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==",
-        httpHeadersCaptor.getValue().getAuthorization());
-
-    Mockito.verify(mockHttpRequestFactory)
-        .buildRequest(Mockito.eq(httpMethod), Mockito.eq(fakeUrl), blobHttpContentCaptor.capture());
-    Assert.assertEquals("fake.content.type", blobHttpContentCaptor.getValue().getType());
-
-    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
-    blobHttpContentCaptor.getValue().writeTo(byteArrayOutputStream);
-
-    Assert.assertEquals("crepecake", byteArrayOutputStream.toString(StandardCharsets.UTF_8.name()));
-    Assert.assertEquals("crepecake".length(), totalByteCount.longValue());
-  }
-
   @Test
   public void testGet_nonHttpsServer_insecureConnectionAndFailoverDisabled()
       throws MalformedURLException, IOException {
-    Connection httpClient = newHttpClient(false, false);
+    FailoverHttpClient httpClient = newHttpClient(false, false);
     try (Response response = httpClient.get(new URL("http://plain.http"), fakeRequest(null))) {
       Assert.fail("Should disallow non-HTTP attempt");
     } catch (SSLException ex) {
@@ -186,7 +125,7 @@ public void testGet_nonHttpsServer_insecureConnectionAndFailoverDisabled()
 
   @Test
   public void testCall_secureClientOnUnverifiableServer() throws IOException {
-    Connection httpClient = newHttpClient(false, false);
+    FailoverHttpClient httpClient = newHttpClient(false, false);
 
     Mockito.when(mockHttpRequest.execute()).thenThrow(new SSLPeerUnverifiedException("unverified"));
 
@@ -200,7 +139,7 @@ public void testCall_secureClientOnUnverifiableServer() throws IOException {
 
   @Test
   public void testGet_insecureClientOnUnverifiableServer() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Mockito.when(mockHttpRequest.execute()).thenThrow(new SSLPeerUnverifiedException(""));
 
@@ -223,7 +162,7 @@ public void testGet_insecureClientOnUnverifiableServer() throws IOException {
 
   @Test
   public void testGet_insecureClientOnHttpServer() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Mockito.when(mockHttpRequest.execute())
         .thenThrow(new SSLException("")) // server is not HTTPS
@@ -256,7 +195,7 @@ public void testGet_insecureClientOnHttpServer() throws IOException {
 
   @Test
   public void testGet_insecureClientOnHttpServerAndNoPortSpecified() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Mockito.when(mockHttpRequest.execute())
         .thenThrow(new ConnectException()) // server is not listening on 443
@@ -283,7 +222,7 @@ public void testGet_insecureClientOnHttpServerAndNoPortSpecified() throws IOExce
 
   @Test
   public void testGet_secureClientOnNonListeningServerAndNoPortSpecified() throws IOException {
-    Connection httpClient = newHttpClient(false, false);
+    FailoverHttpClient httpClient = newHttpClient(false, false);
 
     Mockito.when(mockHttpRequest.execute())
         .thenThrow(new ConnectException("my exception")); // server not listening on 443
@@ -303,7 +242,7 @@ public void testGet_secureClientOnNonListeningServerAndNoPortSpecified() throws
 
   @Test
   public void testGet_insecureClientOnNonListeningServerAndPortSpecified() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Mockito.when(mockHttpRequest.execute())
         .thenThrow(new ConnectException("my exception")); // server is not listening on 5000
@@ -324,7 +263,7 @@ public void testGet_insecureClientOnNonListeningServerAndPortSpecified() throws
 
   @Test
   public void testGet_timeoutFromConnectException() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Mockito.when(mockHttpRequest.execute()).thenThrow(new ConnectException("Connection timed out"));
 
@@ -344,7 +283,7 @@ public void testGet_timeoutFromConnectException() throws IOException {
 
   @Test
   public void testGet_doNotSendCredentialsOverHttp() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     // make it fall back to HTTP
     Mockito.when(mockHttpRequest.execute())
@@ -367,7 +306,7 @@ public void testGet_doNotSendCredentialsOverHttp() throws IOException {
 
   @Test
   public void testGet_sendCredentialsOverHttp() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, true); // sendCredentialsOverHttp
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, true); // sendCredentialsOverHttp
 
     try (Response response =
         insecureHttpClient.get(new URL("http://plain.http"), fakeRequest(null))) {}
@@ -381,7 +320,7 @@ public void testGet_sendCredentialsOverHttp() throws IOException {
 
   @Test
   public void testGet_originalRequestHeaderUntouchedWhenClearingHeader() throws IOException {
-    Connection insecureHttpClient = newHttpClient(true, false);
+    FailoverHttpClient insecureHttpClient = newHttpClient(true, false);
 
     Request request = fakeRequest(null);
     try (Response response = insecureHttpClient.get(new URL("http://plain.http"), request)) {}
@@ -393,4 +332,66 @@ public void testGet_originalRequestHeaderUntouchedWhenClearingHeader() throws IO
     Assert.assertEquals(
         "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==", request.getHeaders().getAuthorization());
   }
+
+  private void setUpMocks(
+      HttpTransport mockHttpTransport,
+      HttpRequestFactory mockHttpRequestFactory,
+      HttpRequest mockHttpRequest)
+      throws IOException {
+    Mockito.when(mockHttpTransport.createRequestFactory()).thenReturn(mockHttpRequestFactory);
+    Mockito.when(
+            mockHttpRequestFactory.buildRequest(Mockito.any(), urlCaptor.capture(), Mockito.any()))
+        .thenReturn(mockHttpRequest);
+
+    Mockito.when(mockHttpRequest.setHeaders(httpHeadersCaptor.capture()))
+        .thenReturn(mockHttpRequest);
+    Mockito.when(mockHttpRequest.setConnectTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
+    Mockito.when(mockHttpRequest.setReadTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
+    Mockito.when(mockHttpRequest.execute()).thenReturn(mockHttpResponse);
+  }
+
+  private FailoverHttpClient newHttpClient(boolean insecure, boolean authOverHttp)
+      throws IOException {
+    setUpMocks(mockHttpTransport, mockHttpRequestFactory, mockHttpRequest);
+    if (insecure) {
+      setUpMocks(
+          mockInsecureHttpTransport, mockInsecureHttpRequestFactory, mockInsecureHttpRequest);
+    }
+    return new FailoverHttpClient(
+        insecure, authOverHttp, logger, () -> mockHttpTransport, () -> mockInsecureHttpTransport);
+  }
+
+  private Request fakeRequest(Integer httpTimeout) {
+    return Request.builder()
+        .setAccept(Arrays.asList("fake.accept", "another.fake.accept"))
+        .setUserAgent("fake user agent")
+        .setBody(
+            new BlobHttpContent(Blobs.from("crepecake"), "fake.content.type", totalByteCount::add))
+        .setAuthorization(Authorization.fromBasicCredentials("fake-username", "fake-secret"))
+        .setHttpTimeout(httpTimeout)
+        .build();
+  }
+
+  private void verifyCall(String httpMethod, CallFunction callFunction) throws IOException {
+    FailoverHttpClient httpClient = newHttpClient(false, false);
+    try (Response ignored = callFunction.call(httpClient, fakeUrl.toURL(), fakeRequest(null))) {}
+
+    Assert.assertEquals(
+        "fake.accept,another.fake.accept", httpHeadersCaptor.getValue().getAccept());
+    Assert.assertEquals("fake user agent", httpHeadersCaptor.getValue().getUserAgent());
+    // Base64 representation of "fake-username:fake-secret"
+    Assert.assertEquals(
+        "Basic ZmFrZS11c2VybmFtZTpmYWtlLXNlY3JldA==",
+        httpHeadersCaptor.getValue().getAuthorization());
+
+    Mockito.verify(mockHttpRequestFactory)
+        .buildRequest(Mockito.eq(httpMethod), Mockito.eq(fakeUrl), blobHttpContentCaptor.capture());
+    Assert.assertEquals("fake.content.type", blobHttpContentCaptor.getValue().getType());
+
+    ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+    blobHttpContentCaptor.getValue().writeTo(byteArrayOutputStream);
+
+    Assert.assertEquals("crepecake", byteArrayOutputStream.toString(StandardCharsets.UTF_8.name()));
+    Assert.assertEquals("crepecake".length(), totalByteCount.longValue());
+  }
 }
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
similarity index 89%
rename from jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java
rename to jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
index ed2cd1063d..471b74002a 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerConnectionTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
@@ -36,9 +36,9 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
-/** Tests for {@link Connection} using an actual local server. */
+/** Tests for {@link FailoverHttpClient} using an actual local server. */
 @RunWith(MockitoJUnitRunner.class)
-public class WithServerConnectionTest { // TODO: rename to WithServerTlsFailoverHttpClientTest
+public class WithServerFailoverHttpClientTest {
 
   @Rule public final RestoreSystemProperties systemPropertyRestorer = new RestoreSystemProperties();
 
@@ -49,7 +49,8 @@ public class WithServerConnectionTest { // TODO: rename to WithServerTlsFailover
   @Test
   public void testGet()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
-    Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger);
+    FailoverHttpClient insecureHttpClient =
+        new FailoverHttpClient(true /*insecure*/, false, logger);
     try (TestWebServer server = new TestWebServer(false);
         Response response = insecureHttpClient.get(new URL(server.getEndpoint()), request)) {
 
@@ -64,7 +65,7 @@ public void testGet()
   @Test
   public void testSecureConnectionOnInsecureHttpsServer()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
-    Connection secureHttpClient = new Connection(false /*secure*/, false, logger);
+    FailoverHttpClient secureHttpClient = new FailoverHttpClient(false /*secure*/, false, logger);
     try (TestWebServer server = new TestWebServer(true);
         Response ignored = secureHttpClient.get(new URL(server.getEndpoint()), request)) {
       Assert.fail("Should fail if cannot verify peer");
@@ -77,7 +78,8 @@ public void testSecureConnectionOnInsecureHttpsServer()
   @Test
   public void testInsecureConnection_insecureHttpsFailover()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
-    Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger);
+    FailoverHttpClient insecureHttpClient =
+        new FailoverHttpClient(true /*insecure*/, false, logger);
     try (TestWebServer server = new TestWebServer(true, 2);
         Response response = insecureHttpClient.get(new URL(server.getEndpoint()), request)) {
 
@@ -96,7 +98,8 @@ public void testInsecureConnection_insecureHttpsFailover()
   @Test
   public void testInsecureConnection_plainHttpFailover()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
-    Connection insecureHttpClient = new Connection(true /*insecure*/, false, logger);
+    FailoverHttpClient insecureHttpClient =
+        new FailoverHttpClient(true /*insecure*/, false, logger);
     try (TestWebServer server = new TestWebServer(false, 3)) {
       String httpsUrl = server.getEndpoint().replace("http://", "https://");
       try (Response response = insecureHttpClient.get(new URL(httpsUrl), request)) {
@@ -127,7 +130,7 @@ public void testProxyCredentialProperties()
             + "Content-Length: 0\n\n";
     String targetServerResponse = "HTTP/1.1 200 OK\nContent-Length:12\n\nHello World!";
 
-    Connection httpClient = new Connection(true /*insecure*/, false, logger);
+    FailoverHttpClient httpClient = new FailoverHttpClient(true /*insecure*/, false, logger);
     try (TestWebServer server =
         new TestWebServer(false, Arrays.asList(proxyResponse, targetServerResponse), 1)) {
       System.setProperty("http.proxyHost", "localhost");
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java
index 9e0ccfc0d6..a6cc88924a 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/AuthenticationMethodRetrieverTest.java
@@ -19,7 +19,7 @@
 import com.google.api.client.http.HttpHeaders;
 import com.google.api.client.http.HttpMethods;
 import com.google.api.client.http.HttpStatusCodes;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.http.ResponseException;
 import java.net.MalformedURLException;
@@ -39,7 +39,7 @@ public class AuthenticationMethodRetrieverTest {
 
   @Mock private ResponseException mockResponseException;
   @Mock private HttpHeaders mockHeaders;
-  @Mock private Connection httpClient;
+  @Mock private FailoverHttpClient httpClient;
 
   private final RegistryEndpointRequestProperties fakeRegistryEndpointRequestProperties =
       new RegistryEndpointRequestProperties("someServerUrl", "someImageName");
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java
index 268dda3475..45431056fc 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryAuthenticatorTest.java
@@ -18,7 +18,7 @@
 
 import com.google.cloud.tools.jib.api.Credential;
 import com.google.cloud.tools.jib.api.RegistryAuthenticationFailedException;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.http.ResponseException;
 import com.google.cloud.tools.jib.http.TestWebServer;
@@ -47,7 +47,7 @@ public class RegistryAuthenticatorTest {
   private final RegistryEndpointRequestProperties registryEndpointRequestProperties =
       new RegistryEndpointRequestProperties("someserver", "someimage");
 
-  @Mock private Connection httpClient;
+  @Mock private FailoverHttpClient httpClient;
   @Mock private Response response;
 
   @Captor private ArgumentCaptor<URL> urlCaptor;
@@ -240,7 +240,7 @@ public void testUserAgent()
                     "Bearer realm=\"" + server.getEndpoint() + "\"",
                     registryEndpointRequestProperties,
                     "Competent-Agent",
-                    new Connection(true, false, ignored -> {}))
+                    new FailoverHttpClient(true, false, ignored -> {}))
                 .get();
         authenticator.authenticatePush(null);
       } catch (RegistryAuthenticationFailedException ex) {
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
index bfebf7455b..f9b9c6f4bf 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
@@ -26,7 +26,7 @@
 import com.google.cloud.tools.jib.global.JibSystemProperties;
 import com.google.cloud.tools.jib.http.Authorization;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
-import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.FailoverHttpClient;
 import com.google.cloud.tools.jib.http.Request;
 import com.google.cloud.tools.jib.http.RequestWrapper;
 import com.google.cloud.tools.jib.http.Response;
@@ -109,7 +109,7 @@ private static ResponseException mockResponseException(
   @Rule public final RestoreSystemProperties systemPropertyRestorer = new RestoreSystemProperties();
 
   @Mock private EventHandlers mockEventHandlers;
-  @Mock private Connection mockHttpClient;
+  @Mock private FailoverHttpClient mockHttpClient;
   @Mock private Response mockResponse;
 
   private RegistryEndpointCaller<String> endpointCaller;

From 2f2c8592ed7a7f0db810a1c0604269fb0e0ba736 Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Fri, 1 Nov 2019 16:12:50 -0400
Subject: [PATCH 2/6] Remove "default" interface implementation

---
 .../cloud/tools/jib/registry/BlobPuller.java  |  7 +++++++
 .../cloud/tools/jib/registry/BlobPusher.java  | 19 +++++++++++++++++++
 .../tools/jib/registry/ManifestPuller.java    |  7 +++++++
 .../registry/RegistryEndpointProvider.java    |  9 ++++-----
 .../registry/RegistryEndpointCallerTest.java  |  6 ++++++
 5 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPuller.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPuller.java
index 04dd6b4bc3..64bf0ff1d6 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPuller.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPuller.java
@@ -23,6 +23,7 @@
 import com.google.cloud.tools.jib.http.BlobHttpContent;
 import com.google.cloud.tools.jib.http.NotifyingOutputStream;
 import com.google.cloud.tools.jib.http.Response;
+import com.google.cloud.tools.jib.http.ResponseException;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.net.MalformedURLException;
@@ -114,4 +115,10 @@ public String getActionDescription() {
         + " with digest "
         + blobDigest;
   }
+
+  @Override
+  public Void handleHttpResponseException(ResponseException responseException)
+      throws ResponseException, RegistryErrorException {
+    throw responseException;
+  }
 }
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPusher.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPusher.java
index 917e573339..dd16cca6cc 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPusher.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/BlobPusher.java
@@ -23,6 +23,7 @@
 import com.google.cloud.tools.jib.blob.Blob;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
 import com.google.cloud.tools.jib.http.Response;
+import com.google.cloud.tools.jib.http.ResponseException;
 import com.google.common.net.MediaType;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
@@ -107,6 +108,12 @@ public String getHttpMethod() {
     public String getActionDescription() {
       return BlobPusher.this.getActionDescription();
     }
+
+    @Override
+    public Optional<URL> handleHttpResponseException(ResponseException responseException)
+        throws ResponseException, RegistryErrorException {
+      throw responseException;
+    }
   }
 
   /** Writes the BLOB content to the upload location. */
@@ -152,6 +159,12 @@ private Writer(URL location, Consumer<Long> writtenByteCountListener) {
       this.location = location;
       this.writtenByteCountListener = writtenByteCountListener;
     }
+
+    @Override
+    public URL handleHttpResponseException(ResponseException responseException)
+        throws ResponseException, RegistryErrorException {
+      throw responseException;
+    }
   }
 
   /** Commits the written BLOB. */
@@ -194,6 +207,12 @@ public String getActionDescription() {
     private Committer(URL location) {
       this.location = location;
     }
+
+    @Override
+    public Void handleHttpResponseException(ResponseException responseException)
+        throws ResponseException, RegistryErrorException {
+      throw responseException;
+    }
   }
 
   BlobPusher(
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPuller.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPuller.java
index b8343d72d8..c8438ee257 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPuller.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/ManifestPuller.java
@@ -23,6 +23,7 @@
 import com.google.cloud.tools.jib.hash.Digests;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
 import com.google.cloud.tools.jib.http.Response;
+import com.google.cloud.tools.jib.http.ResponseException;
 import com.google.cloud.tools.jib.image.json.ManifestTemplate;
 import com.google.cloud.tools.jib.image.json.OCIManifestTemplate;
 import com.google.cloud.tools.jib.image.json.UnknownManifestFormatException;
@@ -160,4 +161,10 @@ private T getManifestTemplateFromJson(String jsonString)
     throw new UnknownManifestFormatException(
         "Unknown schemaVersion: " + schemaVersion + " - only 1 and 2 are supported");
   }
+
+  @Override
+  public ManifestAndDigest<T> handleHttpResponseException(ResponseException responseException)
+      throws ResponseException, RegistryErrorException {
+    throw responseException;
+  }
 }
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java
index d6a2e7c502..6ae0842519 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointProvider.java
@@ -54,17 +54,16 @@ interface RegistryEndpointProvider<T> {
   T handleResponse(Response response) throws IOException, RegistryException;
 
   /**
-   * Handles an {@link ResponseException} that occurs.
+   * Handles an {@link ResponseException} that occurs. Implementation must re-throw the given
+   * exception if it did not conclusively handled the response exception.
    *
    * @param responseException the {@link ResponseException} to handle
    * @throws HttpResponseException {@code responseException} if {@code responseException} could not
    *     be handled
    * @throws RegistryErrorException if there is an error with a remote registry
    */
-  default T handleHttpResponseException(ResponseException responseException)
-      throws ResponseException, RegistryErrorException {
-    throw responseException;
-  }
+  T handleHttpResponseException(ResponseException responseException)
+      throws ResponseException, RegistryErrorException;
 
   /**
    * @return a description of the registry action performed, used in error messages to describe the
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
index f9b9c6f4bf..2179cd5b2b 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
@@ -96,6 +96,12 @@ public String handleResponse(Response response) throws IOException {
     public String getActionDescription() {
       return "actionDescription";
     }
+
+    @Override
+    public String handleHttpResponseException(ResponseException responseException)
+        throws ResponseException, RegistryErrorException {
+      throw responseException;
+    }
   }
 
   private static ResponseException mockResponseException(

From c4fc87bd9a7d06ef61995d2ba1fc8b8bf687b0f1 Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Fri, 1 Nov 2019 18:16:57 -0400
Subject: [PATCH 3/6] Upgrade google-http-client to 1.33.0 (v2)

---
 build.gradle                                  |   7 +-
 jib-core/build.gradle                         |   1 +
 .../tools/jib/http/FailoverHttpClient.java    |  57 ++-----
 .../jib/registry/RegistryEndpointCaller.java  |   9 +-
 ...ailoverHttpClientProxyCredentialsTest.java | 150 ------------------
 .../WithServerFailoverHttpClientTest.java     |  40 +++++
 6 files changed, 62 insertions(+), 202 deletions(-)
 delete mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java

diff --git a/build.gradle b/build.gradle
index bad6e6b992..0acb92475d 100644
--- a/build.gradle
+++ b/build.gradle
@@ -46,9 +46,10 @@ subprojects {
     // For Google libraries, check <http-client-bom.version>, <google.auth.version>, <guava.version>,
     // ... in https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/pom.xml
     // for best compatibility.
-    GOOGLE_HTTP_CLIENT: '1.27.0',
-    GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP: '0.16.2',
-    GUAVA: '28.0-jre',
+    GOOGLE_HTTP_CLIENT: '1.33.0',
+    GOOGLE_HTTP_CLIENT_APACHE_V2: '1.33.0',
+    GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP: '0.18.0',
+    GUAVA: '28.1-jre',
 
     COMMONS_COMPRESS: '1.19',
     JACKSON_DATABIND: '2.9.10',
diff --git a/jib-core/build.gradle b/jib-core/build.gradle
index 624a319697..69b4ccccda 100644
--- a/jib-core/build.gradle
+++ b/jib-core/build.gradle
@@ -5,6 +5,7 @@ plugins {
 
 dependencies {
   implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}"
+  implementation "com.google.http-client:google-http-client-apache-v2:${dependencyVersions.GOOGLE_HTTP_CLIENT_APACHE_V2}"
   implementation("com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}") {
     exclude group: "com.google.http-client", module: "google-http-client"
   }
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java b/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
index 46bf2615ee..78e073c5ca 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/http/FailoverHttpClient.java
@@ -22,10 +22,10 @@
 import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpTransport;
-import com.google.api.client.http.apache.ApacheHttpTransport;
+import com.google.api.client.http.apache.v2.ApacheHttpTransport;
+import com.google.api.client.util.SslUtils;
 import com.google.cloud.tools.jib.api.LogEvent;
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
 import java.io.IOException;
 import java.net.ConnectException;
 import java.net.URL;
@@ -33,9 +33,8 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 import javax.net.ssl.SSLException;
-import org.apache.http.auth.AuthScope;
-import org.apache.http.auth.UsernamePasswordCredentials;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.impl.client.HttpClientBuilder;
 
 /**
  * Thread-safe HTTP client that can automatically failover from secure HTTPS to insecure HTTPS or
@@ -86,55 +85,23 @@ private static HttpTransport getSecureHttpTransport() {
     //
     // A new ApacheHttpTransport needs to be created for each connection because otherwise HTTP
     // connection persistence causes the connection to throw NoHttpResponseException.
-    ApacheHttpTransport transport = new ApacheHttpTransport();
-    addProxyCredentials(transport);
-    return transport;
+    return new ApacheHttpTransport();
   }
 
   private static HttpTransport getInsecureHttpTransport() {
     try {
-      ApacheHttpTransport insecureTransport =
-          new ApacheHttpTransport.Builder().doNotValidateCertificate().build();
-      addProxyCredentials(insecureTransport);
-      return insecureTransport;
+      HttpClientBuilder httpClientBuilder =
+          ApacheHttpTransport.newDefaultHttpClientBuilder()
+              .setSSLSocketFactory(null) // creates new factory with the SSLContext given below
+              .setSSLContext(SslUtils.trustAllSSLContext())
+              .setSSLHostnameVerifier(new NoopHostnameVerifier());
+      // Do not use NetHttpTransport. See comments in getConnectionFactory for details.
+      return new ApacheHttpTransport(httpClientBuilder.build());
     } catch (GeneralSecurityException ex) {
       throw new RuntimeException("platform does not support TLS protocol", ex);
     }
   }
 
-  /**
-   * Registers proxy credentials onto transport client, in order to deal with proxies that require
-   * basic authentication.
-   *
-   * @param transport Apache HTTP transport
-   */
-  @VisibleForTesting
-  static void addProxyCredentials(ApacheHttpTransport transport) {
-    addProxyCredentials(transport, "https");
-    addProxyCredentials(transport, "http");
-  }
-
-  private static void addProxyCredentials(ApacheHttpTransport transport, String protocol) {
-    Preconditions.checkArgument(protocol.equals("http") || protocol.equals("https"));
-
-    String proxyHost = System.getProperty(protocol + ".proxyHost");
-    String proxyUser = System.getProperty(protocol + ".proxyUser");
-    String proxyPassword = System.getProperty(protocol + ".proxyPassword");
-    if (proxyHost == null || proxyUser == null || proxyPassword == null) {
-      return;
-    }
-
-    String defaultProxyPort = protocol.equals("http") ? "80" : "443";
-    int proxyPort = Integer.parseInt(System.getProperty(protocol + ".proxyPort", defaultProxyPort));
-
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    httpClient
-        .getCredentialsProvider()
-        .setCredentials(
-            new AuthScope(proxyHost, proxyPort),
-            new UsernamePasswordCredentials(proxyUser, proxyPassword));
-  }
-
   private final boolean enableHttpAndInsecureFailover;
   private final boolean sendAuthorizationOverHttp;
   private final Consumer<LogEvent> logger;
diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
index eb5d0fd98a..debfbd4737 100644
--- a/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
+++ b/jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java
@@ -164,10 +164,11 @@ private T call(URL url) throws IOException, RegistryException {
             throw new RegistryUnauthorizedException(serverUrl, imageName, responseException);
           }
 
-        } else if (responseException.getStatusCode()
-                == HttpStatusCodes.STATUS_CODE_TEMPORARY_REDIRECT
-            || responseException.getStatusCode() == HttpStatusCodes.STATUS_CODE_MOVED_PERMANENTLY
-            || responseException.getStatusCode() == STATUS_CODE_PERMANENT_REDIRECT) {
+          // 301 (Moved Permanently), 302 (Found), 303 (See Other), and 307 (Temporary Redirect) are
+          // automatically followed by Google HTTP Client (setFollowRedirects(true)), but 308 isn't.
+          // https://github.com/googleapis/google-http-java-client/issues/873
+          // TODO: remove this when the bug is fixed.
+        } else if (responseException.getStatusCode() == STATUS_CODE_PERMANENT_REDIRECT) {
           // 'Location' header can be relative or absolute.
           URL redirectLocation = new URL(url, responseException.getHeaders().getLocation());
           return call(redirectLocation);
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java
deleted file mode 100644
index 615f3f0cfb..0000000000
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/FailoverHttpClientProxyCredentialsTest.java
+++ /dev/null
@@ -1,150 +0,0 @@
-/*
- * Copyright 2018 Google LLC.
- *
- * 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.cloud.tools.jib.http;
-
-import com.google.api.client.http.apache.ApacheHttpTransport;
-import com.google.common.collect.ImmutableList;
-import org.apache.http.auth.AuthScope;
-import org.apache.http.auth.Credentials;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.contrib.java.lang.system.RestoreSystemProperties;
-
-/** Tests for {@link FailoverHttpClient} with setting proxy credentials. */
-public class FailoverHttpClientProxyCredentialsTest {
-
-  private static final ImmutableList<String> proxyProperties =
-      ImmutableList.of(
-          "http.proxyHost",
-          "http.proxyPort",
-          "http.proxyUser",
-          "http.proxyPassword",
-          "https.proxyHost",
-          "https.proxyPort",
-          "https.proxyUser",
-          "https.proxyPassword");
-
-  @Rule public final RestoreSystemProperties systemPropertyRestorer = new RestoreSystemProperties();
-
-  private final ApacheHttpTransport transport = new ApacheHttpTransport();
-
-  @Before
-  public void setUp() {
-    proxyProperties.stream().forEach(key -> System.clearProperty(key));
-  }
-
-  @Test
-  public void testAddProxyCredentials_undefined() {
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
-    Assert.assertNull(credentials);
-  }
-
-  @Test
-  public void testAddProxyCredentials() {
-    System.setProperty("http.proxyHost", "http://localhost");
-    System.setProperty("http.proxyPort", "1080");
-    System.setProperty("http.proxyUser", "user");
-    System.setProperty("http.proxyPassword", "pass");
-
-    System.setProperty("https.proxyHost", "https://host.com");
-    System.setProperty("https.proxyPort", "1443");
-    System.setProperty("https.proxyUser", "s-user");
-    System.setProperty("https.proxyPassword", "s-pass");
-
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials httpCredentials =
-        httpClient.getCredentialsProvider().getCredentials(new AuthScope("http://localhost", 1080));
-    Assert.assertEquals("user", httpCredentials.getUserPrincipal().getName());
-    Assert.assertEquals("pass", httpCredentials.getPassword());
-
-    Credentials httpsCredentials =
-        httpClient.getCredentialsProvider().getCredentials(new AuthScope("https://host.com", 1443));
-    Assert.assertEquals("s-user", httpsCredentials.getUserPrincipal().getName());
-    Assert.assertEquals("s-pass", httpsCredentials.getPassword());
-  }
-
-  @Test
-  public void testAddProxyCredentials_defaultPorts() {
-    System.setProperty("http.proxyHost", "http://localhost");
-    System.setProperty("http.proxyUser", "user");
-    System.setProperty("http.proxyPassword", "pass");
-
-    System.setProperty("https.proxyHost", "https://host.com");
-    System.setProperty("https.proxyUser", "s-user");
-    System.setProperty("https.proxyPassword", "s-pass");
-
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials httpCredentials =
-        httpClient.getCredentialsProvider().getCredentials(new AuthScope("http://localhost", 80));
-    Assert.assertEquals("user", httpCredentials.getUserPrincipal().getName());
-    Assert.assertEquals("pass", httpCredentials.getPassword());
-
-    Credentials httpsCredentials =
-        httpClient.getCredentialsProvider().getCredentials(new AuthScope("https://host.com", 443));
-    Assert.assertEquals("s-user", httpsCredentials.getUserPrincipal().getName());
-    Assert.assertEquals("s-pass", httpsCredentials.getPassword());
-  }
-
-  @Test
-  public void testAddProxyCredentials_hostUndefined() {
-    System.setProperty("http.proxyUser", "user");
-    System.setProperty("http.proxyPassword", "pass");
-
-    System.setProperty("https.proxyUser", "s-user");
-    System.setProperty("https.proxyPassword", "s-pass");
-
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
-    Assert.assertNull(credentials);
-  }
-
-  @Test
-  public void testAddProxyCredentials_userUndefined() {
-    System.setProperty("http.proxyHost", "http://localhost");
-    System.setProperty("http.proxyPassword", "pass");
-
-    System.setProperty("https.proxyHost", "https://host.com");
-    System.setProperty("https.proxyPassword", "s-pass");
-
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
-    Assert.assertNull(credentials);
-  }
-
-  @Test
-  public void testAddProxyCredentials_passwordUndefined() {
-    System.setProperty("http.proxyHost", "http://localhost");
-    System.setProperty("http.proxyUser", "user");
-
-    System.setProperty("https.proxyHost", "https://host.com");
-    System.setProperty("https.proxyUser", "s-user");
-
-    FailoverHttpClient.addProxyCredentials(transport);
-    DefaultHttpClient httpClient = (DefaultHttpClient) transport.getHttpClient();
-    Credentials credentials = httpClient.getCredentialsProvider().getCredentials(AuthScope.ANY);
-    Assert.assertNull(credentials);
-  }
-}
diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
index 471b74002a..d33506f543 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
@@ -24,6 +24,7 @@
 import java.nio.charset.StandardCharsets;
 import java.security.GeneralSecurityException;
 import java.util.Arrays;
+import java.util.List;
 import java.util.function.Consumer;
 import javax.net.ssl.SSLException;
 import org.hamcrest.CoreMatchers;
@@ -149,4 +150,43 @@ public void testProxyCredentialProperties()
       }
     }
   }
+
+  @Test
+  public void testVerbatimRedirectionUrls()
+      throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
+    String url1 = "?id=301&_auth_=exp=1572285389~hmac=f0a387f0";
+    String url2 = "?id=302&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614";
+    String url3 = "?id=303&_auth_=exp=1572285389~hmac=f0a387f0";
+    String url4 = "?id=307&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614";
+
+    String redirect301 =
+        "HTTP/1.1 301 Moved Permanently\nLocation: " + url1 + "\nContent-Length: 0\n\n";
+    String redirect302 = "HTTP/1.1 302 Found\nLocation: " + url2 + "\nContent-Length: 0\n\n";
+    String redirect303 = "HTTP/1.1 303 See Other\nLocation: " + url3 + "\nContent-Length: 0\n\n";
+    String redirect307 =
+        "HTTP/1.1 307 Temporary Redirect\nLocation: " + url4 + "\nContent-Length: 0\n\n";
+    String ok200 = "HTTP/1.1 200 OK\nContent-Length:12\n\nHello World!";
+    List<String> responses =
+        Arrays.asList(redirect301, redirect302, redirect303, redirect307, ok200);
+
+    FailoverHttpClient httpClient = new FailoverHttpClient(true /*insecure*/, false, logger);
+    try (TestWebServer server = new TestWebServer(false, responses, 1)) {
+      httpClient.get(new URL(server.getEndpoint()), request);
+
+      Assert.assertThat(
+          server.getInputRead(),
+          CoreMatchers.containsString("GET /?id=301&_auth_=exp%3D1572285389~hmac%3Df0a387f0 "));
+      Assert.assertThat(
+          server.getInputRead(),
+          CoreMatchers.containsString(
+              "GET /?id=302&Signature=2wYOD0a%2BDAkK/9lQJUOuIpYti8o%3D&Expires=1569997614 "));
+      Assert.assertThat(
+          server.getInputRead(),
+          CoreMatchers.containsString("GET /?id=303&_auth_=exp%3D1572285389~hmac%3Df0a387f0 "));
+      Assert.assertThat(
+          server.getInputRead(),
+          CoreMatchers.containsString(
+              "GET /?id=307&Signature=2wYOD0a%2BDAkK/9lQJUOuIpYti8o%3D&Expires=1569997614 "));
+    }
+  }
 }

From e8796ec886b56e31e2f7cf907a5cdf4d1e4d15de Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Fri, 1 Nov 2019 18:27:44 -0400
Subject: [PATCH 4/6] Remove unnecessary code

---
 .../registry/RegistryEndpointCallerTest.java  | 46 ++++++-------------
 1 file changed, 14 insertions(+), 32 deletions(-)

diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
index 2179cd5b2b..3487df0b58 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java
@@ -242,18 +242,22 @@ public void testCall_unknown() throws IOException, RegistryException {
   }
 
   @Test
-  public void testCall_temporaryRedirect() throws IOException, RegistryException {
-    verifyRetriesWithNewLocation(HttpStatusCodes.STATUS_CODE_TEMPORARY_REDIRECT);
-  }
+  public void testCall_permanentRedirect() throws IOException, RegistryException {
+    ResponseException redirectException =
+        mockResponseException(
+            RegistryEndpointCaller.STATUS_CODE_PERMANENT_REDIRECT,
+            new HttpHeaders().setLocation("https://newlocation"));
 
-  @Test
-  public void testCall_movedPermanently() throws IOException, RegistryException {
-    verifyRetriesWithNewLocation(HttpStatusCodes.STATUS_CODE_MOVED_PERMANENTLY);
-  }
+    // Make httpClient.call() throw first, then succeed.
+    setUpRegistryResponse(redirectException);
+    Mockito.when(
+            mockHttpClient.call(
+                Mockito.eq("httpMethod"),
+                Mockito.eq(new URL("https://newlocation")),
+                Mockito.any()))
+        .thenReturn(mockResponse);
 
-  @Test
-  public void testCall_permanentRedirect() throws IOException, RegistryException {
-    verifyRetriesWithNewLocation(RegistryEndpointCaller.STATUS_CODE_PERMANENT_REDIRECT);
+    Assert.assertEquals("body", endpointCaller.call());
   }
 
   @Test
@@ -471,28 +475,6 @@ private void verifyThrowsRegistryErrorException(int httpStatusCode)
     }
   }
 
-  /**
-   * Verifies that a response with {@code httpStatusCode} retries the request with the {@code
-   * Location} header.
-   */
-  private void verifyRetriesWithNewLocation(int httpStatusCode)
-      throws IOException, RegistryException {
-    // Mocks a response for temporary redirect to a new location.
-    ResponseException redirectException =
-        mockResponseException(httpStatusCode, new HttpHeaders().setLocation("https://newlocation"));
-
-    // Make httpClient.call() throw first, then succeed.
-    setUpRegistryResponse(redirectException);
-    Mockito.when(
-            mockHttpClient.call(
-                Mockito.eq("httpMethod"),
-                Mockito.eq(new URL("https://newlocation")),
-                Mockito.any()))
-        .thenReturn(mockResponse);
-
-    Assert.assertEquals("body", endpointCaller.call());
-  }
-
   private void setUpRegistryResponse(Exception exceptionToThrow)
       throws MalformedURLException, IOException {
     Mockito.when(

From 5fd49d760e9ab50f0917155f64722ed038948dea Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Mon, 4 Nov 2019 14:01:40 -0500
Subject: [PATCH 5/6] Remove "excludes" from "google-auth-library-oauth2-http"

---
 build.gradle          | 5 ++++-
 jib-core/build.gradle | 4 +---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/build.gradle b/build.gradle
index 0acb92475d..5340839127 100644
--- a/build.gradle
+++ b/build.gradle
@@ -65,7 +65,10 @@ subprojects {
   // Use this to ensure we correctly override transitive dependencies
   // TODO: There might be a plugin that does this
   task ensureTransitiveDependencyOverrides {
-    def rules = ["google-http-client": dependencyVersions.GOOGLE_HTTP_CLIENT]
+    def rules = [
+        "google-http-client": dependencyVersions.GOOGLE_HTTP_CLIENT,
+        "google-http-client-apache-v2": dependencyVersions.GOOGLE_HTTP_CLIENT_APACHE_V2,
+    ]
     doLast {
       configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.each { artifact ->
         def dependency = artifact.moduleVersion.id
diff --git a/jib-core/build.gradle b/jib-core/build.gradle
index 69b4ccccda..672ab9f3ba 100644
--- a/jib-core/build.gradle
+++ b/jib-core/build.gradle
@@ -6,9 +6,7 @@ plugins {
 dependencies {
   implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}"
   implementation "com.google.http-client:google-http-client-apache-v2:${dependencyVersions.GOOGLE_HTTP_CLIENT_APACHE_V2}"
-  implementation("com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}") {
-    exclude group: "com.google.http-client", module: "google-http-client"
-  }
+  implementation "com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}"
 
   implementation "org.apache.commons:commons-compress:${dependencyVersions.COMMONS_COMPRESS}"
   implementation "com.google.guava:guava:${dependencyVersions.GUAVA}"

From 7c92d5dc82913a62f2da25a2e055d5fa2f391675 Mon Sep 17 00:00:00 2001
From: Chanseok Oh <chanseok@google.com>
Date: Mon, 4 Nov 2019 14:22:21 -0500
Subject: [PATCH 6/6] Add reference for sample query string values

---
 .../tools/jib/http/WithServerFailoverHttpClientTest.java      | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
index d33506f543..72e8baa979 100644
--- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
+++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/WithServerFailoverHttpClientTest.java
@@ -152,8 +152,10 @@ public void testProxyCredentialProperties()
   }
 
   @Test
-  public void testVerbatimRedirectionUrls()
+  public void testRedirectionUrls()
       throws IOException, InterruptedException, GeneralSecurityException, URISyntaxException {
+    // Sample query strings from
+    // https://github.com/GoogleContainerTools/jib/issues/1986#issuecomment-547610104
     String url1 = "?id=301&_auth_=exp=1572285389~hmac=f0a387f0";
     String url2 = "?id=302&Signature=2wYOD0a%2BDAkK%2F9lQJUOuIpYti8o%3D&Expires=1569997614";
     String url3 = "?id=303&_auth_=exp=1572285389~hmac=f0a387f0";