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.

Sign up for GitHub

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

Jump to bottom

Update OkHttp version to 3.10.0 #72

Merged
merged 2 commits into from
Mar 2, 2018
Merged

Update OkHttp version to 3.10.0 #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c92a4ac
bump okhttp version to 3.10.0
Guardiola31337 Feb 27, 2018
3ab3c16
add hostname verifier to telemetry client settings builder needed to …
Guardiola31337 Mar 2, 2018
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
2 changes: 1 addition & 1 deletion gradle/dependencies.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ ext {
constraintLayout : '1.0.2',
mockito : '2.11.0',
testRunnerVersion: '1.0.1',
okhttp3 : '3.9.0',
okhttp3 : '3.10.0',
gson : '2.8.2',
espressoVersion : '3.0.1'
]
Expand Down
11 changes: 11 additions & 0 deletions libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryClientSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.HashMap;
import java.util.Map;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLSocketFactory;
import javax.net.ssl.X509TrustManager;

Expand All @@ -29,6 +30,7 @@ class TelemetryClientSettings {
private final HttpUrl baseUrl;
private final SSLSocketFactory sslSocketFactory;
private final X509TrustManager x509TrustManager;
private final HostnameVerifier hostnameVerifier;
private boolean debugLoggingEnabled;

TelemetryClientSettings(Builder builder) {
Expand All @@ -37,6 +39,7 @@ class TelemetryClientSettings {
this.baseUrl = builder.baseUrl;
this.sslSocketFactory = builder.sslSocketFactory;
this.x509TrustManager = builder.x509TrustManager;
this.hostnameVerifier = builder.hostnameVerifier;
this.debugLoggingEnabled = builder.debugLoggingEnabled;
}

Expand All @@ -63,6 +66,7 @@ Builder toBuilder() {
.baseUrl(baseUrl)
.sslSocketFactory(sslSocketFactory)
.x509TrustManager(x509TrustManager)
.hostnameVerifier(hostnameVerifier)
.debugLoggingEnabled(debugLoggingEnabled);
}

Expand All @@ -78,6 +82,7 @@ static final class Builder {
HttpUrl baseUrl = null;
SSLSocketFactory sslSocketFactory = null;
X509TrustManager x509TrustManager = null;
HostnameVerifier hostnameVerifier = null;
boolean debugLoggingEnabled = false;

Builder() {
Expand Down Expand Up @@ -112,6 +117,11 @@ Builder x509TrustManager(X509TrustManager x509TrustManager) {
return this;
}

Builder hostnameVerifier(HostnameVerifier hostnameVerifier) {
this.hostnameVerifier = hostnameVerifier;
return this;
}

Builder debugLoggingEnabled(boolean debugLoggingEnabled) {
this.debugLoggingEnabled = debugLoggingEnabled;
return this;
Expand All @@ -135,6 +145,7 @@ private OkHttpClient configureHttpClient() {
.connectionSpecs(Arrays.asList(ConnectionSpec.MODERN_TLS, ConnectionSpec.COMPATIBLE_TLS));
if (isSocketFactoryUnset(sslSocketFactory, x509TrustManager)) {
builder.sslSocketFactory(sslSocketFactory, x509TrustManager);
builder.hostnameVerifier(hostnameVerifier);
}

return builder.build();
Expand Down
22 changes: 20 additions & 2 deletions libtelemetry/src/main/java/com/mapbox/android/telemetry/TelemetryUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import java.util.Locale;
import java.util.UUID;

import okhttp3.internal.Util;
import okio.Buffer;

class TelemetryUtils {
static final String MAPBOX_SHARED_PREFERENCES = "MapboxSharedPreferences";
Expand All @@ -37,7 +37,7 @@ static String obtainUniversalUniqueIdentifier() {

static String createFullUserAgent(String userAgent, Context context) {
String appIdentifier = TelemetryUtils.obtainApplicationIdentifier(context);
String newUserAgent = Util.toHumanReadableAscii(String.format(DEFAULT_LOCALE, TWO_STRING_FORMAT, appIdentifier,
String newUserAgent = toHumanReadableAscii(String.format(DEFAULT_LOCALE, TWO_STRING_FORMAT, appIdentifier,
userAgent));
String fullUserAgent = TextUtils.isEmpty(appIdentifier) ? userAgent : newUserAgent;

Expand Down Expand Up @@ -90,4 +90,22 @@ private static String obtainApplicationIdentifier(Context context) {
return EMPTY_STRING;
}
}

private static String toHumanReadableAscii(String s) {
for (int i = 0, length = s.length(), c; i < length; i += Character.charCount(c)) {
c = s.codePointAt(i);
if (c > '\u001f' && c < '\u007f') {
continue;
}

Buffer buffer = new Buffer();
buffer.writeUtf8(s, 0, i);
for (int j = i; j < length; j += Character.charCount(c)) {
c = s.codePointAt(j);
buffer.writeUtf8CodePoint(c > '\u001f' && c < '\u007f' ? c : '?');
}
return buffer.readUtf8();
}
return s;
}
}
11 changes: 10 additions & 1 deletion libtelemetry/src/test/java/com/mapbox/android/telemetry/MockWebServerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
import java.util.Arrays;
import java.util.List;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLSession;

import okhttp3.HttpUrl;
import okhttp3.internal.tls.SslClient;
import okhttp3.mockwebserver.internal.tls.SslClient;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we continue to use an internal api if future changes could cause issues similar issues? Is there any way we can get rid of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a bit different because MockWebServer is only used for testing. So although APIs could change it's not a problem because it's not affecting production code which is what we're exposing and what could cause issues in the future. Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we can get rid of this?

Nope, it's actually needed for testing HTTP requests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Sounds good.

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
Expand Down Expand Up @@ -172,6 +175,12 @@ private TelemetryClientSettings provideDefaultTelemetryClientSettings() {
.baseUrl(localUrl)
.sslSocketFactory(sslClient.socketFactory)
.x509TrustManager(sslClient.trustManager)
.hostnameVerifier(new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true;
}
})
.build();
}
}
11 changes: 10 additions & 1 deletion libtelemetry/src/test/java/com/mapbox/android/telemetry/TelemetryClientTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;

import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.SSLSession;

import okhttp3.Call;
import okhttp3.Callback;
import okhttp3.HttpUrl;
import okhttp3.OkHttpClient;
import okhttp3.Response;
import okhttp3.internal.tls.SslClient;
import okhttp3.mockwebserver.internal.tls.SslClient;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as ☝️

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already answered ☝️


import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -183,6 +186,12 @@ public void checksRequestTimeoutFailure() throws Exception {
.baseUrl(localUrl)
.sslSocketFactory(sslClient.socketFactory)
.x509TrustManager(sslClient.trustManager)
.hostnameVerifier(new HostnameVerifier() {
@Override
public boolean verify(String hostname, SSLSession session) {
return true;
}
})
.build();
TelemetryClient telemetryClient = new TelemetryClient("anyAccessToken", "anyUserAgent", telemetryClientSettings,
mock(Logger.class));
Expand Down