Enable jib.httpTimeout system property for HTTP connection and read timeout

jib-core/src/main/java/com/google/cloud/tools/jib/http/Connection.java

@@ -18,6 +18,7 @@
 
 import com.google.api.client.http.GenericUrl;
 import com.google.api.client.http.HttpMethods;
+import com.google.api.client.http.HttpRequest;
 import com.google.api.client.http.HttpRequestFactory;
 import com.google.api.client.http.HttpResponse;
 import com.google.api.client.http.apache.ApacheHttpTransport;
@@ -117,11 +118,16 @@ public Response put(Request request) throws IOException {
    * @throws IOException if building the HTTP request fails.
    */
   public Response send(String httpMethod, Request request) throws IOException {
-    httpResponse =
+    HttpRequest httpRequest =
         requestFactory
             .buildRequest(httpMethod, url, request.getHttpContent())
-            .setHeaders(request.getHeaders())
-            .execute();
+            .setHeaders(request.getHeaders());
+    if (request.getHttpTimeout() != null) {
+      httpRequest.setConnectTimeout(request.getHttpTimeout());
+      httpRequest.setReadTimeout(request.getHttpTimeout());
+    }
+
+    httpResponse = httpRequest.execute();
     return new Response(httpResponse);
   }
 }

jib-core/src/main/java/com/google/cloud/tools/jib/http/Request.java

