From 155a516b54ed60ec796cf5b9f2d8705012c5ceae Mon Sep 17 00:00:00 2001 From: Narayan Kamath Date: Fri, 16 Aug 2013 14:30:14 +0100 Subject: [PATCH] Upstream changes made directly to AOSP. This change is a combination of 4 separate commits to fix various bugs observed during android platform testing. https://android-review.googlesource.com/#/c/63781/ -------------------------------------------- Allow certificate inspections after connect. We don't have to wait for the request to be sent before allowing inspection of SSL certs. They can be inspected as soon as the connection is established. Fixes CTS test UrlConnectionTest#testInspectSslAfterConnect https://android-review.googlesource.com/#/c/63821/ --------------------------------------------- Respect read timeout on recycled connections. Partial fix for CTS test : URLConnectionTest#testGetKeepAlive. https://android-review.googlesource.com/#/c/63782/ ----------------------------------- Fix HttpUrlConnection#isUsingProxy. The earlier implementation disregarded what the ProxySelector had to say. We now query the selected route (if one has been established). Fixes compatibility test: HttpURLConnectionTest#testUsingProxySelector https://android-review.googlesource.com/#/c/63872/ -------------------------------------------- Don't support anything other than Basic auth. We should disregard authentication schemes other than "Basic" and let clients handle them themselves. The java Authenticator API gives us a user name and password combination, but we can't know how to format that information for any scheme other than basic. Historically: The JB implementation responds to challenges from an arbitrary scheme "X" by sending a header with scheme "X" but formatted like the "Basic" scheme. The current implementation responds to challenges from an arbitrary scheme "X" by sending a header with scheme "Basic" and formatter like the "Basic scheme". Partial fix for test cases in URLConnectionTest: - testAuthenticateWithCommaSeparatedAuthenticationMethods - testAuthenticateWithMultipleAuthenticationHeaders --- .../java/com/squareup/okhttp/Connection.java | 5 +++++ .../internal/http/HttpAuthenticator.java | 8 ++++++++ .../okhttp/internal/http/HttpEngine.java | 7 +++++++ .../internal/http/HttpURLConnectionImpl.java | 20 ++++++++++++++++++- .../okhttp/internal/http/HttpsEngine.java | 1 + .../internal/http/HttpsURLConnectionImpl.java | 2 +- .../squareup/okhttp/internal/http/Policy.java | 7 +++++++ .../internal/http/URLConnectionTest.java | 10 ++-------- 8 files changed, 50 insertions(+), 10 deletions(-) diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index f2149a3c1..6be565670 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java @@ -296,6 +296,11 @@ public final class Connection implements Closeable { return route.address.sslSocketFactory != null && route.proxy.type() == Proxy.Type.HTTP; } + public void updateReadTimeout(int newTimeout) throws IOException { + if (!connected) throw new IllegalStateException("updateReadTimeout - not connected"); + socket.setSoTimeout(newTimeout); + } + /** * To make an HTTPS connection over an HTTP proxy, send an unencrypted * CONNECT request to create the proxy connection. This may need to be diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpAuthenticator.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpAuthenticator.java index 566431e33..63f39e47f 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpAuthenticator.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpAuthenticator.java @@ -39,6 +39,10 @@ public final class HttpAuthenticator { @Override public Credential authenticate( Proxy proxy, URL url, List challenges) throws IOException { for (Challenge challenge : challenges) { + if (!"Basic".equals(challenge.getScheme())) { + continue; + } + PasswordAuthentication auth = Authenticator.requestPasswordAuthentication(url.getHost(), getConnectToInetAddress(proxy, url), url.getPort(), url.getProtocol(), challenge.getRealm(), challenge.getScheme(), url, Authenticator.RequestorType.SERVER); @@ -52,6 +56,10 @@ public final class HttpAuthenticator { @Override public Credential authenticateProxy( Proxy proxy, URL url, List challenges) throws IOException { for (Challenge challenge : challenges) { + if (!"Basic".equals(challenge.getScheme())) { + continue; + } + InetSocketAddress proxyAddress = (InetSocketAddress) proxy.address(); PasswordAuthentication auth = Authenticator.requestPasswordAuthentication( proxyAddress.getHostName(), getConnectToInetAddress(proxy, url), proxyAddress.getPort(), diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java index 2782faf30..142def466 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpEngine.java @@ -110,6 +110,9 @@ public class HttpEngine { /** The time when the request headers were written, or -1 if they haven't been written yet. */ long sentRequestMillis = -1; + /** Whether the connection has been established. */ + boolean connected; + /** * True if this client added an "Accept-Encoding: gzip" header field and is * therefore responsible for also decompressing the transfer stream. @@ -291,6 +294,8 @@ public class HttpEngine { connection.connect(client.getConnectTimeout(), client.getReadTimeout(), getTunnelConfig()); client.getConnectionPool().maybeShare(connection); client.getRoutesDatabase().connected(connection.getRoute()); + } else { + connection.updateReadTimeout(client.getReadTimeout()); } connected(connection); if (connection.getRoute().getProxy() != client.getProxy()) { @@ -304,6 +309,8 @@ public class HttpEngine { * pool. Subclasses use this hook to get a reference to the TLS data. */ protected void connected(Connection connection) { + policy.setSelectedProxy(connection.getRoute().getProxy()); + connected = true; } /** diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java index c615a87cc..f273039b1 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java @@ -75,6 +75,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { private int redirectionCount; protected IOException httpEngineFailure; protected HttpEngine httpEngine; + private Proxy selectedProxy; public HttpURLConnectionImpl(URL url, OkHttpClient client) { super(url); @@ -350,6 +351,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { if (readResponse) { httpEngine.readResponse(); } + return true; } catch (IOException e) { if (handleFailure(e)) { @@ -482,7 +484,19 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { } @Override public final boolean usingProxy() { - Proxy proxy = client.getProxy(); + if (selectedProxy != null) { + return isValidNonDirectProxy(selectedProxy); + } + + // This behavior is a bit odd (but is probably justified by the + // oddness of the APIs involved). Before a connection is established, + // this method will return true only if this connection was explicitly + // opened with a Proxy. We don't attempt to query the ProxySelector + // at all. + return isValidNonDirectProxy(client.getProxy()); + } + + private static boolean isValidNonDirectProxy(Proxy proxy) { return proxy != null && proxy.type() != Proxy.Type.DIRECT; } @@ -569,4 +583,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { this.fixedContentLength = contentLength; super.fixedContentLength = (int) Math.min(contentLength, Integer.MAX_VALUE); } + + @Override public final void setSelectedProxy(Proxy proxy) { + this.selectedProxy = proxy; + } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsEngine.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsEngine.java index 2427d4ed0..2bc1d68e4 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsEngine.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsEngine.java @@ -43,6 +43,7 @@ public final class HttpsEngine extends HttpEngine { @Override protected void connected(Connection connection) { this.sslSocket = (SSLSocket) connection.getSocket(); + super.connected(connection); } @Override protected boolean acceptCacheResponseType(CacheResponse cacheResponse) { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java index 0a4efea78..e8c656a2a 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpsURLConnectionImpl.java @@ -112,7 +112,7 @@ public final class HttpsURLConnectionImpl extends HttpsURLConnection { } private SSLSocket getSslSocket() { - if (delegate.httpEngine == null || delegate.httpEngine.sentRequestMillis == -1) { + if (delegate.httpEngine == null || !delegate.httpEngine.connected) { throw new IllegalStateException("Connection has not yet been established"); } return delegate.httpEngine instanceof HttpsEngine diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java index fcdd5cea0..0a29d4b1a 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Policy.java @@ -16,6 +16,7 @@ package com.squareup.okhttp.internal.http; import java.net.HttpURLConnection; +import java.net.Proxy; import java.net.URL; public interface Policy { @@ -39,4 +40,10 @@ public interface Policy { /** @see java.net.HttpURLConnection#setFixedLengthStreamingMode(int) */ long getFixedContentLength(); + + /** + * Sets the current proxy that this connection is using. + * @see java.net.HttpURLConnection#usingProxy + */ + void setSelectedProxy(Proxy proxy); } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 06918bdb6..5af2812cc 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -1207,10 +1207,7 @@ public final class URLConnectionTest { @Test public void nonStandardAuthenticationSchemeWithRealm() throws Exception { List calls = authCallsForHeader("WWW-Authenticate: Foo realm=\"Bar\""); - assertEquals(1, calls.size()); - String call = calls.get(0); - assertTrue(call, call.contains("scheme=Foo")); - assertTrue(call, call.contains("prompt=Bar")); + assertEquals(0, calls.size()); } // Digest auth is currently unsupported. Test that digest requests should fail reasonably. @@ -1220,10 +1217,7 @@ public final class URLConnectionTest { + "realm=\"testrealm@host.com\", qop=\"auth,auth-int\", " + "nonce=\"dcd98b7102dd2f0e8b11d0f600bfb0c093\", " + "opaque=\"5ccc069c403ebaf9f0171e9517f40e41\""); - assertEquals(1, calls.size()); - String call = calls.get(0); - assertTrue(call, call.contains("scheme=Digest")); - assertTrue(call, call.contains("prompt=testrealm@host.com")); + assertEquals(0, calls.size()); } @Test public void allAttributesSetInServerAuthenticationCallbacks() throws Exception {