From 9417fa5c2e84580411e0fa4905b1f109e2d49533 Mon Sep 17 00:00:00 2001 From: jwilson Date: Thu, 2 Jan 2014 23:43:34 -0500 Subject: [PATCH] Another round of header APIs cleanup. This promotes Headers to a public API from the internal package. It moves some of its methods to OkHeaders, which has been renamed from SyntheticHeaders. Making the Headers class public API makes it possible to remove more exotic APIs from Request and Response like the APIs to track headers by index rather than by name. --- .../java/com/squareup/okhttp/Connection.java | 18 ++- .../java/com/squareup/okhttp/Dispatcher.java | 29 ---- .../okhttp/{internal/http => }/Headers.java | 108 ++++---------- .../squareup/okhttp/HttpResponseCache.java | 1 - .../main/java/com/squareup/okhttp/Job.java | 33 ++++- .../java/com/squareup/okhttp/Request.java | 95 ++----------- .../java/com/squareup/okhttp/Response.java | 47 ++---- .../main/java/com/squareup/okhttp/Route.java | 23 ++- .../internal/http/HttpAuthenticator.java | 1 + .../okhttp/internal/http/HttpEngine.java | 73 ++++------ .../okhttp/internal/http/HttpTransport.java | 22 ++- .../internal/http/HttpURLConnectionImpl.java | 5 +- .../okhttp/internal/http/OkHeaders.java | 134 ++++++++++++++++++ .../okhttp/internal/http/SpdyTransport.java | 12 +- .../internal/http/SyntheticHeaders.java | 23 --- .../com/squareup/okhttp/RecordedResponse.java | 5 +- .../okhttp/internal/http/HeadersTest.java | 5 +- .../internal/http/HttpResponseCacheTest.java | 20 ++- 18 files changed, 301 insertions(+), 353 deletions(-) rename okhttp/src/main/java/com/squareup/okhttp/{internal/http => }/Headers.java (70%) create mode 100644 okhttp/src/main/java/com/squareup/okhttp/internal/http/OkHeaders.java delete mode 100644 okhttp/src/main/java/com/squareup/okhttp/internal/http/SyntheticHeaders.java diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index a9d41822a..bac09a213 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java @@ -38,9 +38,9 @@ import static java.net.HttpURLConnection.HTTP_OK; import static java.net.HttpURLConnection.HTTP_PROXY_AUTH; /** - * Holds the sockets and streams of an HTTP, HTTPS, or HTTPS+SPDY connection, - * which may be used for multiple HTTP request/response exchanges. Connections - * may be direct to the origin server or via a proxy. + * The sockets and streams of an HTTP, HTTPS, or HTTPS+SPDY connection. May be + * used for multiple HTTP request/response exchanges. Connections may be direct + * to the origin server or via a proxy. * *

Typically instances of this class are created, connected and exercised * automatically by the HTTP client. Applications may use this class to monitor @@ -53,10 +53,10 @@ import static java.net.HttpURLConnection.HTTP_PROXY_AUTH; * There are tradeoffs when selecting which options to include when negotiating * a secure connection to a remote host. Newer TLS options are quite useful: *

* Unfortunately, older HTTPS servers refuse to connect when such options are * presented. Rather than avoiding these options entirely, this class allows a @@ -223,9 +223,7 @@ public final class Connection implements Closeable { } public void resetIdleStartTime() { - if (spdyConnection != null) { - throw new IllegalStateException("spdyConnection != null"); - } + if (spdyConnection != null) throw new IllegalStateException("spdyConnection != null"); this.idleStartTimeNs = System.nanoTime(); } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java b/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java index c5fa343f5..915bb8757 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Dispatcher.java @@ -15,8 +15,6 @@ */ package com.squareup.okhttp; -import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -55,31 +53,4 @@ final class Dispatcher { List jobs = enqueuedJobs.get(job.tag()); if (jobs != null) jobs.remove(job); } - - static class RealResponseBody extends Response.Body { - private final Response response; - private final InputStream in; - - RealResponseBody(Response response, InputStream in) { - this.response = response; - this.in = in; - } - - @Override public boolean ready() throws IOException { - return true; - } - - @Override public MediaType contentType() { - String contentType = response.getContentType(); - return contentType != null ? MediaType.parse(contentType) : null; - } - - @Override public long contentLength() { - return response.getContentLength(); - } - - @Override public InputStream byteStream() { - return in; - } - } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Headers.java b/okhttp/src/main/java/com/squareup/okhttp/Headers.java similarity index 70% rename from okhttp/src/main/java/com/squareup/okhttp/internal/http/Headers.java rename to okhttp/src/main/java/com/squareup/okhttp/Headers.java index d2a2eefcc..19b349feb 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Headers.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Headers.java @@ -15,27 +15,21 @@ * limitations under the License. */ -package com.squareup.okhttp.internal.http; +package com.squareup.okhttp; import com.squareup.okhttp.internal.Util; -import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.TreeMap; import java.util.TreeSet; /** - * The HTTP status and unparsed header fields of a single HTTP message. Values - * are represented as uninterpreted strings; use {@code Request} and - * {@code Response} for interpreted headers. This class maintains the - * order of the header fields within the HTTP message. + * The header fields of a single HTTP message. Values are uninterpreted strings; + * use {@code Request} and {@code Response} for interpreted headers. This class + * maintains the order of the header fields within the HTTP message. * - *

This class tracks fields line-by-line. A field with multiple comma- + *

This class tracks header values line-by-line. A field with multiple comma- * separated values on the same line will be treated as a field with a single * value by this class. It is the caller's responsibility to detect and split * on commas if their field permits multiple values. This simplifies use of @@ -44,29 +38,22 @@ import java.util.TreeSet; * *

This class trims whitespace from values. It never returns values with * leading or trailing whitespace. + * + *

Instances of this class are immutable. Use {@link Builder} to create + * instances. */ public final class Headers { - private static final Comparator FIELD_NAME_COMPARATOR = new Comparator() { - // @FindBugsSuppressWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") - @Override public int compare(String a, String b) { - if (a == b) { - return 0; - } else if (a == null) { - return -1; - } else if (b == null) { - return 1; - } else { - return String.CASE_INSENSITIVE_ORDER.compare(a, b); - } - } - }; - private final List namesAndValues; private Headers(Builder builder) { this.namesAndValues = Util.immutableList(builder.namesAndValues); } + /** Returns the last value corresponding to the specified field, or null. */ + public String get(String fieldName) { + return get(namesAndValues, fieldName); + } + /** Returns the number of field values. */ public int size() { return namesAndValues.size() / 2; @@ -81,15 +68,6 @@ public final class Headers { return namesAndValues.get(fieldNameIndex); } - /** Returns an immutable case-insensitive set of header names. */ - public Set names() { - TreeSet result = new TreeSet(String.CASE_INSENSITIVE_ORDER); - for (int i = 0; i < size(); i++) { - result.add(name(i)); - } - return Collections.unmodifiableSet(result); - } - /** Returns the value at {@code index} or null if that is out of range. */ public String value(int index) { int valueIndex = index * 2 + 1; @@ -99,9 +77,13 @@ public final class Headers { return namesAndValues.get(valueIndex); } - /** Returns the last value corresponding to the specified field, or null. */ - public String get(String fieldName) { - return get(namesAndValues, fieldName); + /** Returns an immutable case-insensitive set of header names. */ + public Set names() { + TreeSet result = new TreeSet(String.CASE_INSENSITIVE_ORDER); + for (int i = 0; i < size(); i++) { + result.add(name(i)); + } + return Collections.unmodifiableSet(result); } /** Returns an immutable list of the header values for {@code name}. */ @@ -119,6 +101,7 @@ public final class Headers { } /** @param fieldNames a case-insensitive set of HTTP header field names. */ + // TODO: it is very weird to request a case-insensitive set as a parameter. public Headers getAll(Set fieldNames) { Builder result = new Builder(); for (int i = 0; i < namesAndValues.size(); i += 2) { @@ -130,32 +113,6 @@ public final class Headers { return result.build(); } - /** - * Returns an immutable map containing each field to its list of values. - * - * @param valueForNullKey the request line for requests, or the status line - * for responses. If non-null, this value is mapped to the null key. - */ - public Map> toMultimap(String valueForNullKey) { - Map> result = new TreeMap>(FIELD_NAME_COMPARATOR); - for (int i = 0; i < namesAndValues.size(); i += 2) { - String fieldName = namesAndValues.get(i); - String value = namesAndValues.get(i + 1); - - List allValues = new ArrayList(); - List otherValues = result.get(fieldName); - if (otherValues != null) { - allValues.addAll(otherValues); - } - allValues.add(value); - result.put(fieldName, Collections.unmodifiableList(allValues)); - } - if (valueForNullKey != null) { - result.put(null, Collections.unmodifiableList(Collections.singletonList(valueForNullKey))); - } - return Collections.unmodifiableMap(result); - } - public Builder newBuilder() { Builder result = new Builder(); result.namesAndValues.addAll(namesAndValues); @@ -174,21 +131,14 @@ public final class Headers { public static class Builder { private final List namesAndValues = new ArrayList(20); - /** Equivalent to {@code build().get(fieldName)}, but potentially faster. */ - public String get(String fieldName) { - return Headers.get(namesAndValues, fieldName); - } - - /** - * Add an HTTP header line containing a field name, a literal colon, and a - * value. This works around empty header names and header names that start - * with a colon (created by old broken SPDY versions of the response cache). - */ + /** Add an header line containing a field name, a literal colon, and a value. */ public Builder addLine(String line) { int index = line.indexOf(":", 1); if (index != -1) { return addLenient(line.substring(0, index), line.substring(index + 1)); } else if (line.startsWith(":")) { + // Work around empty header names and header names that start with a + // colon (created by old broken SPDY versions of the response cache). return addLenient("", line.substring(1)); // Empty header name. } else { return addLenient("", line); // No header name. @@ -235,13 +185,9 @@ public final class Headers { return this; } - /** Reads headers or trailers into {@code out}. */ - public Builder readHeaders(InputStream in) throws IOException { - // parse the result headers until the first blank line - for (String line; (line = Util.readAsciiLine(in)).length() != 0; ) { - addLine(line); - } - return this; + /** Equivalent to {@code build().get(fieldName)}, but potentially faster. */ + public String get(String fieldName) { + return Headers.get(namesAndValues, fieldName); } public Headers build() { diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java index 7873fa444..aa8b38f9e 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java @@ -20,7 +20,6 @@ import com.squareup.okhttp.internal.Base64; import com.squareup.okhttp.internal.DiskLruCache; import com.squareup.okhttp.internal.StrictLineReader; import com.squareup.okhttp.internal.Util; -import com.squareup.okhttp.internal.http.Headers; import java.io.BufferedWriter; import java.io.ByteArrayInputStream; import java.io.File; diff --git a/okhttp/src/main/java/com/squareup/okhttp/Job.java b/okhttp/src/main/java/com/squareup/okhttp/Job.java index 1ada85fc3..a4f05c14b 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Job.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Job.java @@ -17,7 +17,9 @@ package com.squareup.okhttp; import com.squareup.okhttp.internal.http.HttpAuthenticator; import com.squareup.okhttp.internal.http.HttpEngine; +import com.squareup.okhttp.internal.http.OkHeaders; import java.io.IOException; +import java.io.InputStream; import java.net.ProtocolException; import java.net.Proxy; import java.net.URL; @@ -82,7 +84,7 @@ final class Job implements Runnable { long contentLength = body.contentLength(); if (contentLength != -1) { - requestBuilder.setContentLength(contentLength); + requestBuilder.header("Content-Length", Long.toString(contentLength)); requestBuilder.removeHeader("Transfer-Encoding"); } else { requestBuilder.header("Transfer-Encoding", "chunked"); @@ -107,7 +109,7 @@ final class Job implements Runnable { if (redirect == null) { engine.automaticallyReleaseConnectionToPool(); return response.newBuilder() - .body(new Dispatcher.RealResponseBody(response, engine.getResponseBody())) + .body(new RealResponseBody(response, engine.getResponseBody())) .redirectedBy(redirectedBy) .build(); } @@ -183,4 +185,31 @@ final class Job implements Runnable { && getEffectivePort(a.url()) == getEffectivePort(b.url()) && a.url().getProtocol().equals(b.url().getProtocol()); } + + static class RealResponseBody extends Response.Body { + private final Response response; + private final InputStream in; + + RealResponseBody(Response response, InputStream in) { + this.response = response; + this.in = in; + } + + @Override public boolean ready() throws IOException { + return true; + } + + @Override public MediaType contentType() { + String contentType = response.header("Content-Type"); + return contentType != null ? MediaType.parse(contentType) : null; + } + + @Override public long contentLength() { + return OkHeaders.contentLength(response); + } + + @Override public InputStream byteStream() { + return in; + } + } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Request.java b/okhttp/src/main/java/com/squareup/okhttp/Request.java index 79f1ca821..5866af0f2 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Request.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Request.java @@ -18,7 +18,6 @@ package com.squareup.okhttp; import com.squareup.okhttp.internal.Platform; import com.squareup.okhttp.internal.Util; import com.squareup.okhttp.internal.http.HeaderParser; -import com.squareup.okhttp.internal.http.Headers; import com.squareup.okhttp.internal.http.HttpDate; import java.io.File; import java.io.FileInputStream; @@ -32,8 +31,6 @@ import java.net.URISyntaxException; import java.net.URL; import java.util.Date; import java.util.List; -import java.util.Map; -import java.util.Set; /** * An HTTP request. Instances of this class are immutable if their {@link #body} @@ -81,6 +78,10 @@ public final class Request { return method; } + public Headers headers() { + return headers; + } + public String header(String name) { return headers.get(name); } @@ -89,26 +90,6 @@ public final class Request { return headers.values(name); } - public Set headerNames() { - return headers.names(); - } - - Headers headers() { - return headers; - } - - public int headerCount() { - return headers.size(); - } - - public String headerName(int index) { - return headers.name(index); - } - - public String headerValue(int index) { - return headers.value(index); - } - public Body body() { return body; } @@ -145,20 +126,10 @@ public final class Request { return parsedHeaders().onlyIfCached; } - // TODO: Make non-public. This conflicts with the Body's content length! - public long getContentLength() { - return parsedHeaders().contentLength; - } - public String getUserAgent() { return parsedHeaders().userAgent; } - // TODO: Make non-public. This conflicts with the Body's content type! - public String getContentType() { - return parsedHeaders().contentType; - } - public String getProxyAuthorization() { return parsedHeaders().proxyAuthorization; } @@ -189,9 +160,7 @@ public final class Request { */ private boolean onlyIfCached; - private long contentLength = -1; private String userAgent; - private String contentType; private String proxyAuthorization; public ParsedHeaders(Headers headers) { @@ -220,15 +189,8 @@ public final class Request { if ("no-cache".equalsIgnoreCase(value)) { noCache = true; } - } else if ("Content-Length".equalsIgnoreCase(fieldName)) { - try { - contentLength = Long.parseLong(value); - } catch (NumberFormatException ignored) { - } } else if ("User-Agent".equalsIgnoreCase(fieldName)) { userAgent = value; - } else if ("Content-Type".equalsIgnoreCase(fieldName)) { - contentType = value; } else if ("Proxy-Authorization".equalsIgnoreCase(fieldName)) { proxyAuthorization = value; } @@ -323,7 +285,7 @@ public final class Request { public static class Builder { private URL url; private String method; - private final Headers.Builder headers; + private Headers.Builder headers; private Body body; private Object tag; @@ -377,51 +339,22 @@ public final class Request { return this; } - // TODO: conflict's with the body's content type. - public Builder setContentLength(long contentLength) { - headers.set("Content-Length", Long.toString(contentLength)); + /** Removes all headers on this builder and adds {@code headers}. */ + public Builder headers(Headers headers) { + this.headers = headers.newBuilder(); return this; } - public void setUserAgent(String userAgent) { - headers.set("User-Agent", userAgent); + public Builder setUserAgent(String userAgent) { + return header("User-Agent", userAgent); } - // TODO: conflict's with the body's content type. - public void setContentType(String contentType) { - headers.set("Content-Type", contentType); + public Builder setIfModifiedSince(Date date) { + return header("If-Modified-Since", HttpDate.format(date)); } - public void setIfModifiedSince(Date date) { - headers.set("If-Modified-Since", HttpDate.format(date)); - } - - public void setIfNoneMatch(String ifNoneMatch) { - headers.set("If-None-Match", ifNoneMatch); - } - - public void addCookies(Map> cookieHeaders) { - for (Map.Entry> entry : cookieHeaders.entrySet()) { - String key = entry.getKey(); - if (("Cookie".equalsIgnoreCase(key) || "Cookie2".equalsIgnoreCase(key)) - && !entry.getValue().isEmpty()) { - headers.add(key, buildCookieHeader(entry.getValue())); - } - } - } - - /** - * Send all cookies in one big header, as recommended by - * RFC 6265. - */ - private String buildCookieHeader(List cookies) { - if (cookies.size() == 1) return cookies.get(0); - StringBuilder sb = new StringBuilder(); - for (int i = 0; i < cookies.size(); i++) { - if (i > 0) sb.append("; "); - sb.append(cookies.get(i)); - } - return sb.toString(); + public Builder setIfNoneMatch(String ifNoneMatch) { + return header("If-None-Match", ifNoneMatch); } public Builder get() { diff --git a/okhttp/src/main/java/com/squareup/okhttp/Response.java b/okhttp/src/main/java/com/squareup/okhttp/Response.java index bdbc8a997..8026a78f6 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Response.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Response.java @@ -17,10 +17,9 @@ package com.squareup.okhttp; import com.squareup.okhttp.internal.Util; import com.squareup.okhttp.internal.http.HeaderParser; -import com.squareup.okhttp.internal.http.Headers; import com.squareup.okhttp.internal.http.HttpDate; +import com.squareup.okhttp.internal.http.OkHeaders; import com.squareup.okhttp.internal.http.StatusLine; -import com.squareup.okhttp.internal.http.SyntheticHeaders; import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.IOException; @@ -105,6 +104,10 @@ public final class Response { return handshake; } + public List headers(String name) { + return headers.values(name); + } + public String header(String name) { return header(name, null); } @@ -114,31 +117,10 @@ public final class Response { return result != null ? result : defaultValue; } - public List headers(String name) { - return headers.values(name); - } - - public Set headerNames() { - return headers.names(); - } - - public int headerCount() { - return headers.size(); - } - - public String headerName(int index) { - return headers.name(index); - } - - // TODO: this shouldn't be public? public Headers headers() { return headers; } - public String headerValue(int index) { - return headers.value(index); - } - public Body body() { return body; } @@ -201,16 +183,6 @@ public final class Response { return parsedHeaders().varyFields; } - // TODO: this shouldn't be public. - public long getContentLength() { - return parsedHeaders().contentLength; - } - - // TODO: this shouldn't be public. - public String getContentType() { - return parsedHeaders().contentType; - } - /** * Returns true if a Vary header contains an asterisk. Such responses cannot * be cached. @@ -471,9 +443,9 @@ public final class Response { } } else if ("Content-Type".equalsIgnoreCase(fieldName)) { contentType = value; - } else if (SyntheticHeaders.SENT_MILLIS.equalsIgnoreCase(fieldName)) { + } else if (OkHeaders.SENT_MILLIS.equalsIgnoreCase(fieldName)) { sentRequestMillis = Long.parseLong(value); - } else if (SyntheticHeaders.RECEIVED_MILLIS.equalsIgnoreCase(fieldName)) { + } else if (OkHeaders.RECEIVED_MILLIS.equalsIgnoreCase(fieldName)) { receivedResponseMillis = Long.parseLong(value); } } @@ -589,7 +561,7 @@ public final class Response { return this; } - // TODO: this shouldn't be public? + /** Removes all headers on this builder and adds {@code headers}. */ public Builder headers(Headers headers) { this.headers = headers.newBuilder(); return this; @@ -602,8 +574,7 @@ public final class Response { // TODO: this shouldn't be public. public Builder setResponseSource(ResponseSource responseSource) { - headers.set(SyntheticHeaders.RESPONSE_SOURCE, responseSource + " " + statusLine.code()); - return this; + return header(OkHeaders.RESPONSE_SOURCE, responseSource + " " + statusLine.code()); } public Builder redirectedBy(Response redirectedBy) { diff --git a/okhttp/src/main/java/com/squareup/okhttp/Route.java b/okhttp/src/main/java/com/squareup/okhttp/Route.java index 4b8786d22..08963ad86 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Route.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Route.java @@ -18,7 +18,21 @@ package com.squareup.okhttp; import java.net.InetSocketAddress; import java.net.Proxy; -/** Represents the route used by a connection to reach an endpoint. */ +/** + * The concrete route used by a connection to reach an abstract origin server. + * When creating a connection the client has many options: + *

+ * Each route is a specific selection of these options. + */ public class Route { final Address address; final Proxy proxy; @@ -44,11 +58,8 @@ public class Route { /** * Returns the {@link Proxy} of this route. * - * Warning: This may be different than the proxy returned - * by {@link #getAddress}! That is the proxy that the user asked to be - * connected to; this returns the proxy that they were actually connected - * to. The two may disagree when a proxy selector selects a different proxy - * for a connection. + * Warning: This may disagree with {@link Address#getProxy} + * is null. When the address's proxy is null, the proxy selector will be used. */ public Proxy getProxy() { return proxy; 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 14fe3c8ff..999cff3cd 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 @@ -16,6 +16,7 @@ */ package com.squareup.okhttp.internal.http; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.OkAuthenticator; import com.squareup.okhttp.OkAuthenticator.Challenge; import com.squareup.okhttp.Request; 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 80b27401c..212f0287b 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 @@ -19,7 +19,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Address; import com.squareup.okhttp.Connection; -import com.squareup.okhttp.MediaType; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.OkResponseCache; import com.squareup.okhttp.Request; @@ -35,6 +35,8 @@ import java.net.CacheRequest; import java.net.CookieHandler; import java.net.URL; import java.net.UnknownHostException; +import java.util.List; +import java.util.Map; import java.util.zip.GZIPInputStream; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLSocketFactory; @@ -206,10 +208,7 @@ public class HttpEngine { private Response cacheableResponse() { // Use an unreadable response body when offering the response to the cache. // The cache isn't allowed to consume the response body bytes! - return response.newBuilder() - .body(new UnreadableResponseBody(response.getContentType(), - response.getContentLength())) - .build(); + return response.newBuilder().body(null).build(); } /** Connect to the origin server either directly or via a proxy. */ @@ -400,7 +399,7 @@ public class HttpEngine { // If the Content-Length or Transfer-Encoding headers disagree with the // response code, the response is malformed. For best compatibility, we // honor the headers. - if (response.getContentLength() != -1 + if (OkHeaders.contentLength(response) != -1 || "chunked".equalsIgnoreCase(response.header("Transfer-Encoding"))) { return true; } @@ -435,13 +434,15 @@ public class HttpEngine { result.header("Accept-Encoding", "gzip"); } - if (hasRequestBody() && request.getContentType() == null) { - result.setContentType("application/x-www-form-urlencoded"); + if (hasRequestBody() && request.header("Content-Type") == null) { + result.header("Content-Type", "application/x-www-form-urlencoded"); } CookieHandler cookieHandler = client.getCookieHandler(); if (cookieHandler != null) { - result.addCookies(cookieHandler.get(request.uri(), request.getHeaders().toMultimap(null))); + Map> cookies = cookieHandler.get( + request.uri(), OkHeaders.toMultimap(request.getHeaders(), null)); + OkHeaders.addCookies(result, cookies); } request = result.build(); @@ -468,11 +469,13 @@ public class HttpEngine { if (!responseSource.requiresConnection()) return; if (sentRequestMillis == -1) { - if (request.getContentLength() == -1 + if (OkHeaders.contentLength(request) == -1 && requestBodyOut instanceof RetryableOutputStream) { // We might not learn the Content-Length until the request body has been buffered. - int contentLength = ((RetryableOutputStream) requestBodyOut).contentLength(); - request = request.newBuilder().setContentLength(contentLength).build(); + long contentLength = ((RetryableOutputStream) requestBodyOut).contentLength(); + request = request.newBuilder() + .header("Content-Length", Long.toString(contentLength)) + .build(); } transport.writeRequestHeaders(request); } @@ -489,8 +492,8 @@ public class HttpEngine { response = transport.readResponseHeaders() .request(request) .handshake(connection.getHandshake()) - .header(SyntheticHeaders.SENT_MILLIS, Long.toString(sentRequestMillis)) - .header(SyntheticHeaders.RECEIVED_MILLIS, Long.toString(System.currentTimeMillis())) + .header(OkHeaders.SENT_MILLIS, Long.toString(sentRequestMillis)) + .header(OkHeaders.RECEIVED_MILLIS, Long.toString(System.currentTimeMillis())) .setResponseSource(responseSource) .build(); connection.setHttpMinorVersion(response.httpMinorVersion()); @@ -530,9 +533,10 @@ public class HttpEngine { private static Response combine(Response cached, Response network) throws IOException { Headers.Builder result = new Headers.Builder(); - for (int i = 0; i < cached.headerCount(); i++) { - String fieldName = cached.headerName(i); - String value = cached.headerValue(i); + Headers cachedHeaders = cached.headers(); + for (int i = 0; i < cachedHeaders.size(); i++) { + String fieldName = cachedHeaders.name(i); + String value = cachedHeaders.value(i); if ("Warning".equals(fieldName) && value.startsWith("1")) { continue; // drop 100-level freshness warnings } @@ -541,10 +545,11 @@ public class HttpEngine { } } - for (int i = 0; i < network.headerCount(); i++) { - String fieldName = network.headerName(i); + Headers networkHeaders = network.headers(); + for (int i = 0; i < networkHeaders.size(); i++) { + String fieldName = networkHeaders.name(i); if (isEndToEnd(fieldName)) { - result.add(fieldName, network.headerValue(i)); + result.add(fieldName, networkHeaders.value(i)); } } @@ -580,33 +585,7 @@ public class HttpEngine { public void receiveHeaders(Headers headers) throws IOException { CookieHandler cookieHandler = client.getCookieHandler(); if (cookieHandler != null) { - cookieHandler.put(request.uri(), headers.toMultimap(null)); - } - } - - static class UnreadableResponseBody extends Response.Body { - private final String contentType; - private final long contentLength; - - public UnreadableResponseBody(String contentType, long contentLength) { - this.contentType = contentType; - this.contentLength = contentLength; - } - - @Override public boolean ready() throws IOException { - throw new IllegalStateException("It is an error to read this response body at this time."); - } - - @Override public MediaType contentType() { - return contentType != null ? MediaType.parse(contentType) : null; - } - - @Override public long contentLength() { - return contentLength; - } - - @Override public InputStream byteStream() { - throw new IllegalStateException("It is an error to read this response body at this time."); + cookieHandler.put(request.uri(), OkHeaders.toMultimap(headers, null)); } } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java index ab9261f32..4a166dcc3 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/HttpTransport.java @@ -17,6 +17,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Connection; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; import com.squareup.okhttp.internal.AbstractOutputStream; @@ -62,7 +63,7 @@ public final class HttpTransport implements Transport { } @Override public OutputStream createRequestBody(Request request) throws IOException { - long contentLength = request.getContentLength(); + long contentLength = OkHeaders.contentLength(request); if (httpEngine.bufferRequestBody) { if (contentLength > Integer.MAX_VALUE) { @@ -154,10 +155,10 @@ public final class HttpTransport implements Transport { Response.Builder responseBuilder = new Response.Builder() .statusLine(statusLine) - .header(SyntheticHeaders.SELECTED_TRANSPORT, "http/1.1"); + .header(OkHeaders.SELECTED_TRANSPORT, "http/1.1"); Headers.Builder headersBuilder = new Headers.Builder(); - headersBuilder.readHeaders(in); + OkHeaders.readHeaders(headersBuilder, in); responseBuilder.headers(headersBuilder.build()); if (statusLine.code() != HTTP_CONTINUE) return responseBuilder; @@ -234,9 +235,9 @@ public final class HttpTransport implements Transport { return new ChunkedInputStream(socketIn, cacheRequest, this); } - if (httpEngine.getResponse().getContentLength() != -1) { - return new FixedLengthInputStream(socketIn, cacheRequest, httpEngine, - httpEngine.getResponse().getContentLength()); + long contentLength = OkHeaders.contentLength(httpEngine.getResponse()); + if (contentLength != -1) { + return new FixedLengthInputStream(socketIn, cacheRequest, httpEngine, contentLength); } // Wrap the input stream from the connection (rather than just returning @@ -443,14 +444,12 @@ public final class HttpTransport implements Transport { /** An HTTP body with alternating chunk sizes and chunk bodies. */ private static class ChunkedInputStream extends AbstractHttpInputStream { private static final int NO_CHUNK_YET = -1; - private final HttpTransport transport; private int bytesRemainingInChunk = NO_CHUNK_YET; private boolean hasMoreChunks = true; ChunkedInputStream(InputStream is, CacheRequest cacheRequest, HttpTransport transport) throws IOException { super(is, transport.httpEngine, cacheRequest); - this.transport = transport; } @Override public int read(byte[] buffer, int offset, int count) throws IOException { @@ -493,10 +492,9 @@ public final class HttpTransport implements Transport { } if (bytesRemainingInChunk == 0) { hasMoreChunks = false; - Headers trailers = new Headers.Builder() - .readHeaders(transport.socketIn) - .build(); - httpEngine.receiveHeaders(trailers); + Headers.Builder trailersBuilder = new Headers.Builder(); + OkHeaders.readHeaders(trailersBuilder, in); + httpEngine.receiveHeaders(trailersBuilder.build()); endOfInput(); } } 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 1517ebaa7..8471baf48 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 @@ -18,6 +18,7 @@ package com.squareup.okhttp.internal.http; import com.squareup.okhttp.Connection; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; @@ -165,7 +166,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { @Override public final Map> getHeaderFields() { try { Response response = getResponse().getResponse(); - return response.headers().toMultimap(response.statusLine()); + return OkHeaders.toMultimap(response.headers(), response.statusLine()); } catch (IOException e) { return Collections.emptyMap(); } @@ -180,7 +181,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { // For the request line property assigned to the null key, just use no proxy and HTTP 1.1. Request request = new Request.Builder().url(getURL()).method(method, null).build(); String requestLine = RequestLine.get(request, null, 1); - return requestHeaders.build().toMultimap(requestLine); + return OkHeaders.toMultimap(requestHeaders.build(), requestLine); } @Override public final InputStream getInputStream() throws IOException { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkHeaders.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkHeaders.java new file mode 100644 index 000000000..09193537c --- /dev/null +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/OkHeaders.java @@ -0,0 +1,134 @@ +package com.squareup.okhttp.internal.http; + +import com.squareup.okhttp.Headers; +import com.squareup.okhttp.Request; +import com.squareup.okhttp.Response; +import com.squareup.okhttp.internal.Platform; +import com.squareup.okhttp.internal.Util; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.TreeMap; + +/** Headers and utilities for internal use by OkHttp. */ +public final class OkHeaders { + private static final Comparator FIELD_NAME_COMPARATOR = new Comparator() { + // @FindBugsSuppressWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") + @Override public int compare(String a, String b) { + if (a == b) { + return 0; + } else if (a == null) { + return -1; + } else if (b == null) { + return 1; + } else { + return String.CASE_INSENSITIVE_ORDER.compare(a, b); + } + } + }; + + static final String PREFIX = Platform.get().getPrefix(); + + /** + * Synthetic response header: the local time when the request was sent. + */ + public static final String SENT_MILLIS = PREFIX + "-Sent-Millis"; + + /** + * Synthetic response header: the local time when the response was received. + */ + public static final String RECEIVED_MILLIS = PREFIX + "-Received-Millis"; + + /** + * Synthetic response header: the response source and status code like + * "CONDITIONAL_CACHE 304". + */ + public static final String RESPONSE_SOURCE = PREFIX + "-Response-Source"; + + /** + * Synthetic response header: the selected transport ("spdy/3", "http/1.1", etc). + */ + public static final String SELECTED_TRANSPORT = PREFIX + "-Selected-Transport"; + + private OkHeaders() { + } + + public static long contentLength(Request request) { + return stringToLong(request.header("Content-Length")); + } + + public static long contentLength(Response response) { + return stringToLong(response.header("Content-Length")); + } + + private static long stringToLong(String s) { + if (s == null) return -1; + try { + return Long.parseLong(s); + } catch (NumberFormatException e) { + return -1; + } + } + + /** + * Returns an immutable map containing each field to its list of values. + * + * @param valueForNullKey the request line for requests, or the status line + * for responses. If non-null, this value is mapped to the null key. + */ + public static Map> toMultimap(Headers headers, String valueForNullKey) { + Map> result = new TreeMap>(FIELD_NAME_COMPARATOR); + for (int i = 0; i < headers.size(); i++) { + String fieldName = headers.name(i); + String value = headers.value(i); + + List allValues = new ArrayList(); + List otherValues = result.get(fieldName); + if (otherValues != null) { + allValues.addAll(otherValues); + } + allValues.add(value); + result.put(fieldName, Collections.unmodifiableList(allValues)); + } + if (valueForNullKey != null) { + result.put(null, Collections.unmodifiableList(Collections.singletonList(valueForNullKey))); + } + return Collections.unmodifiableMap(result); + } + + public static void addCookies(Request.Builder builder, Map> cookieHeaders) { + for (Map.Entry> entry : cookieHeaders.entrySet()) { + String key = entry.getKey(); + if (("Cookie".equalsIgnoreCase(key) || "Cookie2".equalsIgnoreCase(key)) + && !entry.getValue().isEmpty()) { + builder.addHeader(key, buildCookieHeader(entry.getValue())); + } + } + } + + /** + * Send all cookies in one big header, as recommended by + * RFC 6265. + */ + private static String buildCookieHeader(List cookies) { + if (cookies.size() == 1) return cookies.get(0); + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < cookies.size(); i++) { + if (i > 0) sb.append("; "); + sb.append(cookies.get(i)); + } + return sb.toString(); + } + + /** Reads headers or trailers into {@code builder}. */ + public static void readHeaders(Headers.Builder builder, InputStream in) throws IOException { + // parse the result headers until the first blank line + for (String line; (line = Util.readAsciiLine(in)).length() != 0; ) { + builder.addLine(line); + } + } +} 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 266512558..3a372e3dc 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 @@ -16,6 +16,7 @@ package com.squareup.okhttp.internal.http; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; import com.squareup.okhttp.internal.spdy.ErrorCode; @@ -78,7 +79,8 @@ public final class SpdyTransport implements Transport { * values, they are concatenated using "\0" as a delimiter. */ public static List writeNameValueBlock(Request request, String version) { - List result = new ArrayList(request.headerCount() + 10); + Headers headers = request.headers(); + List result = new ArrayList(headers.size() + 10); result.add(":method"); result.add(request.method()); result.add(":path"); @@ -91,9 +93,9 @@ public final class SpdyTransport implements Transport { result.add(request.url().getProtocol()); Set names = new LinkedHashSet(); - for (int i = 0; i < request.headerCount(); i++) { - String name = request.headerName(i).toLowerCase(Locale.US); - String value = request.headerValue(i); + for (int i = 0; i < headers.size(); i++) { + String name = headers.name(i).toLowerCase(Locale.US); + String value = headers.value(i); // Drop headers that are forbidden when layering HTTP over SPDY. if (name.equals("connection") @@ -141,7 +143,7 @@ public final class SpdyTransport implements Transport { String version = null; Headers.Builder headersBuilder = new Headers.Builder(); - headersBuilder.set(SyntheticHeaders.SELECTED_TRANSPORT, "spdy/3"); + headersBuilder.set(OkHeaders.SELECTED_TRANSPORT, "spdy/3"); for (int i = 0; i < nameValueBlock.size(); i += 2) { String name = nameValueBlock.get(i); String values = nameValueBlock.get(i + 1); diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SyntheticHeaders.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/SyntheticHeaders.java deleted file mode 100644 index fbb633d65..000000000 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/SyntheticHeaders.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.squareup.okhttp.internal.http; - -import com.squareup.okhttp.internal.Platform; - -/** Headers added to the HTTP response for internal use by OkHttp. */ -public final class SyntheticHeaders { - static final String PREFIX = Platform.get().getPrefix(); - - /** The local time when the request was sent. */ - public static final String SENT_MILLIS = PREFIX + "-Sent-Millis"; - - /** The local time when the response was received. */ - public static final String RECEIVED_MILLIS = PREFIX + "-Received-Millis"; - - /** The response source. */ - public static final String RESPONSE_SOURCE = PREFIX + "-Response-Source"; - - /** The selected transport (spdy/3, http/1.1, etc). */ - public static final String SELECTED_TRANSPORT = PREFIX + "-Selected-Transport"; - - private SyntheticHeaders() { - } -} diff --git a/okhttp/src/test/java/com/squareup/okhttp/RecordedResponse.java b/okhttp/src/test/java/com/squareup/okhttp/RecordedResponse.java index eb6273f0d..192f47905 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/RecordedResponse.java +++ b/okhttp/src/test/java/com/squareup/okhttp/RecordedResponse.java @@ -47,8 +47,9 @@ public class RecordedResponse { public RecordedResponse assertContainsHeaders(String... expectedHeaders) { List actualHeaders = new ArrayList(); - for (int i = 0; i < response.headerCount(); i++) { - actualHeaders.add(response.headerName(i) + ": " + response.headerValue(i)); + Headers headers = response.headers(); + for (int i = 0; i < headers.size(); i++) { + actualHeaders.add(headers.name(i) + ": " + headers.value(i)); } if (!actualHeaders.containsAll(Arrays.asList(expectedHeaders))) { fail("Expected: " + actualHeaders + "\nto contain: " + Arrays.toString(expectedHeaders)); diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java index 3b4dfcf09..ab001e3e8 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HeadersTest.java @@ -15,6 +15,7 @@ */ package com.squareup.okhttp.internal.http; +import com.squareup.okhttp.Headers; import com.squareup.okhttp.Request; import com.squareup.okhttp.Response; import java.io.IOException; @@ -39,8 +40,8 @@ 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("spdy/3", headers.get(SyntheticHeaders.SELECTED_TRANSPORT)); - assertEquals(SyntheticHeaders.SELECTED_TRANSPORT, headers.name(0)); + assertEquals("spdy/3", headers.get(OkHeaders.SELECTED_TRANSPORT)); + assertEquals(OkHeaders.SELECTED_TRANSPORT, headers.name(0)); assertEquals("spdy/3", headers.value(0)); assertEquals("cache-control", headers.name(1)); assertEquals("no-cache, no-store", headers.value(1)); diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java index 946c4084e..91dfcd1f2 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/HttpResponseCacheTest.java @@ -233,14 +233,10 @@ public final class HttpResponseCacheTest { @Override public CacheRequest put(Response response) throws IOException { assertEquals(server.getUrl("/"), response.request().url()); assertEquals(200, response.code()); - assertEquals(body.length(), response.body().contentLength()); - assertEquals("text/plain", response.body().contentType().toString()); + assertNull(response.body()); + assertEquals("5", response.header("Content-Length")); + assertEquals("text/plain", response.header("Content-Type")); assertEquals("ijk", response.header("fgh")); - try { - response.body().byteStream(); // the RI doesn't forbid this, but it should - fail(); - } catch (IllegalStateException expected) { - } cacheCount.incrementAndGet(); return null; } @@ -1696,7 +1692,7 @@ public final class HttpResponseCacheTest { connection.addRequestProperty("Cache-Control", "only-if-cached"); assertEquals("A", readAscii(connection)); - String source = connection.getHeaderField(SyntheticHeaders.RESPONSE_SOURCE); + String source = connection.getHeaderField(OkHeaders.RESPONSE_SOURCE); assertEquals(ResponseSource.CACHE + " 200", source); } @@ -1713,7 +1709,7 @@ public final class HttpResponseCacheTest { HttpURLConnection connection = openConnection(server.getUrl("/")); assertEquals("B", readAscii(connection)); - String source = connection.getHeaderField(SyntheticHeaders.RESPONSE_SOURCE); + String source = connection.getHeaderField(OkHeaders.RESPONSE_SOURCE); assertEquals(ResponseSource.CONDITIONAL_CACHE + " 200", source); } @@ -1728,7 +1724,7 @@ public final class HttpResponseCacheTest { HttpURLConnection connection = openConnection(server.getUrl("/")); assertEquals("A", readAscii(connection)); - String source = connection.getHeaderField(SyntheticHeaders.RESPONSE_SOURCE); + String source = connection.getHeaderField(OkHeaders.RESPONSE_SOURCE); assertEquals(ResponseSource.CONDITIONAL_CACHE + " 304", source); } @@ -1739,7 +1735,7 @@ public final class HttpResponseCacheTest { URLConnection connection = openConnection(server.getUrl("/")); assertEquals("A", readAscii(connection)); - String source = connection.getHeaderField(SyntheticHeaders.RESPONSE_SOURCE); + String source = connection.getHeaderField(OkHeaders.RESPONSE_SOURCE); assertEquals(ResponseSource.NETWORK + " 200", source); } @@ -1956,7 +1952,7 @@ public final class HttpResponseCacheTest { assertEquals(504, connection.getResponseCode()); assertEquals(-1, connection.getErrorStream().read()); assertEquals(ResponseSource.NONE + " 504", - connection.getHeaderField(SyntheticHeaders.RESPONSE_SOURCE)); + connection.getHeaderField(OkHeaders.RESPONSE_SOURCE)); } enum TransferKind {