@@ -27,12 +27,16 @@ public class Request {
   private final HttpHeaders headers;
 
   /** The HTTP request body. */
-  @Nullable private BlobHttpContent body;
+  @Nullable private final BlobHttpContent body;
+
+  /** HTTP connection and read timeout. */
+  @Nullable private final Integer httpTimeout;
 
   public static class Builder {
 
     private final HttpHeaders headers = new HttpHeaders().setAccept("*/*");
     @Nullable private BlobHttpContent body;
+    @Nullable private Integer httpTimeout;
 
     public Request build() {
       return new Request(this);
@@ -73,6 +77,18 @@ public Builder setUserAgent(String userAgent) {
       return this;
     }
 
+    /**
+     * Sets the HTTP connection and read timeout in milliseconds. {@code null} uses the default
+     * timeout and {@code 0} an infinite timeout.
+     *
+     * @param httpTimeout timeout in milliseconds
+     * @return this
+     */
+    public Builder setHttpTimeout(@Nullable Integer httpTimeout) {
+      this.httpTimeout = httpTimeout;
+      return this;
+    }
+
     /**
      * Sets the body and its corresponding {@code Content-Type} header.
      *
@@ -92,6 +108,7 @@ public static Builder builder() {
   private Request(Builder builder) {
     this.headers = builder.headers;
     this.body = builder.body;
+    this.httpTimeout = builder.httpTimeout;
   }
 
   HttpHeaders getHeaders() {
@@ -102,4 +119,9 @@ HttpHeaders getHeaders() {
   BlobHttpContent getHttpContent() {
     return body;
   }
+
+  @Nullable
+  Integer getHttpTimeout() {
+    return httpTimeout;
+  }
 }

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

@@ -239,7 +239,8 @@ private Authorization authenticate(String scope) throws RegistryAuthenticationFa
       URL authenticationUrl = getAuthenticationUrl(scope);
 
       try (Connection connection = new Connection(authenticationUrl)) {
-        Request.Builder requestBuilder = Request.builder();
+        Request.Builder requestBuilder =
+            Request.builder().setHttpTimeout(Integer.getInteger("jib.httpTimeout"));
         if (authorization != null) {
           requestBuilder.setAuthorization(authorization);
         }

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

@@ -162,6 +162,7 @@ T call(RequestState requestState) throws IOException, RegistryException {
       Request.Builder requestBuilder =
           Request.builder()
               .setUserAgent(userAgent)
+              .setHttpTimeout(Integer.getInteger("jib.httpTimeout"))
               .setAccept(registryEndpointProvider.getAccept())
               .setBody(registryEndpointProvider.getContent());
       // Only sends authorization if using HTTPS.

jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionTest.java

@@ -28,7 +28,6 @@
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import org.junit.Assert;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
@@ -55,47 +54,76 @@ public class ConnectionTest {
 
   @InjectMocks private final Connection testConnection = new Connection(fakeUrl.toURL());
 
-  @Before
-  public void setUpMocksAndFakes() throws IOException {
-    fakeRequest =
-        Request.builder()
-            .setAccept(Arrays.asList("fake.accept", "another.fake.accept"))
-            .setUserAgent("fake user agent")
-            .setBody(new BlobHttpContent(Blobs.from("crepecake"), "fake.content.type"))
-            .setAuthorization(Authorizations.withBasicCredentials("fake-username", "fake-secret"))
-            .build();
-
-    Mockito.when(
-            mockHttpRequestFactory.buildRequest(
-                Mockito.any(String.class), Mockito.eq(fakeUrl), Mockito.any(BlobHttpContent.class)))
-        .thenReturn(mockHttpRequest);
-
-    Mockito.when(mockHttpRequest.setHeaders(Mockito.any(HttpHeaders.class)))
-        .thenReturn(mockHttpRequest);
-    Mockito.when(mockHttpRequest.execute()).thenReturn(mockHttpResponse);
-  }
-
   @Test
   public void testGet() throws IOException {
+    setUpMocksAndFakes(null);
     testSend(HttpMethods.GET, Connection::get);
   }
 
   @Test
   public void testPost() throws IOException {
+    setUpMocksAndFakes(null);
     testSend(HttpMethods.POST, Connection::post);
   }
 
   @Test
   public void testPut() throws IOException {
+    setUpMocksAndFakes(null);
     testSend(HttpMethods.PUT, Connection::put);
   }
 
+  @Test
+  public void testHttpTimeout_doNotSetByDefault() throws IOException {
+    setUpMocksAndFakes(null);
+    try (Connection connection = testConnection) {
+      connection.send(HttpMethods.GET, fakeRequest);
+    }
+
+    Mockito.verify(mockHttpRequest, Mockito.never()).setConnectTimeout(Mockito.anyInt());
+    Mockito.verify(mockHttpRequest, Mockito.never()).setReadTimeout(Mockito.anyInt());
+  }
+
+  @Test
+  public void testHttpTimeout() throws IOException {
+    setUpMocksAndFakes(5982);
+    try (Connection connection = testConnection) {
+      connection.send(HttpMethods.GET, fakeRequest);
+    }
+
+    Mockito.verify(mockHttpRequest).setConnectTimeout(5982);
+    Mockito.verify(mockHttpRequest).setReadTimeout(5982);
+  }
+
   @FunctionalInterface
   private interface SendFunction {
 
     Response send(Connection connection, Request request) throws IOException;
   }
 
+  private void setUpMocksAndFakes(Integer httpTimeout) throws IOException {
+    fakeRequest =
+        Request.builder()
+            .setAccept(Arrays.asList("fake.accept", "another.fake.accept"))
+            .setUserAgent("fake user agent")
+            .setBody(new BlobHttpContent(Blobs.from("crepecake"), "fake.content.type"))
+            .setAuthorization(Authorizations.withBasicCredentials("fake-username", "fake-secret"))
+            .setHttpTimeout(httpTimeout)
+            .build();
+
+    Mockito.when(
+            mockHttpRequestFactory.buildRequest(
+                Mockito.any(String.class), Mockito.eq(fakeUrl), Mockito.any(BlobHttpContent.class)))
+        .thenReturn(mockHttpRequest);
+
+    Mockito.when(mockHttpRequest.setHeaders(Mockito.any(HttpHeaders.class)))
+        .thenReturn(mockHttpRequest);
+    if (httpTimeout != null) {
+      Mockito.when(mockHttpRequest.setConnectTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
+      Mockito.when(mockHttpRequest.setReadTimeout(Mockito.anyInt())).thenReturn(mockHttpRequest);
+    }
+    Mockito.when(mockHttpRequest.execute()).thenReturn(mockHttpResponse);
+  }
+
   private void testSend(String httpMethod, SendFunction sendFunction) throws IOException {
     try (Connection connection = testConnection) {
       sendFunction.send(connection, fakeRequest);

jib-core/src/test/java/com/google/cloud/tools/jib/http/MockConnection.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2018 Google LLC. 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.cloud.tools.jib.http;
+
+import com.google.api.client.http.GenericUrl;
+import java.io.IOException;
+import java.util.function.BiFunction;
+
+/**
+ * Mock {@link Connection} used for testing. Normally, you would use {@link
+ * org.mockito.Mockito#mock}; this class is intended to examine the {@link Request) object by
+ * calling its non-public package-protected methods.
+ */
+public class MockConnection extends Connection {
+
+  private BiFunction<String, Request, Response> responseSupplier;
+  private Integer httpTimeout;
+
+  public MockConnection(BiFunction<String, Request, Response> responseSupplier) {
+    super(new GenericUrl("ftp://non-exisiting.example.url.ever").toURL());
+    this.responseSupplier = responseSupplier;
+  }
+
+  @Override
+  public Response send(String httpMethod, Request request) throws IOException {
+    httpTimeout = request.getHttpTimeout();
+    return responseSupplier.apply(httpMethod, request);
+  }
+
+  public Integer getRequestedHttpTimeout() {
+    return httpTimeout;
+  }
+}

jib-core/src/test/java/com/google/cloud/tools/jib/http/RequestTest.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2018 Google LLC. 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.cloud.tools.jib.http;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/** Tests for {@link Request}. */
+public class RequestTest {
+
+  @Test
+  public void testGetHttpTimeout() {
+    Request request = Request.builder().build();
+
+    Assert.assertNull(request.getHttpTimeout());
+  }
+
+  @Test
+  public void testSetHttpTimeout() {
+    Request request = Request.builder().setHttpTimeout(3000).build();
+
+    Assert.assertEquals(Integer.valueOf(3000), request.getHttpTimeout());
+  }
+}

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

@@ -20,10 +20,12 @@
 import com.google.api.client.http.HttpResponse;
 import com.google.api.client.http.HttpResponseException;
 import com.google.api.client.http.HttpStatusCodes;
+import com.google.cloud.tools.jib.blob.Blob;
 import com.google.cloud.tools.jib.blob.Blobs;
 import com.google.cloud.tools.jib.http.Authorizations;
 import com.google.cloud.tools.jib.http.BlobHttpContent;
 import com.google.cloud.tools.jib.http.Connection;
+import com.google.cloud.tools.jib.http.MockConnection;
 import com.google.cloud.tools.jib.http.Response;
 import com.google.cloud.tools.jib.json.JsonTemplateMapper;
 import com.google.cloud.tools.jib.registry.RegistryEndpointCaller.RequestState;
@@ -40,6 +42,7 @@
 import org.apache.http.NoHttpResponseException;
 import org.apache.http.conn.HttpHostConnectException;
 import org.hamcrest.CoreMatchers;
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -113,6 +116,11 @@ public void setUp() throws IOException {
     Mockito.when(mockHttpResponse.getHeaders()).thenReturn(new HttpHeaders());
   }
 
+  @After
+  public void tearDown() {
+    System.clearProperty("jib.httpTimeout");
+  }
+
   @Test
   public void testCall_httpsPeerUnverified() throws IOException, RegistryException {
     verifyRetriesWithHttp(SSLPeerUnverifiedException.class);
@@ -253,6 +261,72 @@ public void testCall_disallowInsecure() throws IOException, RegistryException {
     }
   }
 
+  @Test
+  public void testHttpTimeout_propertyNotSet() throws IOException, RegistryException {
+    MockConnection mockConnection = new MockConnection((httpMethod, request) -> mockResponse);
+    Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
+    Mockito.when(mockResponse.getBody()).thenReturn(Mockito.mock(Blob.class));
+
+    Assert.assertNull(System.getProperty("jib.httpTimeout"));
+    testRegistryEndpointCallerSecure.call();
+
+    // We fall back to the default timeout:
+    // https://github.com/GoogleContainerTools/jib/pull/656#discussion_r203562639
+    Assert.assertNull(mockConnection.getRequestedHttpTimeout());
+  }
+
+  @Test
+  public void testHttpTimeout_stringValue() throws IOException, RegistryException {
+    MockConnection mockConnection = new MockConnection((httpMethod, request) -> mockResponse);
+    Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
+    Mockito.when(mockResponse.getBody()).thenReturn(Mockito.mock(Blob.class));
+
+    System.setProperty("jib.httpTimeout", "random string");
+    testRegistryEndpointCallerSecure.call();
+
+    Assert.assertNull(mockConnection.getRequestedHttpTimeout());
+  }
+
+  @Test
+  public void testHttpTimeout_negativeValue() throws IOException, RegistryException {
+    MockConnection mockConnection = new MockConnection((httpMethod, request) -> mockResponse);
+    Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
+    Mockito.when(mockResponse.getBody()).thenReturn(Mockito.mock(Blob.class));
+
+    System.setProperty("jib.httpTimeout", "-1");
+    testRegistryEndpointCallerSecure.call();
+
+    // We let the negative value pass through:
+    // https://github.com/GoogleContainerTools/jib/pull/656#discussion_r203562639
+    Assert.assertEquals(Integer.valueOf(-1), mockConnection.getRequestedHttpTimeout());
+  }
+
+  @Test
+  public void testHttpTimeout_0accepted() throws IOException, RegistryException {
+    System.setProperty("jib.httpTimeout", "0");
+
+    MockConnection mockConnection = new MockConnection((httpMethod, request) -> mockResponse);
+    Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
+
+    Mockito.when(mockResponse.getBody()).thenReturn(Mockito.mock(Blob.class));
+    testRegistryEndpointCallerSecure.call();
+
+    Assert.assertEquals(Integer.valueOf(0), mockConnection.getRequestedHttpTimeout());
+  }
+
+  @Test
+  public void testHttpTimeout() throws IOException, RegistryException {
+    System.setProperty("jib.httpTimeout", "7593");
+
+    MockConnection mockConnection = new MockConnection((httpMethod, request) -> mockResponse);
+    Mockito.when(mockConnectionFactory.apply(Mockito.any())).thenReturn(mockConnection);
+
+    Mockito.when(mockResponse.getBody()).thenReturn(Mockito.mock(Blob.class));
+    testRegistryEndpointCallerSecure.call();
+
+    Assert.assertEquals(Integer.valueOf(7593), mockConnection.getRequestedHttpTimeout());
+  }
+
   /** Verifies a request is retried with HTTP protocol if {@code exceptionClass} is thrown. */
   private void verifyRetriesWithHttp(Class<? extends Throwable> exceptionClass)
       throws IOException, RegistryException {