Skip to content

Commit

Permalink
Revert Google HTTP Client upgrade (#1980)
Browse files Browse the repository at this point in the history
* Revert Google HTTP Client upgrade
* Exclude google-http-client from google-auth-library-oauth2-http
  • Loading branch information
chanseokoh authored Sep 12, 2019
1 parent cdb696f commit 1deba99
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 42 deletions.
10 changes: 2 additions & 8 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +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.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',
Expand All @@ -70,7 +64,7 @@ subprojects {
// 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]
def rules = ["google-http-client": dependencyVersions.GOOGLE_HTTP_CLIENT]
doLast {
configurations.runtimeClasspath.resolvedConfiguration.resolvedArtifacts.each { artifact ->
def dependency = artifact.moduleVersion.id
Expand Down
10 changes: 2 additions & 8 deletions jib-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
exclude group: "com.google.http-client", module: "google-http-client"
}
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}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -61,7 +61,9 @@ public static Function<URL, Connection> 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);
}

/**
Expand All @@ -72,14 +74,44 @@ public static Function<URL, Connection> getConnectionFactory() {
*/
public static Function<URL, Connection> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> 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<String, String> 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<Map.Entry<String, String>> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
9 changes: 0 additions & 9 deletions jib-gradle-plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,9 @@ 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.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}"
Expand Down
5 changes: 1 addition & 4 deletions jib-plugins-common/build.gradle
Original file line number Diff line number Diff line change
@@ -1,9 +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 "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}"

Expand Down

0 comments on commit 1deba99

Please sign in to comment.