Fall back to HTTP on right condition when HttpHostConnectException

jib-core/src/main/java/com/google/cloud/tools/jib/registry/RegistryEndpointCaller.java

@@ -151,6 +151,14 @@ private T callWithAllowInsecureRegistryHandling(URL url) throws IOException, Reg
 
     } catch (SSLPeerUnverifiedException ex) {
       return handleUnverifiableServerException(url);
+
+    } catch (HttpHostConnectException ex) {
+      if (allowInsecureRegistries && isHttpsProtocol(url) && url.getPort() == -1) {
+        // Fall back to HTTP only if "url" had no port specified (i.e., we tried the default HTTPS
+        // port 443) and we could not connect to 443. It's worth trying port 80.
+        return fallBackToHttp(url);
+      }
+      throw ex;
     }
   }
 
@@ -161,18 +169,24 @@ private T handleUnverifiableServerException(URL url) throws IOException, Registr
     }
 
     try {
+      logger.warn(
+          "Cannot verify server at " + url + ". Attempting again with no TLS verification.");
       return call(url, getInsecureConnectionFactory());
 
-    } catch (SSLPeerUnverifiedException | HttpHostConnectException ex) {
-      // Try HTTP as a last resort.
-      GenericUrl httpUrl = new GenericUrl(url);
-      httpUrl.setScheme("http");
-      logger.warn(
-          "Failed to connect to " + url + " over HTTPS. Attempting again with HTTP: " + httpUrl);
-      return call(httpUrl.toURL(), connectionFactory);
+    } catch (SSLPeerUnverifiedException ex) {
+      return fallBackToHttp(url);
     }
   }
 
+  @Nullable
+  private T fallBackToHttp(URL url) throws IOException, RegistryException {
+    GenericUrl httpUrl = new GenericUrl(url);
+    httpUrl.setScheme("http");
+    logger.warn(
+        "Failed to connect to " + url + " over HTTPS. Attempting again with HTTP: " + httpUrl);
+    return call(httpUrl.toURL(), connectionFactory);
+  }
+
   private Function<URL, Connection> getInsecureConnectionFactory() throws RegistryException {
     try {
       if (insecureConnectionFactory == null) {

jib-core/src/test/java/com/google/cloud/tools/jib/registry/RegistryEndpointCallerTest.java

@@ -113,7 +113,7 @@ private static HttpResponse mockRedirectHttpResponse(String redirectLocation) th
 
   @Before
   public void setUp() throws IOException {
-    secureEndpointCaller = createRegistryEndpointCaller(false);
+    secureEndpointCaller = createRegistryEndpointCaller(false, -1);
 
     Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
     Mockito.when(mockInsecureConnectionFactory.apply(Mockito.any()))
@@ -149,15 +149,22 @@ public void testCall_insecureCallerOnUnverifiableServer() throws IOException, Re
     Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
         .thenReturn(mockResponse); // OK with non-verifying connection
 
-    RegistryEndpointCaller<String> insecureCaller = createRegistryEndpointCaller(true);
+    RegistryEndpointCaller<String> insecureCaller = createRegistryEndpointCaller(true, -1);
     Assert.assertEquals("body", insecureCaller.call());
 
     ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
-    Mockito.verify(mockConnectionFactory, Mockito.times(1)).apply(urlCaptor.capture());
+    Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture());
     Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(0));
 
-    Mockito.verify(mockInsecureConnectionFactory, Mockito.times(1)).apply(urlCaptor.capture());
+    Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture());
     Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(1));
+
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
+
+    Mockito.verify(mockBuildLogger)
+        .warn(
+            "Cannot verify server at https://apiRouteBase/api. Attempting again with no TLS verification.");
   }
 
   @Test
@@ -168,51 +175,93 @@ public void testCall_insecureCallerOnHttpServer() throws IOException, RegistryEx
     Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
         .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS
 
-    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true);
+    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true, -1);
     Assert.assertEquals("body", insecureEndpointCaller.call());
 
     ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
     Mockito.verify(mockConnectionFactory, Mockito.times(2)).apply(urlCaptor.capture());
     Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(0));
     Assert.assertEquals(new URL("http://apiRouteBase/api"), urlCaptor.getAllValues().get(1));
 
-    Mockito.verify(mockInsecureConnectionFactory, Mockito.times(1)).apply(urlCaptor.capture());
+    Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture());
     Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(2));
 
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
+
+    Mockito.verify(mockBuildLogger)
+        .warn(
+            "Cannot verify server at https://apiRouteBase/api. Attempting again with no TLS verification.");
     Mockito.verify(mockBuildLogger)
         .warn(
-            "Failed to connect to "
-                + urlCaptor.getAllValues().get(0)
-                + " over HTTPS. Attempting again with HTTP: "
-                + urlCaptor.getAllValues().get(1));
+            "Failed to connect to https://apiRouteBase/api over HTTPS. Attempting again with HTTP: http://apiRouteBase/api");
   }
 
   @Test
-  public void testCall_insecureCallerOnHttpServerByHttpHostConnectException()
+  public void testCall_insecureCallerOnHttpServerAndNoPortSpecified()
       throws IOException, RegistryException {
     Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
-        .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)) // server is not HTTPS
-        .thenReturn(mockResponse);
-    Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
-        .thenThrow(Mockito.mock(HttpHostConnectException.class)); // server is not HTTPS
+        .thenThrow(Mockito.mock(HttpHostConnectException.class)) // server is not listening on 443
+        .thenReturn(mockResponse); // respond when connected through 80
 
