From 0eb91da22e7ba321ea59598e8ae46d6ac0cdd9e4 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 12 Sep 2019 11:09:24 -0400 Subject: [PATCH 1/3] Revert Google HTTP Client upgrade --- build.gradle | 25 +-- jib-core/build.gradle | 8 +- .../cloud/tools/jib/http/Connection.java | 56 ++++-- .../ConnectionWithProxyCredentialsTest.java | 167 ++++++++++++++++++ .../cloud/tools/jib/http/MockConnection.java | 2 +- jib-gradle-plugin/build.gradle | 6 +- jib-plugins-common/build.gradle | 3 +- 7 files changed, 216 insertions(+), 51 deletions(-) create mode 100644 jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java diff --git a/build.gradle b/build.gradle index cbc802143d..43831a19e1 100644 --- a/build.gradle +++ b/build.gradle @@ -46,16 +46,10 @@ subprojects { // For Google libraries, check , , , // ... in https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/pom.xml // for best compatibility. - GOOGLE_HTTP_CLIENT: '1.31.0', - GOOGLE_HTTP_CLIENT_APACHE_V2: '1.31.0', + GOOGLE_HTTP_CLIENT: '1.27.0', GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP: '0.16.2', GUAVA: '28.0-jre', - // TODO: remove once https://github.com/googleapis/google-http-java-client/issues/795 is fixed and released. - // Forcing to downgrade this to 4.5.6 fixes https://github.com/GoogleContainerTools/jib/issues/1914 - // However, #795 and upgrading httpclient alone may not fix #1914. We may need to explicitly disable URI - // normalization as discussed in #795. - APACHE_HTTP_CLIENT_OVERRIDE: '4.5.6', COMMONS_COMPRESS: '1.18', JACKSON_DATABIND: '2.9.9.2', ASM: '7.0', @@ -67,23 +61,6 @@ subprojects { SYSTEM_RULES: '1.19.0', ] - // Use this to ensure we correctly override transitive dependencies - // TODO: There might be a plugin that does this - task ensureTransitiveDependencyOverrides { - def rules = ["httpclient": dependencyVersions.APACHE_HTTP_CLIENT_OVERRIDE] - doLast { - configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.each { artifact -> - def dependency = artifact.moduleVersion.id - if (rules[dependency.name] && rules[dependency.name] != dependency.version) { - throw new GradleException( - dependency.name + " version error in " + project - + ", expected:" + rules[dependency.name] - + ", found:" + dependency.version); - } - } - } - } - compileJava.dependsOn ensureTransitiveDependencyOverrides /* PROJECT DEPENDENCY VERSIONS */ /* NULLAWAY */ diff --git a/jib-core/build.gradle b/jib-core/build.gradle index dd158cd31f..1a296baa48 100644 --- a/jib-core/build.gradle +++ b/jib-core/build.gradle @@ -4,16 +4,10 @@ plugins { } dependencies { - implementation("com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}") { - exclude group: "org.apache.httpcomponents", module: "httpclient" - } - implementation("com.google.http-client:google-http-client-apache-v2:${dependencyVersions.GOOGLE_HTTP_CLIENT_APACHE_V2}") { - exclude group: "org.apache.httpcomponents", module: "httpclient" - } + implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" implementation("com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}") { exclude group: "org.apache.httpcomponents", module: "httpclient" } - implementation "org.apache.httpcomponents:httpclient:${dependencyVersions.APACHE_HTTP_CLIENT_OVERRIDE}" implementation "org.apache.commons:commons-compress:${dependencyVersions.COMMONS_COMPRESS}" implementation "com.google.guava:guava:${dependencyVersions.GUAVA}" 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 834c87779f..6e678cbe21 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,8 +22,7 @@ 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.v2.ApacheHttpTransport; -import com.google.api.client.util.SslUtils; +import com.google.api.client.http.apache.ApacheHttpTransport; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import java.io.Closeable; @@ -32,8 +31,9 @@ import java.security.GeneralSecurityException; import java.util.function.Function; import javax.annotation.Nullable; -import org.apache.http.conn.ssl.NoopHostnameVerifier; -import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.auth.AuthScope; +import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.impl.client.DefaultHttpClient; /** * Sends an HTTP {@link Request} and stores the {@link Response}. Clients should not send more than @@ -61,7 +61,9 @@ public static Function getConnectionFactory() { // // 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()); + ApacheHttpTransport transport = new ApacheHttpTransport(); + addProxyCredentials(transport); + return url -> new Connection(url, transport); } /** @@ -72,14 +74,44 @@ public static Function getConnectionFactory() { */ public static Function getInsecureConnectionFactory() throws GeneralSecurityException { - 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())); + 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)); } 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 new file mode 100644 index 0000000000..0b86e54a59 --- /dev/null +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/http/ConnectionWithProxyCredentialsTest.java @@ -0,0 +1,167 @@ +/* + * 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 2ec95c638a..3470ba8d19 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.v2.ApacheHttpTransport; +import com.google.api.client.http.apache.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 e3c6b8045b..9a56683c5b 100644 --- a/jib-gradle-plugin/build.gradle +++ b/jib-gradle-plugin/build.gradle @@ -14,14 +14,10 @@ repositories { } dependencies { - sourceProject project(":jib-core") sourceProject project(":jib-plugins-common") - implementation ("com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}") { - exclude group: "org.apache.httpcomponents", module: "httpclient" - } - implementation "org.apache.httpcomponents:httpclient:${dependencyVersions.APACHE_HTTP_CLIENT_OVERRIDE}" + implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" implementation "com.google.guava:guava:${dependencyVersions.GUAVA}" implementation "com.fasterxml.jackson.core:jackson-databind:${dependencyVersions.JACKSON_DATABIND}" diff --git a/jib-plugins-common/build.gradle b/jib-plugins-common/build.gradle index 413febd980..29a493a6e7 100644 --- a/jib-plugins-common/build.gradle +++ b/jib-plugins-common/build.gradle @@ -1,9 +1,8 @@ dependencies { implementation project(':jib-core') - implementation ("com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}") { + implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" exclude group: "org.apache.httpcomponents", module: "httpclient" } - implementation "org.apache.httpcomponents:httpclient:${dependencyVersions.APACHE_HTTP_CLIENT_OVERRIDE}" implementation "com.google.guava:guava:${dependencyVersions.GUAVA}" implementation "com.fasterxml.jackson.core:jackson-databind:${dependencyVersions.JACKSON_DATABIND}" From cb3a27901476011c86037c845a08269723331c6e Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 12 Sep 2019 11:19:38 -0400 Subject: [PATCH 2/3] Fix broken build.gradle --- jib-plugins-common/build.gradle | 2 -- 1 file changed, 2 deletions(-) diff --git a/jib-plugins-common/build.gradle b/jib-plugins-common/build.gradle index 29a493a6e7..e35a0ca220 100644 --- a/jib-plugins-common/build.gradle +++ b/jib-plugins-common/build.gradle @@ -1,8 +1,6 @@ dependencies { implementation project(':jib-core') implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" - exclude group: "org.apache.httpcomponents", module: "httpclient" - } implementation "com.google.guava:guava:${dependencyVersions.GUAVA}" implementation "com.fasterxml.jackson.core:jackson-databind:${dependencyVersions.JACKSON_DATABIND}" From bf0ddbbce8202a15faa67d67980bbc4a995193e5 Mon Sep 17 00:00:00 2001 From: Chanseok Oh Date: Thu, 12 Sep 2019 12:16:05 -0400 Subject: [PATCH 3/3] Exclude google-http-client from google-auth-library-oauth2-http --- build.gradle | 17 +++++++++++++++++ jib-core/build.gradle | 2 +- jib-gradle-plugin/build.gradle | 5 ----- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/build.gradle b/build.gradle index 43831a19e1..983cb4db3c 100644 --- a/build.gradle +++ b/build.gradle @@ -61,6 +61,23 @@ subprojects { SYSTEM_RULES: '1.19.0', ] + // 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] + doLast { + configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.each { artifact -> + def dependency = artifact.moduleVersion.id + if (rules[dependency.name] && rules[dependency.name] != dependency.version) { + throw new GradleException( + dependency.name + " version error in " + project + + ", expected:" + rules[dependency.name] + + ", found:" + dependency.version); + } + } + } + } + compileJava.dependsOn ensureTransitiveDependencyOverrides /* PROJECT DEPENDENCY VERSIONS */ /* NULLAWAY */ diff --git a/jib-core/build.gradle b/jib-core/build.gradle index 1a296baa48..624a319697 100644 --- a/jib-core/build.gradle +++ b/jib-core/build.gradle @@ -6,7 +6,7 @@ plugins { dependencies { implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" implementation("com.google.auth:google-auth-library-oauth2-http:${dependencyVersions.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP}") { - exclude group: "org.apache.httpcomponents", module: "httpclient" + exclude group: "com.google.http-client", module: "google-http-client" } implementation "org.apache.commons:commons-compress:${dependencyVersions.COMMONS_COMPRESS}" diff --git a/jib-gradle-plugin/build.gradle b/jib-gradle-plugin/build.gradle index 9a56683c5b..24d81cd15c 100644 --- a/jib-gradle-plugin/build.gradle +++ b/jib-gradle-plugin/build.gradle @@ -17,11 +17,6 @@ dependencies { sourceProject project(":jib-core") sourceProject project(":jib-plugins-common") - implementation "com.google.http-client:google-http-client:${dependencyVersions.GOOGLE_HTTP_CLIENT}" - - implementation "com.google.guava:guava:${dependencyVersions.GUAVA}" - implementation "com.fasterxml.jackson.core:jackson-databind:${dependencyVersions.JACKSON_DATABIND}" - testImplementation "junit:junit:${dependencyVersions.JUNIT}" testImplementation "org.mockito:mockito-core:${dependencyVersions.MOCKITO_CORE}" testImplementation "org.slf4j:slf4j-api:${dependencyVersions.SLF4J_API}"