From d3675a5e0f7ca0287d3441d1dc1284ea7b06a26d Mon Sep 17 00:00:00 2001 From: jwilson Date: Fri, 1 Jan 2016 17:16:47 -0500 Subject: [PATCH] Detect API use errors earlier. Previously we would accept invalid input, but crash when that invalid input was used. --- .../internal/huc/JavaApiConverter.java | 5 ++- .../test/java/okhttp3/URLConnectionTest.java | 8 +++-- .../okhttp3/internal/http/HeadersTest.java | 32 +++++++++++++++++++ .../internal/huc/HttpURLConnectionImpl.java | 8 +++-- okhttp/src/main/java/okhttp3/Headers.java | 16 +++------- .../src/main/java/okhttp3/OkHttpClient.java | 26 +++++++++++++++ 6 files changed, 77 insertions(+), 18 deletions(-) diff --git a/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java b/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java index e862022f4..0c7a75a87 100644 --- a/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java +++ b/okhttp-android-support/src/main/java/okhttp3/internal/huc/JavaApiConverter.java @@ -633,7 +633,7 @@ public final class JavaApiConverter { if (position < 0) { throw new IllegalArgumentException("Invalid header index: " + position); } - if (position == 0) { + if (position == 0 || position > response.headers().size()) { return null; } return response.headers().name(position - 1); @@ -648,6 +648,9 @@ public final class JavaApiConverter { if (position == 0) { return StatusLine.get(response).toString(); } + if (position > response.headers().size()) { + return null; + } return response.headers().value(position - 1); } diff --git a/okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java b/okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java index 16aec50b8..133b11e69 100644 --- a/okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java +++ b/okhttp-tests/src/test/java/okhttp3/URLConnectionTest.java @@ -590,8 +590,7 @@ public final class URLConnectionTest { assertEquals(1, server.takeRequest().getSequenceNumber()); } - @Test public void connectViaHttpsReusingConnectionsDifferentFactories() - throws IOException, InterruptedException { + @Test public void connectViaHttpsReusingConnectionsDifferentFactories() throws Exception { server.useHttps(sslContext.getSocketFactory(), false); server.enqueue(new MockResponse().setBody("this response comes via HTTPS")); server.enqueue(new MockResponse().setBody("another response via HTTPS")); @@ -604,8 +603,11 @@ public final class URLConnectionTest { HttpURLConnection connection1 = urlFactory.open(server.url("/").url()); assertContent("this response comes via HTTPS", connection1); + SSLContext sslContext2 = SSLContext.getInstance("TLS"); + sslContext2.init(null, null, null); + SSLSocketFactory sslSocketFactory2 = sslContext2.getSocketFactory(); urlFactory.setClient(urlFactory.client().newBuilder() - .sslSocketFactory(null) + .sslSocketFactory(sslSocketFactory2) .build()); HttpURLConnection connection2 = urlFactory.open(server.url("/").url()); try { diff --git a/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java b/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java index 18e41d9e9..039126387 100644 --- a/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java +++ b/okhttp-tests/src/test/java/okhttp3/internal/http/HeadersTest.java @@ -302,4 +302,36 @@ public final class HeadersTest { assertEquals(2, headerMap.get("cache-control").size()); assertEquals(1, headerMap.get("user-agent").size()); } + + @Test public void nameIndexesAreStrict() { + Headers headers = Headers.of("a", "b", "c", "d"); + try { + headers.name(-1); + fail(); + } catch (IndexOutOfBoundsException expected) { + } + assertEquals("a", headers.name(0)); + assertEquals("c", headers.name(1)); + try { + headers.name(2); + fail(); + } catch (IndexOutOfBoundsException expected) { + } + } + + @Test public void valueIndexesAreStrict() { + Headers headers = Headers.of("a", "b", "c", "d"); + try { + headers.value(-1); + fail(); + } catch (IndexOutOfBoundsException expected) { + } + assertEquals("b", headers.value(0)); + assertEquals("d", headers.value(1)); + try { + headers.value(2); + fail(); + } catch (IndexOutOfBoundsException expected) { + } + } } diff --git a/okhttp-urlconnection/src/main/java/okhttp3/internal/huc/HttpURLConnectionImpl.java b/okhttp-urlconnection/src/main/java/okhttp3/internal/huc/HttpURLConnectionImpl.java index 48edc1436..fabedaa67 100644 --- a/okhttp-urlconnection/src/main/java/okhttp3/internal/huc/HttpURLConnectionImpl.java +++ b/okhttp-urlconnection/src/main/java/okhttp3/internal/huc/HttpURLConnectionImpl.java @@ -181,7 +181,9 @@ public class HttpURLConnectionImpl extends HttpURLConnection { */ @Override public final String getHeaderField(int position) { try { - return getHeaders().value(position); + Headers headers = getHeaders(); + if (position < 0 || position >= headers.size()) return null; + return headers.value(position); } catch (IOException e) { return null; } @@ -203,7 +205,9 @@ public class HttpURLConnectionImpl extends HttpURLConnection { @Override public final String getHeaderFieldKey(int position) { try { - return getHeaders().name(position); + Headers headers = getHeaders(); + if (position < 0 || position >= headers.size()) return null; + return headers.name(position); } catch (IOException e) { return null; } diff --git a/okhttp/src/main/java/okhttp3/Headers.java b/okhttp/src/main/java/okhttp3/Headers.java index 43f1dd1d4..a2dcf952d 100644 --- a/okhttp/src/main/java/okhttp3/Headers.java +++ b/okhttp/src/main/java/okhttp3/Headers.java @@ -73,22 +73,14 @@ public final class Headers { return namesAndValues.length / 2; } - /** Returns the field at {@code position} or null if that is out of range. */ + /** Returns the field at {@code position}. */ public String name(int index) { - int nameIndex = index * 2; - if (nameIndex < 0 || nameIndex >= namesAndValues.length) { - return null; - } - return namesAndValues[nameIndex]; + return namesAndValues[index * 2]; } - /** Returns the value at {@code index} or null if that is out of range. */ + /** Returns the value at {@code index}. */ public String value(int index) { - int valueIndex = index * 2 + 1; - if (valueIndex < 0 || valueIndex >= namesAndValues.length) { - return null; - } - return namesAndValues[valueIndex]; + return namesAndValues[index * 2 + 1]; } /** Returns an immutable case-insensitive set of header names. */ diff --git a/okhttp/src/main/java/okhttp3/OkHttpClient.java b/okhttp/src/main/java/okhttp3/OkHttpClient.java index 121839f3e..30257a9cf 100644 --- a/okhttp/src/main/java/okhttp3/OkHttpClient.java +++ b/okhttp/src/main/java/okhttp3/OkHttpClient.java @@ -453,6 +453,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, {@linkplain CookieJar#NO_COOKIES no cookies} will be accepted nor provided. */ public Builder cookieJar(CookieJar cookieJar) { + if (cookieJar == null) throw new NullPointerException("cookieJar == null"); this.cookieJar = cookieJar; return this; } @@ -475,6 +476,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, the {@link Dns#SYSTEM system-wide default} DNS will be used. */ public Builder dns(Dns dns) { + if (dns == null) throw new NullPointerException("dns == null"); this.dns = dns; return this; } @@ -488,6 +490,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { * be used. */ public Builder socketFactory(SocketFactory socketFactory) { + if (socketFactory == null) throw new NullPointerException("socketFactory == null"); this.socketFactory = socketFactory; return this; } @@ -498,6 +501,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, a lazily created SSL socket factory will be used. */ public Builder sslSocketFactory(SSLSocketFactory sslSocketFactory) { + if (sslSocketFactory == null) throw new NullPointerException("sslSocketFactory == null"); this.sslSocketFactory = sslSocketFactory; return this; } @@ -509,6 +513,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, a default hostname verifier will be used. */ public Builder hostnameVerifier(HostnameVerifier hostnameVerifier) { + if (hostnameVerifier == null) throw new NullPointerException("hostnameVerifier == null"); this.hostnameVerifier = hostnameVerifier; return this; } @@ -519,6 +524,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { * trust. Pinning certificates avoids the need to trust certificate authorities. */ public Builder certificatePinner(CertificatePinner certificatePinner) { + if (certificatePinner == null) throw new NullPointerException("certificatePinner == null"); this.certificatePinner = certificatePinner; return this; } @@ -530,6 +536,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, the {@linkplain Authenticator#NONE no authentication will be attempted}. */ public Builder authenticator(Authenticator authenticator) { + if (authenticator == null) throw new NullPointerException("authenticator == null"); this.authenticator = authenticator; return this; } @@ -541,6 +548,7 @@ public class OkHttpClient implements Cloneable, Call.Factory { *

If unset, the {@linkplain Authenticator#NONE no authentication will be attempted}. */ public Builder proxyAuthenticator(Authenticator proxyAuthenticator) { + if (proxyAuthenticator == null) throw new NullPointerException("proxyAuthenticator == null"); this.proxyAuthenticator = proxyAuthenticator; return this; } @@ -654,11 +662,29 @@ public class OkHttpClient implements Cloneable, Call.Factory { return this; } + /** + * Returns a modifiable list of interceptors that observe the full span of each call: from + * before the connection is established (if any) until after the response source is selected + * (either the origin server, cache, or both). + */ + public List interceptors() { + return interceptors; + } + public Builder addInterceptor(Interceptor interceptor) { interceptors.add(interceptor); return this; } + /** + * Returns a modifiable list of interceptors that observe a single network request and response. + * These interceptors must call {@link Interceptor.Chain#proceed} exactly once: it is an error + * for a network interceptor to short-circuit or repeat a network request. + */ + public List networkInterceptors() { + return networkInterceptors; + } + public Builder addNetworkInterceptor(Interceptor interceptor) { networkInterceptors.add(interceptor); return this;