-    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true);
+    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true, -1);
     Assert.assertEquals("body", insecureEndpointCaller.call());
 
     ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
     Mockito.verify(mockConnectionFactory, Mockito.times(2)).apply(urlCaptor.capture());
     Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(0));
     Assert.assertEquals(new URL("http://apiRouteBase/api"), urlCaptor.getAllValues().get(1));
 
-    Mockito.verify(mockInsecureConnectionFactory, Mockito.times(1)).apply(urlCaptor.capture());
-    Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(2));
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
 
     Mockito.verify(mockBuildLogger)
         .warn(
-            "Failed to connect to "
-                + urlCaptor.getAllValues().get(0)
-                + " over HTTPS. Attempting again with HTTP: "
-                + urlCaptor.getAllValues().get(1));
+            "Failed to connect to https://apiRouteBase/api over HTTPS. Attempting again with HTTP: http://apiRouteBase/api");
+  }
+
+  @Test
+  public void testCall_secureCallerOnNonListeningServerAndNoPortSpecified()
+      throws IOException, RegistryException {
+    Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
+        .thenThrow(Mockito.mock(HttpHostConnectException.class)); // server is not listening on 443
+
+    try {
+      secureEndpointCaller.call();
+      Assert.fail("Should not fall back to HTTP if not allowInsecureRegistries");
+    } catch (HttpHostConnectException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
+    Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture());
+    Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(0));
+
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
+  }
+
+  @Test
+  public void testCall_insecureCallerOnNonListeningServerAndPortSpecified()
+      throws IOException, RegistryException {
+    Mockito.when(mockConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
+        .thenThrow(Mockito.mock(HttpHostConnectException.class)); // server is not listening on 5000
+
+    RegistryEndpointCaller<String> insecureEndpointCaller =
+        createRegistryEndpointCaller(true, 5000);
+    try {
+      insecureEndpointCaller.call();
+      Assert.fail("Should not fall back to HTTP if port was explicitly given and cannot connect");
+    } catch (HttpHostConnectException ex) {
+      Assert.assertNull(ex.getMessage());
+    }
+
+    ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
+    Mockito.verify(mockConnectionFactory).apply(urlCaptor.capture());
+    Assert.assertEquals(new URL("https://apiRouteBase:5000/api"), urlCaptor.getAllValues().get(0));
+
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
   }
 
   @Test
@@ -249,7 +298,7 @@ public void testCall_credentialsNotSentOverHttp() throws IOException, RegistryEx
     Mockito.when(mockInsecureConnection.send(Mockito.eq("httpMethod"), Mockito.any()))
         .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS
 
-    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true);
+    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true, -1);
     try {
       insecureEndpointCaller.call();
       Assert.fail("Call should have failed");
@@ -273,8 +322,27 @@ public void testCall_credentialsForcedOverHttp() throws IOException, RegistryExc
         .thenThrow(Mockito.mock(SSLPeerUnverifiedException.class)); // server is not HTTPS
 
     System.setProperty("sendCredentialsOverHttp", "true");
-    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true);
+    RegistryEndpointCaller<String> insecureEndpointCaller = createRegistryEndpointCaller(true, -1);
     Assert.assertEquals("body", insecureEndpointCaller.call());
+
+    ArgumentCaptor<URL> urlCaptor = ArgumentCaptor.forClass(URL.class);
+    Mockito.verify(mockConnectionFactory, Mockito.times(3)).apply(urlCaptor.capture());
+    Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(0));
+    Assert.assertEquals(new URL("http://apiRouteBase/api"), urlCaptor.getAllValues().get(1));
+    Assert.assertEquals(new URL("http://newlocation"), urlCaptor.getAllValues().get(2));
+
+    Mockito.verify(mockInsecureConnectionFactory).apply(urlCaptor.capture());
+    Assert.assertEquals(new URL("https://apiRouteBase/api"), urlCaptor.getAllValues().get(3));
+
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
+
+    Mockito.verify(mockBuildLogger)
+        .warn(
+            "Cannot verify server at https://apiRouteBase/api. Attempting again with no TLS verification.");
+    Mockito.verify(mockBuildLogger)
+        .warn(
+            "Failed to connect to https://apiRouteBase/api over HTTPS. Attempting again with HTTP: http://apiRouteBase/api");
   }
 
   @Test
@@ -482,14 +550,17 @@ private void verifyRetriesWithNewLocation(int httpStatusCode)
     Assert.assertEquals(
         new URL("https://apiRouteBase/api"), urlArgumentCaptor.getAllValues().get(0));
     Assert.assertEquals(new URL("https://newlocation"), urlArgumentCaptor.getAllValues().get(1));
+
+    Mockito.verifyNoMoreInteractions(mockConnectionFactory);
+    Mockito.verifyNoMoreInteractions(mockInsecureConnectionFactory);
   }
 
-  private RegistryEndpointCaller<String> createRegistryEndpointCaller(boolean allowInsecure)
-      throws MalformedURLException {
+  private RegistryEndpointCaller<String> createRegistryEndpointCaller(
+      boolean allowInsecure, int port) throws MalformedURLException {
     return new RegistryEndpointCaller<>(
         mockBuildLogger,
         "userAgent",
-        "apiRouteBase",
+        (port == -1) ? "apiRouteBase" : ("apiRouteBase:" + port),
         new TestRegistryEndpointProvider(),
         Authorizations.withBasicToken("token"),
         new RegistryEndpointRequestProperties("serverUrl", "imageName"),