From ab4927b857b1feb45d78f2fec8ae27ab6ad7d541 Mon Sep 17 00:00:00 2001 From: Jesse Wilson Date: Fri, 18 Apr 2014 20:46:54 -0400 Subject: [PATCH] Hide Protocol's internal byte string. --- .../okhttp/internal/spdy/SpdyServer.java | 2 +- .../okhttp/mockwebserver/MockWebServer.java | 8 ++-- .../java/com/squareup/okhttp/curl/Main.java | 10 +---- .../okhttp/internal/http/HeadersTest.java | 8 ++-- .../internal/http/URLConnectionTest.java | 2 +- .../java/com/squareup/okhttp/Connection.java | 2 +- .../com/squareup/okhttp/OkHttpClient.java | 6 +-- .../java/com/squareup/okhttp/Protocol.java | 42 +++++++++++-------- .../squareup/okhttp/internal/Platform.java | 22 ++++------ .../okhttp/internal/http/HttpConnection.java | 2 +- .../internal/http/HttpURLConnectionImpl.java | 3 +- .../okhttp/internal/http/SpdyTransport.java | 2 +- 12 files changed, 49 insertions(+), 60 deletions(-) diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java index 38da33ba1..27ddce073 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/internal/spdy/SpdyServer.java @@ -70,7 +70,7 @@ public final class SpdyServer implements IncomingStreamHandler { System.out.println("UNSUPPORTED"); } @Override public List protocols() { - return Arrays.asList(Protocol.SPDY_3.name.utf8()); + return Arrays.asList(Protocol.SPDY_3.toString()); } @Override public void protocolSelected(String protocol) { System.out.println("PROTOCOL SELECTED: " + protocol); diff --git a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java index 0f0a875b0..804b5c967 100644 --- a/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java +++ b/mockwebserver/src/main/java/com/squareup/okhttp/mockwebserver/MockWebServer.java @@ -333,8 +333,10 @@ public final class MockWebServer { sslSocket.startHandshake(); if (npnEnabled) { - ByteString selectedProtocol = Platform.get().getNpnSelectedProtocol(sslSocket); - protocol = Protocol.find(selectedProtocol); + ByteString protocolBytes = Platform.get().getNpnSelectedProtocol(sslSocket); + protocol = protocolBytes != null + ? Protocol.find(protocolBytes.utf8()) + : Protocol.HTTP_11; } openClientSockets.remove(raw); } else { @@ -634,7 +636,7 @@ public final class MockWebServer { writeResponse(stream, response); if (logger.isLoggable(Level.INFO)) { logger.info("Received request: " + request + " and responded: " + response - + " protocol is " + protocol.name.utf8()); + + " protocol is " + protocol.toString()); } } diff --git a/okcurl/src/main/java/com/squareup/okhttp/curl/Main.java b/okcurl/src/main/java/com/squareup/okhttp/curl/Main.java index 6b8cfd08e..e2ea9b183 100644 --- a/okcurl/src/main/java/com/squareup/okhttp/curl/Main.java +++ b/okcurl/src/main/java/com/squareup/okhttp/curl/Main.java @@ -15,9 +15,7 @@ */ package com.squareup.okhttp.curl; -import com.google.common.base.Function; import com.google.common.base.Joiner; -import com.google.common.collect.Lists; import com.squareup.okhttp.ConnectionPool; import com.squareup.okhttp.Headers; import com.squareup.okhttp.MediaType; @@ -34,7 +32,6 @@ import java.io.IOException; import java.io.InputStream; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; -import java.util.Arrays; import java.util.List; import java.util.Properties; import javax.net.ssl.HostnameVerifier; @@ -73,12 +70,7 @@ public class Main extends HelpOption implements Runnable { } private static String protocols() { - return Joiner.on(", ").join(Lists.transform(Arrays.asList(Protocol.values()), - new Function() { - @Override public String apply(Protocol protocol) { - return protocol.name.utf8(); - } - })); + return Joiner.on(", ").join(Protocol.values()); } @Option(name = { "-X", "--request" }, description = "Specify request command to use") diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java index 80db74704..0f18da8d9 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java @@ -43,9 +43,9 @@ public final class HeadersTest { assertEquals("HTTP/1.1 200 OK", response.statusLine()); assertEquals("no-cache, no-store", headers.get("cache-control")); assertEquals("Cookie2", headers.get("set-cookie")); - assertEquals(Protocol.SPDY_3.name.utf8(), headers.get(OkHeaders.SELECTED_PROTOCOL)); + assertEquals(Protocol.SPDY_3.toString(), headers.get(OkHeaders.SELECTED_PROTOCOL)); assertEquals(OkHeaders.SELECTED_PROTOCOL, headers.name(0)); - assertEquals(Protocol.SPDY_3.name.utf8(), headers.value(0)); + assertEquals(Protocol.SPDY_3.toString(), headers.value(0)); assertEquals("cache-control", headers.name(1)); assertEquals("no-cache, no-store", headers.value(1)); assertEquals("set-cookie", headers.name(2)); @@ -67,7 +67,7 @@ public final class HeadersTest { Headers headers = response.headers(); assertEquals(1, headers.size()); assertEquals(OkHeaders.SELECTED_PROTOCOL, headers.name(0)); - assertEquals(Protocol.SPDY_3.name.utf8(), headers.value(0)); + assertEquals(Protocol.SPDY_3.toString(), headers.value(0)); } @Test public void readNameValueBlockDropsForbiddenHeadersHttp2() throws IOException { @@ -81,7 +81,7 @@ public final class HeadersTest { Headers headers = response.headers(); assertEquals(1, headers.size()); assertEquals(OkHeaders.SELECTED_PROTOCOL, headers.name(0)); - assertEquals(Protocol.HTTP_2.name.utf8(), headers.value(0)); + assertEquals(Protocol.HTTP_2.toString(), headers.value(0)); } @Test public void toNameValueBlock() { diff --git a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index c5bdb6405..4c74cb20f 100644 --- a/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/okhttp-tests/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -2723,7 +2723,7 @@ public final class URLConnectionTest { client.setProtocols(Arrays.asList(Protocol.HTTP_11, protocol)); connection = client.open(server.getUrl("/")); List protocolValues = connection.getHeaderFields().get(SELECTED_PROTOCOL); - assertEquals(Arrays.asList(protocol.name.utf8()), protocolValues); + assertEquals(Arrays.asList(protocol.toString()), protocolValues); assertContent("A", connection); } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index 4b9999c04..4b43ac2ae 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java @@ -207,7 +207,7 @@ public final class Connection implements Closeable { ByteString maybeProtocol; Protocol selectedProtocol = Protocol.HTTP_11; if (useNpn && (maybeProtocol = platform.getNpnSelectedProtocol(sslSocket)) != null) { - selectedProtocol = Protocol.find(maybeProtocol); // Throws IOE on unknown. + selectedProtocol = Protocol.find(maybeProtocol.utf8()); // Throws IOE on unknown. } if (selectedProtocol.spdyVariant) { diff --git a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java index f5f6eb9da..6c48eecc5 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java +++ b/okhttp/src/main/java/com/squareup/okhttp/OkHttpClient.java @@ -39,7 +39,6 @@ import javax.net.SocketFactory; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSocketFactory; -import okio.ByteString; /** * Configures and creates HTTP connections. Most applications can use a single @@ -320,8 +319,7 @@ public final class OkHttpClient implements URLStreamHandlerFactory, Cloneable { List protocols = new ArrayList(transports.size()); for (int i = 0, size = transports.size(); i < size; i++) { try { - Protocol protocol = Protocol.find(ByteString.encodeUtf8(transports.get(i))); - protocols.add(protocol); + protocols.add(Protocol.find(transports.get(i))); } catch (IOException e) { throw new IllegalArgumentException(e); } @@ -378,7 +376,7 @@ public final class OkHttpClient implements URLStreamHandlerFactory, Cloneable { public List getTransports() { List transports = new ArrayList(protocols.size()); for (int i = 0, size = protocols.size(); i < size; i++) { - transports.add(protocols.get(i).name.utf8()); + transports.add(protocols.get(i).toString()); } return transports; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Protocol.java b/okhttp/src/main/java/com/squareup/okhttp/Protocol.java index 9024bebfc..772c56048 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Protocol.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Protocol.java @@ -19,18 +19,17 @@ import com.squareup.okhttp.internal.Util; import java.io.IOException; import java.util.Arrays; import java.util.List; -import okio.ByteString; /** - * Contains protocols that OkHttp supports - * NPN or - * ALPN selection. + * Protocols that OkHttp implements for NPN and + * ALPN. * *

