From 61ff620852a0049d837b56f1cb8fe557c58c84ea Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 1 Aug 2019 18:00:53 -0400 Subject: [PATCH] Upgrade google-http-client to eliminate deprecated usage (#1882) * Upgrade google-http-client * Handle SSLHandshakeException * SSLException for all * Migrate deprecated usage * Upgrade Guava and Google HTTP Client * Migrate * No need to set proxy credentials on our side * Wrap up * Update comment * Upgrade to newer verisons * wip * Apply fix from google-http-client PR * Unset default SSLSocketFactory to create new one * Add test for proxy credential properties * Update comment * Add proxy properties test * No need to create readers repeatedly * Simplify * Remove blank lines * Use Apache v2 client * Migrate to 1.31 --- jib-core/build.gradle | 5 +- .../cloud/tools/jib/http/Connection.java | 71 ++------ .../ConnectionWithProxyCredentialsTest.java | 167 ------------------ .../cloud/tools/jib/http/MockConnection.java | 2 +- jib-gradle-plugin/build.gradle | 3 +- jib-maven-plugin/pom.xml | 8 +- jib-plugins-common/build.gradle | 3 +- 7 files changed, 34 insertions(+), 225 deletions(-) delete mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java diff --git a/jib-core/build.gradle b/jib-core/build.gradle index 4402863195..6b640838a1 100644 --- a/jib-core/build.gradle +++ b/jib-core/build.gradle @@ -16,6 +16,8 @@ group 'com.google.cloud.tools' sourceCompatibility = JavaVersion.VERSION_1_8 targetCompatibility = JavaVersion.VERSION_1_8 compileJava.options.encoding = 'UTF-8' +compileJava.options.compilerArgs += [ '-Xlint:deprecation' ] +compileTestJava.options.compilerArgs += [ '-Xlint:deprecation' ] repositories { mavenCentral() @@ -35,7 +37,8 @@ configurations { dependencies { // Make sure these are consistent with jib-maven-plugin. - implementation 'com.google.http-client:google-http-client:1.27.0' + implementation 'com.google.http-client:google-http-client:1.31.0' + implementation 'com.google.http-client:google-http-client-apache-v2:1.31.0' implementation 'org.apache.commons:commons-compress:1.18' implementation 'com.google.guava:guava:27.0.1-jre' implementation 'com.fasterxml.jackson.core:jackson-databind:2.9.9.2' 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/Connection.java index 750d6e27cc..834c87779f 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/Connection.java @@ -22,7 +22,8 @@ import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpResponse; 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.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import java.io.Closeable; @@ -31,9 +32,8 @@ import java.security.GeneralSecurityException; import java.util.function.Function; import javax.annotation.Nullable; -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; /** * Sends an HTTP {@link Request} and stores the {@link Response}. Clients should not send more than @@ -56,17 +56,12 @@ public class Connection implements Closeable { * @return {@link Connection} factory, a function that generates a {@link Connection} to a URL */ public static Function getConnectionFactory() { - /* - * Do not use {@link NetHttpTransport}. It does not process response errors properly. A new - * {@link ApacheHttpTransport} needs to be created for each connection because otherwise HTTP - * connection persistence causes the connection to throw {@link NoHttpResponseException}. - * - * @see https://github.com/google/google-http-java-client/issues/39 - */ - ApacheHttpTransport transport = new ApacheHttpTransport(); - addProxyCredentials(transport); - return url -> new Connection(url, transport); + // Do not use NetHttpTransport. It does not process response errors properly. + // See https://github.com/google/google-http-java-client/issues/39 + // + // A new ApacheHttpTransport needs to be created for each connection because otherwise HTTP + // connection persistence causes the connection to throw NoHttpResponseException. + return url -> new Connection(url, new ApacheHttpTransport()); } /** @@ -77,44 +72,14 @@ public static Function getConnectionFactory() { */ public static Function getInsecureConnectionFactory() throws GeneralSecurityException { - // Do not use {@link NetHttpTransport}. See {@link getConnectionFactory} for details. - ApacheHttpTransport transport = - new ApacheHttpTransport.Builder().doNotValidateCertificate().build(); - addProxyCredentials(transport); - return url -> new Connection(url, transport); - } - - /** - * 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)); + 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 url -> new Connection(url, new ApacheHttpTransport(httpClientBuilder.build())); } private HttpRequestFactory requestFactory; 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/ConnectionWithProxyCredentialsTest.java deleted file mode 100644 index 0b86e54a59..0000000000 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java +++ /dev/null @@ -1,167 +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 java.util.HashMap; -import java.util.Map; -import java.util.function.Consumer; -import org.apache.http.auth.AuthScope; -import org.apache.http.auth.Credentials; -import org.apache.http.impl.client.DefaultHttpClient; -import org.junit.After; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; - -/** Tests for {@link Connection} with setting proxy credentials. */ -public class ConnectionWithProxyCredentialsTest { - - private static final ImmutableList proxyProperties = - ImmutableList.of( - "http.proxyHost", - "http.proxyPort", - "http.proxyUser", - "http.proxyPassword", - "https.proxyHost", - "https.proxyPort", - "https.proxyUser", - "https.proxyPassword"); - - // HashMap to allow saving null values. - private final HashMap savedProperties = new HashMap<>(); - - private final ApacheHttpTransport transport = new ApacheHttpTransport(); - - @Before - public void setUp() { - proxyProperties.stream().forEach(key -> savedProperties.put(key, System.getProperty(key))); - proxyProperties.stream().forEach(key -> System.clearProperty(key)); - } - - @After - public void tearDown() { - Consumer> restoreProperty = - entry -> { - if (entry.getValue() == null) { - System.clearProperty(entry.getKey()); - } else { - System.setProperty(entry.getKey(), entry.getValue()); - } - }; - savedProperties.entrySet().stream().forEach(restoreProperty); - } - - @Test - public void testAddProxyCredentials_undefined() { - Connection.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"); - - Connection.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"); - - Connection.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"); - - Connection.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"); - - Connection.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"); - - Connection.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/MockConnection.java b/jib-core/src/test/java/com/google/cloud/tools/jib/http/MockConnection.java index 3470ba8d19..2ec95c638a 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/http/MockConnection.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/MockConnection.java @@ -17,7 +17,7 @@ package com.google.cloud.tools.jib.http; import com.google.api.client.http.GenericUrl; -import com.google.api.client.http.apache.ApacheHttpTransport; +import com.google.api.client.http.apache.v2.ApacheHttpTransport; import java.io.IOException; import java.util.function.BiFunction; diff --git a/jib-gradle-plugin/build.gradle b/jib-gradle-plugin/build.gradle index a1b7635e6c..9b8b922b6e 100644 --- a/jib-gradle-plugin/build.gradle +++ b/jib-gradle-plugin/build.gradle @@ -61,7 +61,8 @@ configurations { dependencies { // These are copied over from jib-core and are necessary for the jib-core sourcesets. - compile 'com.google.http-client:google-http-client:1.27.0' + compile 'com.google.http-client:google-http-client:1.31.0' + compile 'com.google.http-client:google-http-client-apache-v2:1.31.0' compile 'org.apache.commons:commons-compress:1.18' compile 'com.google.guava:guava:27.0.1-jre' compile 'com.fasterxml.jackson.core:jackson-databind:2.9.9.2' diff --git a/jib-maven-plugin/pom.xml b/jib-maven-plugin/pom.xml index 2526fe4442..3ccb8f23d7 100644 --- a/jib-maven-plugin/pom.xml +++ b/jib-maven-plugin/pom.xml @@ -53,7 +53,13 @@ com.google.http-client google-http-client - 1.27.0 + 1.31.0 + compile + + + com.google.http-client + google-http-client-apache-v2 + 1.31.0 compile diff --git a/jib-plugins-common/build.gradle b/jib-plugins-common/build.gradle index 6e41ea7cd4..c9e27e5d50 100644 --- a/jib-plugins-common/build.gradle +++ b/jib-plugins-common/build.gradle @@ -29,7 +29,8 @@ sourceSets { dependencies { // Make sure these are consistent with jib-maven-plugin. - compile 'com.google.http-client:google-http-client:1.27.0' + compile 'com.google.http-client:google-http-client:1.31.0' + compile 'com.google.http-client:google-http-client-apache-v2:1.31.0' compile 'org.apache.commons:commons-compress:1.18' compile 'com.google.guava:guava:27.0.1-jre' compile 'com.fasterxml.jackson.core:jackson-databind:2.9.9.2'