Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert Google HTTP Client upgrade #1980

Merged
merged 3 commits into from
Sep 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
chanseokoh marked this conversation as resolved.
Show resolved Hide resolved
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