Protocol vs Scheme

* Despite its name, {@link java.net.URL#getProtocol()} returns the * {@link java.net.URI#getScheme() scheme} (http, https, etc.) of the URL, not - * the protocol (http/1.1, spdy/3.1, etc.). OkHttp uses the word protocol to - * indicate how HTTP messages are framed. + * the protocol (http/1.1, spdy/3.1, etc.). OkHttp uses the word protocol + * to identify how HTTP messages are framed. */ public enum Protocol { HTTP_2("h2-10", true), @@ -45,7 +44,7 @@ public enum Protocol { Util.immutableList(Arrays.asList(HTTP_2, HTTP_11)); /** Identifier string used in NPN or ALPN selection. */ - public final ByteString name; + private final String protocol; /** * When true the protocol is binary framed and derived from SPDY. @@ -54,21 +53,28 @@ public enum Protocol { */ public final boolean spdyVariant; - Protocol(String name, boolean spdyVariant) { - this.name = ByteString.encodeUtf8(name); + Protocol(String protocol, boolean spdyVariant) { + this.protocol = protocol; this.spdyVariant = spdyVariant; } /** - * Returns the protocol matching {@code input} or {@link #HTTP_11} is on - * {@code null}. Throws an {@link IOException} when {@code input} doesn't - * match the {@link #name} of a supported protocol. + * Returns the protocol identified by {@code protocol}. + * @throws IOException if {@code protocol} is unknown. */ - public static Protocol find(ByteString input) throws IOException { - if (input == null) return HTTP_11; - for (Protocol protocol : values()) { - if (protocol.name.equals(input)) return protocol; - } - throw new IOException("Unexpected protocol: " + input.utf8()); + public static Protocol find(String protocol) throws IOException { + // Unroll the loop over values() to save an allocation. + if (protocol.equals(HTTP_11.protocol)) return HTTP_11; + if (protocol.equals(HTTP_2.protocol)) return HTTP_2; + if (protocol.equals(SPDY_3.protocol)) return SPDY_3; + throw new IOException("Unexpected protocol: " + protocol); + } + + /** + * Returns the string used to identify this protocol for ALPN and NPN, like + * "http/1.1", "spdy/3.1" or "h2-10". + */ + @Override public String toString() { + return protocol; } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java b/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java index 231ce3bdf..767b8b14e 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java @@ -37,6 +37,7 @@ import java.util.logging.Logger; import java.util.zip.Deflater; import java.util.zip.DeflaterOutputStream; import javax.net.ssl.SSLSocket; +import okio.Buffer; import okio.ByteString; /** @@ -303,7 +304,7 @@ public class Platform { try { List names = new ArrayList(npnProtocols.size()); for (int i = 0, size = npnProtocols.size(); i < size; i++) { - names.add(npnProtocols.get(i).name.utf8()); + names.add(npnProtocols.get(i).toString()); } Object provider = Proxy.newProxyInstance(Platform.class.getClassLoader(), new Class[] { clientProviderClass, serverProviderClass }, new JettyNpnProvider(names)); @@ -386,24 +387,15 @@ public class Platform { } /** - * Concatenation of 8-bit, length prefixed protocol names. - * + * Returns the concatenation of 8-bit, length prefixed protocol names. * http://tools.ietf.org/html/draft-agl-tls-nextprotoneg-04#page-4 */ static byte[] concatLengthPrefixed(List protocols) { - int size = 0; + Buffer result = new Buffer(); for (Protocol protocol : protocols) { - size += protocol.name.size() + 1; // add a byte for 8-bit length prefix. + result.writeByte(protocol.toString().length()); + result.writeUtf8(protocol.toString()); } - byte[] result = new byte[size]; - int pos = 0; - for (Protocol protocol : protocols) { - int nameSize = protocol.name.size(); - result[pos++] = (byte) nameSize; - // toByteArray allocates an array, but this is only called on new connections. - System.arraycopy(protocol.name.toByteArray(), 0, result, pos, nameSize); - pos += nameSize; - } - return result; + return result.readByteArray(result.size()); } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpConnection.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpConnection.java index 098424b0d..fd198d8b9 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpConnection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpConnection.java @@ -189,7 +189,7 @@ public final class HttpConnection { Response.Builder responseBuilder = new Response.Builder() .statusLine(statusLine) - .header(OkHeaders.SELECTED_PROTOCOL, Protocol.HTTP_11.name.utf8()); + .header(OkHeaders.SELECTED_PROTOCOL, Protocol.HTTP_11.toString()); Headers.Builder headersBuilder = new Headers.Builder(); readHeaders(headersBuilder); 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 5676dbbb5..78eaea20e 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 @@ -46,7 +46,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; import okio.BufferedSink; -import okio.ByteString; import okio.Sink; import static com.squareup.okhttp.internal.Util.getEffectivePort; @@ -568,7 +567,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { } for (String protocol : protocolsString.split(",", -1)) { try { - protocolsList.add(Protocol.find(ByteString.encodeUtf8(protocol))); + protocolsList.add(Protocol.find(protocol)); } catch (IOException e) { throw new IllegalStateException(e); } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java index c4b95c500..a03fe3092 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/SpdyTransport.java @@ -179,7 +179,7 @@ public final class SpdyTransport implements Transport { String version = "HTTP/1.1"; // :version present only in spdy/3. Headers.Builder headersBuilder = new Headers.Builder(); - headersBuilder.set(OkHeaders.SELECTED_PROTOCOL, protocol.name.utf8()); + headersBuilder.set(OkHeaders.SELECTED_PROTOCOL, protocol.toString()); for (int i = 0; i < headerBlock.size(); i++) { ByteString name = headerBlock.get(i).name; String values = headerBlock.get(i).value.utf8();