From 350c43b6fe02401a73f967d9ef322061638b372a Mon Sep 17 00:00:00 2001 From: jwilson Date: Sun, 29 Dec 2013 19:14:06 -0500 Subject: [PATCH] Make RawHeaders, RequestHeaders and ResponseHeaders immutable. This introduces a new, poorly-named class ResponseStrategy that pulls some code out of ResponseHeaders. That was necessary because the old method mutated itself and its parameters in place. Obvious follow-up for this is to combine ResponseHeaders with Response, and RequestHeaders with Response. --- .../java/com/squareup/okhttp/Connection.java | 15 +- .../squareup/okhttp/HttpResponseCache.java | 12 +- .../main/java/com/squareup/okhttp/Job.java | 16 +- .../java/com/squareup/okhttp/Request.java | 8 +- .../java/com/squareup/okhttp/Response.java | 8 +- .../com/squareup/okhttp/TunnelRequest.java | 6 +- .../internal/http/HttpAuthenticator.java | 21 +- .../okhttp/internal/http/HttpEngine.java | 63 +-- .../okhttp/internal/http/HttpTransport.java | 40 +- .../internal/http/HttpURLConnectionImpl.java | 47 ++- .../okhttp/internal/http/RawHeaders.java | 366 +++++++++--------- .../okhttp/internal/http/RequestHeaders.java | 211 +++++----- .../okhttp/internal/http/ResponseHeaders.java | 234 +++-------- .../internal/http/ResponseStrategy.java | 172 ++++++++ .../okhttp/internal/http/SpdyTransport.java | 41 +- .../okhttp/internal/http/Transport.java | 7 + .../com/squareup/okhttp/AsyncApiTest.java | 10 +- .../squareup/okhttp/RecordingReceiver.java | 17 +- .../internal/http/HttpResponseCacheTest.java | 2 +- .../okhttp/internal/http/RawHeadersTest.java | 71 ++-- .../internal/http/URLConnectionTest.java | 14 +- 21 files changed, 738 insertions(+), 643 deletions(-) create mode 100644 okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseStrategy.java diff --git a/okhttp/src/main/java/com/squareup/okhttp/Connection.java b/okhttp/src/main/java/com/squareup/okhttp/Connection.java index a46bb31a8..cf22f04cd 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Connection.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Connection.java @@ -306,22 +306,17 @@ public final class Connection implements Closeable { RawHeaders requestHeaders = tunnelRequest.getRequestHeaders(); while (true) { out.write(requestHeaders.toBytes()); - RawHeaders responseHeaders = RawHeaders.fromBytes(in); + RawHeaders responseHeaders = RawHeaders.readHttpHeaders(in); switch (responseHeaders.getResponseCode()) { case HTTP_OK: return; case HTTP_PROXY_AUTH: - requestHeaders = new RawHeaders(requestHeaders); URL url = new URL("https", tunnelRequest.host, tunnelRequest.port, "/"); - boolean credentialsFound = HttpAuthenticator.processAuthHeader( - route.address.authenticator, HTTP_PROXY_AUTH, responseHeaders, requestHeaders, - route.proxy, url); - if (credentialsFound) { - continue; - } else { - throw new IOException("Failed to authenticate with proxy"); - } + requestHeaders = HttpAuthenticator.processAuthHeader(route.address.authenticator, + HTTP_PROXY_AUTH, responseHeaders, requestHeaders, route.proxy, url); + if (requestHeaders != null) continue; + throw new IOException("Failed to authenticate with proxy"); default: throw new IOException( "Unexpected response code for CONNECT: " + responseHeaders.getResponseCode()); diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java index 646031bdf..91471cade 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java @@ -415,18 +415,20 @@ public final class HttpResponseCache extends ResponseCache implements OkResponse StrictLineReader reader = new StrictLineReader(in, US_ASCII); url = reader.readLine(); requestMethod = reader.readLine(); - varyHeaders = new RawHeaders(); + RawHeaders.Builder varyHeadersBuilder = new RawHeaders.Builder(); int varyRequestHeaderLineCount = reader.readInt(); for (int i = 0; i < varyRequestHeaderLineCount; i++) { - varyHeaders.addLine(reader.readLine()); + varyHeadersBuilder.addLine(reader.readLine()); } + varyHeaders = varyHeadersBuilder.build(); - responseHeaders = new RawHeaders(); - responseHeaders.setStatusLine(reader.readLine()); + RawHeaders.Builder responseHeadersBuilder = new RawHeaders.Builder(); + responseHeadersBuilder.setStatusLine(reader.readLine()); int responseHeaderLineCount = reader.readInt(); for (int i = 0; i < responseHeaderLineCount; i++) { - responseHeaders.addLine(reader.readLine()); + responseHeadersBuilder.addLine(reader.readLine()); } + responseHeaders = responseHeadersBuilder.build(); if (isHttps()) { String blank = reader.readLine(); diff --git a/okhttp/src/main/java/com/squareup/okhttp/Job.java b/okhttp/src/main/java/com/squareup/okhttp/Job.java index cacda366d..ad2768627 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Job.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Job.java @@ -104,17 +104,16 @@ final class Job implements Runnable, Policy { Response redirectedBy = null; while (true) { - HttpEngine engine = newEngine(connection); - Request.Body body = request.body(); if (body != null) { MediaType contentType = body.contentType(); if (contentType == null) throw new IllegalStateException("contentType == null"); - if (engine.getRequestHeaders().getContentType() == null) { - engine.getRequestHeaders().setContentType(contentType.toString()); + if (request.header("Content-Type") == null) { + request = request.newBuilder().header("Content-Type", contentType.toString()).build(); } } + HttpEngine engine = newEngine(connection); engine.sendRequest(); if (body != null) { @@ -183,11 +182,10 @@ final class Job implements Runnable, Policy { } // fall-through case HTTP_UNAUTHORIZED: - RawHeaders successorRequestHeaders = request.rawHeaders(); - boolean credentialsFound = HttpAuthenticator.processAuthHeader(client.getAuthenticator(), - response.code(), response.rawHeaders(), successorRequestHeaders, selectedProxy, - this.request.url()); - return credentialsFound + RawHeaders successorRequestHeaders = HttpAuthenticator.processAuthHeader( + client.getAuthenticator(), response.code(), response.rawHeaders(), request.rawHeaders(), + selectedProxy, this.request.url()); + return successorRequestHeaders != null ? request.newBuilder().rawHeaders(successorRequestHeaders).build() : null; diff --git a/okhttp/src/main/java/com/squareup/okhttp/Request.java b/okhttp/src/main/java/com/squareup/okhttp/Request.java index 449fff9d9..01e7bb61a 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Request.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Request.java @@ -45,7 +45,7 @@ public final class Request { private Request(Builder builder) { this.url = builder.url; this.method = builder.method; - this.headers = new RawHeaders(builder.headers); + this.headers = builder.headers.build(); this.body = builder.body; this.tag = builder.tag != null ? builder.tag : this; } @@ -75,7 +75,7 @@ public final class Request { } RawHeaders rawHeaders() { - return new RawHeaders(headers); + return headers; } public int headerCount() { @@ -192,7 +192,7 @@ public final class Request { public static class Builder { private URL url; private String method = "GET"; - private RawHeaders headers = new RawHeaders(); + private RawHeaders.Builder headers = new RawHeaders.Builder(); private Body body; private Object tag; @@ -239,7 +239,7 @@ public final class Request { // TODO: this shouldn't be public. public Builder rawHeaders(RawHeaders rawHeaders) { - headers = new RawHeaders(rawHeaders); + headers = rawHeaders.newBuilder(); return this; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/Response.java b/okhttp/src/main/java/com/squareup/okhttp/Response.java index 2d24e7122..042c6a3d4 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/Response.java +++ b/okhttp/src/main/java/com/squareup/okhttp/Response.java @@ -49,7 +49,7 @@ public final class Response { this.request = builder.request; this.code = builder.code; this.handshake = builder.handshake; - this.headers = new RawHeaders(builder.headers); + this.headers = builder.headers.build(); this.body = builder.body; this.redirectedBy = builder.redirectedBy; } @@ -109,7 +109,7 @@ public final class Response { // TODO: this shouldn't be public. public RawHeaders rawHeaders() { - return new RawHeaders(headers); + return headers; } public String headerValue(int index) { @@ -254,7 +254,7 @@ public final class Response { private final Request request; private final int code; private Handshake handshake; - private RawHeaders headers = new RawHeaders(); + private RawHeaders.Builder headers = new RawHeaders.Builder(); private Body body; private Response redirectedBy; @@ -290,7 +290,7 @@ public final class Response { // TODO: this shouldn't be public. public Builder rawHeaders(RawHeaders rawHeaders) { - headers = new RawHeaders(rawHeaders); + headers = rawHeaders.newBuilder(); return this; } diff --git a/okhttp/src/main/java/com/squareup/okhttp/TunnelRequest.java b/okhttp/src/main/java/com/squareup/okhttp/TunnelRequest.java index 5260b87c4..261595349 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/TunnelRequest.java +++ b/okhttp/src/main/java/com/squareup/okhttp/TunnelRequest.java @@ -55,8 +55,8 @@ public final class TunnelRequest { * the proxy unencrypted. */ RawHeaders getRequestHeaders() { - RawHeaders result = new RawHeaders(); - result.setRequestLine("CONNECT " + host + ":" + port + " HTTP/1.1"); + RawHeaders.Builder result = new RawHeaders.Builder() + .setRequestLine("CONNECT " + host + ":" + port + " HTTP/1.1"); // Always set Host and User-Agent. result.set("Host", port == getDefaultPort("https") ? host : (host + ":" + port)); @@ -70,6 +70,6 @@ public final class TunnelRequest { // Always set the Proxy-Connection to Keep-Alive for the benefit of // HTTP/1.0 proxies like Squid. result.set("Proxy-Connection", "Keep-Alive"); - return result; + return result.build(); } } 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 1ad36898e..2e385106e 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 @@ -84,12 +84,11 @@ public final class HttpAuthenticator { /** * React to a failed authorization response by looking up new credentials. - * - * @return true if credentials have been added to successorRequestHeaders - * and another request should be attempted. + * Returns headers for a subsequent attempt, or null if no further attempts + * should be made. */ - public static boolean processAuthHeader(OkAuthenticator authenticator, int responseCode, - RawHeaders responseHeaders, RawHeaders successorRequestHeaders, Proxy proxy, URL url) + public static RawHeaders processAuthHeader(OkAuthenticator authenticator, int responseCode, + RawHeaders responseHeaders, RawHeaders requestHeaders, Proxy proxy, URL url) throws IOException { String responseField; String requestField; @@ -103,18 +102,14 @@ public final class HttpAuthenticator { throw new IllegalArgumentException(); // TODO: ProtocolException? } List challenges = parseChallenges(responseHeaders, responseField); - if (challenges.isEmpty()) { - return false; // Could not find a challenge so end the request cycle. - } + if (challenges.isEmpty()) return null; // Could not find a challenge so end the request cycle. Credential credential = responseHeaders.getResponseCode() == HTTP_PROXY_AUTH ? authenticator.authenticateProxy(proxy, url, challenges) : authenticator.authenticate(proxy, url, challenges); - if (credential == null) { - return false; // Could not satisfy the challenge so end the request cycle. - } + if (credential == null) return null; // Couldn't satisfy the challenge so end the request cycle. + // Add authorization credentials, bypassing the already-connected check. - successorRequestHeaders.set(requestField, credential.getHeaderValue()); - return true; + return requestHeaders.newBuilder().set(requestField, credential.getHeaderValue()).build(); } /** 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 568b1aaef..ee931174b 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 @@ -123,7 +123,7 @@ public class HttpEngine { final URI uri; - final RequestHeaders requestHeaders; + RequestHeaders requestHeaders; /** Null until a response is received from the network or the cache. */ ResponseHeaders responseHeaders; @@ -170,7 +170,7 @@ public class HttpEngine { throw new IOException(e.getMessage()); } - this.requestHeaders = new RequestHeaders(uri, new RawHeaders(requestHeaders)); + this.requestHeaders = new RequestHeaders(uri, requestHeaders); if (connection != null && connection.getSocket() instanceof SSLSocket) { handshake = Handshake.get(((SSLSocket) connection.getSocket()).getSession()); @@ -208,8 +208,9 @@ public class HttpEngine { } this.responseSource = ResponseSource.CACHE; - RawHeaders gatewayTimeoutHeaders = new RawHeaders(); - gatewayTimeoutHeaders.setStatusLine("HTTP/1.1 504 Gateway Timeout"); + RawHeaders gatewayTimeoutHeaders = new RawHeaders.Builder() + .setStatusLine("HTTP/1.1 504 Gateway Timeout") + .build(); this.validatingResponse = new Response.Builder(request(), 504) .rawHeaders(gatewayTimeoutHeaders) .body(EMPTY_BODY) @@ -246,7 +247,12 @@ public class HttpEngine { ResponseHeaders cachedResponseHeaders = new ResponseHeaders(uri, candidate.rawHeaders()); long now = System.currentTimeMillis(); - this.responseSource = cachedResponseHeaders.chooseResponseSource(now, requestHeaders); + ResponseStrategy responseStrategy = ResponseStrategy.get( + now, cachedResponseHeaders, requestHeaders); + this.responseSource = responseStrategy.source; + this.requestHeaders = responseStrategy.request; + cachedResponseHeaders = responseStrategy.response; + if (responseSource == ResponseSource.CACHE) { this.validatingResponse = candidate; promoteValidatingResponse(cachedResponseHeaders); @@ -292,6 +298,7 @@ public class HttpEngine { } transport = (Transport) connection.newTransport(this); + requestHeaders = transport.prepareRequestHeaders(requestHeaders); if (hasRequestBody() && requestBodyOut == null) { // Create a request body if we don't have one already. We'll already @@ -332,7 +339,7 @@ public class HttpEngine { connected(connection); if (connection.getRoute().getProxy() != client.getProxy()) { // Update the request line if the proxy changed; it may need a host name. - requestHeaders.getHeaders().setRequestLine(getRequestLine()); + requestHeaders = requestHeaders.newBuilder().setRequestLine(getRequestLine()).build(); } } @@ -428,7 +435,7 @@ public class HttpEngine { if (responseCache == null) return; // Should we cache this response for this request? - if (!responseHeaders.isCacheable(requestHeaders)) { + if (!ResponseStrategy.isCacheable(responseHeaders, requestHeaders)) { responseCache.maybeRemove(request()); return; } @@ -488,8 +495,10 @@ public class HttpEngine { // length of the gzipped response. This isn't terribly useful and is // dangerous because clients can query the content length, but not the // content encoding. - responseHeaders.stripContentEncoding(); - responseHeaders.stripContentLength(); + responseHeaders = responseHeaders.newBuilder() + .stripContentEncoding() + .stripContentLength() + .build(); responseBodyIn = new GZIPInputStream(transferStream); } else { responseBodyIn = transferStream; @@ -531,40 +540,44 @@ public class HttpEngine { * doesn't know what content types the application is interested in. */ private void prepareRawRequestHeaders() throws IOException { - requestHeaders.getHeaders().setRequestLine(getRequestLine()); + RequestHeaders.Builder result = requestHeaders.newBuilder(); + + result.setRequestLine(getRequestLine()); if (requestHeaders.getUserAgent() == null) { - requestHeaders.setUserAgent(getDefaultUserAgent()); + result.setUserAgent(getDefaultUserAgent()); } if (requestHeaders.getHost() == null) { - requestHeaders.setHost(getOriginAddress(policy.getURL())); + result.setHost(getOriginAddress(policy.getURL())); } if ((connection == null || connection.getHttpMinorVersion() != 0) && requestHeaders.getConnection() == null) { - requestHeaders.setConnection("Keep-Alive"); + result.setConnection("Keep-Alive"); } if (requestHeaders.getAcceptEncoding() == null) { transparentGzip = true; - requestHeaders.setAcceptEncoding("gzip"); + result.setAcceptEncoding("gzip"); } if (hasRequestBody() && requestHeaders.getContentType() == null) { - requestHeaders.setContentType("application/x-www-form-urlencoded"); + result.setContentType("application/x-www-form-urlencoded"); } long ifModifiedSince = policy.getIfModifiedSince(); if (ifModifiedSince != 0) { - requestHeaders.setIfModifiedSince(new Date(ifModifiedSince)); + result.setIfModifiedSince(new Date(ifModifiedSince)); } CookieHandler cookieHandler = client.getCookieHandler(); if (cookieHandler != null) { - requestHeaders.addCookies( + result.addCookies( cookieHandler.get(uri, requestHeaders.getHeaders().toMultimap(false))); } + + requestHeaders = result.build(); } /** @@ -645,7 +658,7 @@ public class HttpEngine { */ public final void readResponse() throws IOException { if (hasResponse()) { - responseHeaders.setResponseSource(responseSource); + responseHeaders = responseHeaders.newBuilder().setResponseSource(responseSource).build(); return; } @@ -658,9 +671,11 @@ public class HttpEngine { } if (sentRequestMillis == -1) { - if (requestBodyOut instanceof RetryableOutputStream) { + if (requestHeaders.getContentLength() == -1 + && requestBodyOut instanceof RetryableOutputStream) { + // We might not learn the Content-Length until the request body has been buffered. int contentLength = ((RetryableOutputStream) requestBodyOut).contentLength(); - requestHeaders.setContentLength(contentLength); + requestHeaders = requestHeaders.newBuilder().setContentLength(contentLength).build(); } transport.writeRequestHeaders(); } @@ -674,9 +689,11 @@ public class HttpEngine { transport.flushRequest(); - responseHeaders = transport.readResponseHeaders(); - responseHeaders.setLocalTimestamps(sentRequestMillis, System.currentTimeMillis()); - responseHeaders.setResponseSource(responseSource); + responseHeaders = transport.readResponseHeaders() + .newBuilder() + .setLocalTimestamps(sentRequestMillis, System.currentTimeMillis()) + .setResponseSource(responseSource) + .build(); if (responseSource == ResponseSource.CONDITIONAL_CACHE) { ResponseHeaders validatingResponseHeaders = new ResponseHeaders( 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 c9678308b..a30dfe711 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 @@ -58,21 +58,28 @@ public final class HttpTransport implements Transport { this.socketIn = inputStream; } - @Override public OutputStream createRequestBody() throws IOException { - boolean chunked = httpEngine.requestHeaders.isChunked(); - if (!chunked + public RequestHeaders prepareRequestHeaders(RequestHeaders requestHeaders) { + if (!httpEngine.hasRequestBody()) return requestHeaders; + + if (!requestHeaders.isChunked() && httpEngine.policy.getChunkLength() > 0 && httpEngine.connection.getHttpMinorVersion() != 0) { - httpEngine.requestHeaders.setChunked(); - chunked = true; + return requestHeaders.newBuilder().setChunked().build(); } + long fixedContentLength = httpEngine.policy.getFixedContentLength(); + if (fixedContentLength != -1) { + return requestHeaders.newBuilder().setContentLength(fixedContentLength).build(); + } + + return requestHeaders; + } + + @Override public OutputStream createRequestBody() throws IOException { // Stream a request body of unknown length. - if (chunked) { + if (httpEngine.requestHeaders.isChunked()) { int chunkLength = httpEngine.policy.getChunkLength(); - if (chunkLength == -1) { - chunkLength = DEFAULT_CHUNK_LENGTH; - } + if (chunkLength == -1) chunkLength = DEFAULT_CHUNK_LENGTH; writeRequestHeaders(); return new ChunkedOutputStream(requestOut, chunkLength); } @@ -80,7 +87,6 @@ public final class HttpTransport implements Transport { // Stream a request body of a known length. long fixedContentLength = httpEngine.policy.getFixedContentLength(); if (fixedContentLength != -1) { - httpEngine.requestHeaders.setContentLength(fixedContentLength); writeRequestHeaders(); return new FixedLengthOutputStream(requestOut, fixedContentLength); } @@ -132,13 +138,10 @@ public final class HttpTransport implements Transport { } @Override public ResponseHeaders readResponseHeaders() throws IOException { - RawHeaders rawHeaders = RawHeaders.fromBytes(socketIn); + RawHeaders rawHeaders = RawHeaders.readHttpHeaders(socketIn); httpEngine.connection.setHttpMinorVersion(rawHeaders.getHttpMinorVersion()); httpEngine.receiveHeaders(rawHeaders); - - ResponseHeaders headers = new ResponseHeaders(httpEngine.uri, rawHeaders); - headers.setTransport("http/1.1"); - return headers; + return new ResponseHeaders(httpEngine.uri, rawHeaders); } public boolean makeReusable(boolean streamCanceled, OutputStream requestBodyOut, @@ -469,9 +472,10 @@ public final class HttpTransport implements Transport { } if (bytesRemainingInChunk == 0) { hasMoreChunks = false; - RawHeaders rawResponseHeaders = httpEngine.responseHeaders.getHeaders(); - RawHeaders.readHeaders(transport.socketIn, rawResponseHeaders); - httpEngine.receiveHeaders(rawResponseHeaders); + RawHeaders trailers = new RawHeaders.Builder() + .readHeaders(transport.socketIn) + .build(); + httpEngine.receiveHeaders(trailers); 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 39c099719..867880fc9 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 @@ -70,7 +70,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { final OkHttpClient client; - private final RawHeaders rawRequestHeaders = new RawHeaders(); + private RawHeaders.Builder requestHeaders = new RawHeaders.Builder(); + /** Like the superclass field of the same name, but a long and available on all platforms. */ private long fixedContentLength = -1; private int redirectionCount; @@ -169,7 +170,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { throw new IllegalStateException( "Cannot access request header fields after connection is set"); } - return rawRequestHeaders.toMultimap(false); + return requestHeaders.build().toMultimap(false); } @Override public final InputStream getInputStream() throws IOException { @@ -219,10 +220,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { } @Override public final String getRequestProperty(String field) { - if (field == null) { - return null; - } - return rawRequestHeaders.get(field); + if (field == null) return null; + return requestHeaders.get(field); } @Override public void setConnectTimeout(int timeoutMillis) { @@ -259,19 +258,19 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { throw new ProtocolException(method + " does not support writing"); } } - httpEngine = newHttpEngine(method, rawRequestHeaders, null, null); + httpEngine = newHttpEngine(method, null, null); } catch (IOException e) { httpEngineFailure = e; throw e; } } - private HttpEngine newHttpEngine(String method, RawHeaders requestHeaders, - Connection connection, RetryableOutputStream requestBody) throws IOException { + private HttpEngine newHttpEngine(String method, Connection connection, + RetryableOutputStream requestBody) throws IOException { if (url.getProtocol().equals("http")) { - return new HttpEngine(client, this, method, requestHeaders, connection, requestBody); + return new HttpEngine(client, this, method, requestHeaders.build(), connection, requestBody); } else if (url.getProtocol().equals("https")) { - return new HttpsEngine(client, this, method, requestHeaders, connection, requestBody); + return new HttpsEngine(client, this, method, requestHeaders.build(), connection, requestBody); } else { throw new AssertionError(); } @@ -313,6 +312,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { || responseCode == HTTP_MOVED_TEMP || responseCode == HTTP_SEE_OTHER) { retryMethod = "GET"; + requestHeaders.removeAll("Content-Length"); requestBody = null; } @@ -325,14 +325,8 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { } httpEngine.release(false); - - httpEngine = newHttpEngine(retryMethod, rawRequestHeaders, httpEngine.getConnection(), + httpEngine = newHttpEngine(retryMethod, httpEngine.getConnection(), (RetryableOutputStream) requestBody); - - if (requestBody == null) { - // Drop the Content-Length header when redirected from POST to GET. - httpEngine.getRequestHeaders().removeContentLength(); - } } } @@ -382,7 +376,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { httpEngine.release(true); RetryableOutputStream retryableOutputStream = (RetryableOutputStream) requestBody; - httpEngine = newHttpEngine(method, rawRequestHeaders, null, retryableOutputStream); + httpEngine = newHttpEngine(method, null, retryableOutputStream); httpEngine.routeSelector = routeSelector; // Keep the same routeSelector. return true; } @@ -423,10 +417,13 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { } // fall-through case HTTP_UNAUTHORIZED: - boolean credentialsFound = HttpAuthenticator.processAuthHeader(client.getAuthenticator(), - getResponseCode(), httpEngine.getResponseHeaders().getHeaders(), rawRequestHeaders, - selectedProxy, url); - return credentialsFound ? Retry.SAME_CONNECTION : Retry.NONE; + RawHeaders successorRequestHeaders = HttpAuthenticator.processAuthHeader( + client.getAuthenticator(), getResponseCode(), + httpEngine.getResponseHeaders().getHeaders(), requestHeaders.build(), selectedProxy, + url); + if (successorRequestHeaders == null) return Retry.NONE; + requestHeaders = successorRequestHeaders.newBuilder(); + return Retry.SAME_CONNECTION; case HTTP_MULT_CHOICE: case HTTP_MOVED_PERM: @@ -524,7 +521,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { if ("X-Android-Transports".equals(field)) { setTransports(newValue, false /* append */); } else { - rawRequestHeaders.set(field, newValue); + requestHeaders.set(field, newValue); } } @@ -548,7 +545,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection implements Policy { if ("X-Android-Transports".equals(field)) { setTransports(value, true /* append */); } else { - rawRequestHeaders.add(field, value); + requestHeaders.add(field, value); } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RawHeaders.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RawHeaders.java index 8b4532070..f3b640db8 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RawHeaders.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RawHeaders.java @@ -29,7 +29,6 @@ import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import java.util.TreeMap; import java.util.TreeSet; @@ -66,78 +65,20 @@ public final class RawHeaders { } }; - private final List namesAndValues = new ArrayList(20); - private String requestLine; - private String statusLine; - private int httpMinorVersion = 1; - private int responseCode = -1; - private String responseMessage; + private final List namesAndValues; + private final String requestLine; + private final String statusLine; + private final int httpMinorVersion; + private final int responseCode; + private final String responseMessage; - public RawHeaders() { - } - - public RawHeaders(RawHeaders copyFrom) { - namesAndValues.addAll(copyFrom.namesAndValues); - requestLine = copyFrom.requestLine; - statusLine = copyFrom.statusLine; - httpMinorVersion = copyFrom.httpMinorVersion; - responseCode = copyFrom.responseCode; - responseMessage = copyFrom.responseMessage; - } - - /** Sets the request line (like "GET / HTTP/1.1"). */ - public void setRequestLine(String requestLine) { - requestLine = requestLine.trim(); - this.requestLine = requestLine; - } - - /** Sets the response status line (like "HTTP/1.0 200 OK"). */ - public void setStatusLine(String statusLine) throws IOException { - // H T T P / 1 . 1 2 0 0 T e m p o r a r y R e d i r e c t - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 - if (this.responseMessage != null) { - throw new IllegalStateException("statusLine is already set"); - } - // We allow empty message without leading white space since some servers - // do not send the white space when the message is empty. - boolean hasMessage = statusLine.length() > 13; - if (!statusLine.startsWith("HTTP/1.") - || statusLine.length() < 12 - || statusLine.charAt(8) != ' ' - || (hasMessage && statusLine.charAt(12) != ' ')) { - throw new ProtocolException("Unexpected status line: " + statusLine); - } - int httpMinorVersion = statusLine.charAt(7) - '0'; - if (httpMinorVersion < 0 || httpMinorVersion > 9) { - throw new ProtocolException("Unexpected status line: " + statusLine); - } - int responseCode; - try { - responseCode = Integer.parseInt(statusLine.substring(9, 12)); - } catch (NumberFormatException e) { - throw new ProtocolException("Unexpected status line: " + statusLine); - } - this.responseMessage = hasMessage ? statusLine.substring(13) : ""; - this.responseCode = responseCode; - this.statusLine = statusLine; - this.httpMinorVersion = httpMinorVersion; - } - - /** - * @param method like "GET", "POST", "HEAD", etc. - * @param path like "/foo/bar.html" - * @param version like "HTTP/1.1" - * @param host like "www.android.com:1234" - * @param scheme like "https" - */ - public void addSpdyRequestHeaders(String method, String path, String version, String host, - String scheme) { - // TODO: populate the statusLine for the client's benefit? - add(":method", method); - add(":scheme", scheme); - add(":path", path); - add(":version", version); - add(":host", host); + private RawHeaders(Builder builder) { + this.namesAndValues = Util.immutableList(builder.namesAndValues); + this.requestLine = builder.requestLine; + this.statusLine = builder.statusLine; + this.httpMinorVersion = builder.httpMinorVersion; + this.responseCode = builder.responseCode; + this.responseMessage = builder.responseMessage; } public String getStatusLine() { @@ -162,65 +103,6 @@ public final class RawHeaders { return responseMessage; } - /** - * 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). - */ - public void addLine(String line) { - int index = line.indexOf(":", 1); - if (index != -1) { - addLenient(line.substring(0, index), line.substring(index + 1)); - } else if (line.startsWith(":")) { - addLenient("", line.substring(1)); // Empty header name. - } else { - addLenient("", line); // No header name. - } - } - - /** Add a field with the specified value. */ - public void add(String fieldName, String value) { - if (fieldName == null) throw new IllegalArgumentException("fieldname == null"); - if (value == null) throw new IllegalArgumentException("value == null"); - if (fieldName.length() == 0 || fieldName.indexOf('\0') != -1 || value.indexOf('\0') != -1) { - throw new IllegalArgumentException("Unexpected header: " + fieldName + ": " + value); - } - addLenient(fieldName, value); - } - - /** - * Add a field with the specified value without any validation. Only - * appropriate for headers from the remote peer. - */ - private void addLenient(String fieldName, String value) { - namesAndValues.add(fieldName); - namesAndValues.add(value.trim()); - } - - public void removeAll(String fieldName) { - for (int i = 0; i < namesAndValues.size(); i += 2) { - if (fieldName.equalsIgnoreCase(namesAndValues.get(i))) { - namesAndValues.remove(i); // field name - namesAndValues.remove(i); // value - } - } - } - - public void addAll(String fieldName, List headerFields) { - for (String value : headerFields) { - add(fieldName, value); - } - } - - /** - * Set a field with the specified value. If the field is not found, it is - * added. If the field is found, the existing values are replaced. - */ - public void set(String fieldName, String value) { - removeAll(fieldName); - add(fieldName, value); - } - /** Returns the number of field values. */ public int length() { return namesAndValues.size() / 2; @@ -255,12 +137,7 @@ public final class RawHeaders { /** Returns the last value corresponding to the specified field, or null. */ public String get(String fieldName) { - for (int i = namesAndValues.size() - 2; i >= 0; i -= 2) { - if (fieldName.equalsIgnoreCase(namesAndValues.get(i))) { - return namesAndValues.get(i + 1); - } - } - return null; + return get(namesAndValues, fieldName); } /** Returns an immutable list of the header values for {@code name}. */ @@ -279,14 +156,14 @@ public final class RawHeaders { /** @param fieldNames a case-insensitive set of HTTP header field names. */ public RawHeaders getAll(Set fieldNames) { - RawHeaders result = new RawHeaders(); + Builder result = new Builder(); for (int i = 0; i < namesAndValues.size(); i += 2) { String fieldName = namesAndValues.get(i); if (fieldNames.contains(fieldName)) { result.add(fieldName, namesAndValues.get(i + 1)); } } - return result; + return result.build(); } /** Returns bytes of a request header for sending on an HTTP transport. */ @@ -304,23 +181,15 @@ public final class RawHeaders { } /** Parses bytes of a response header from an HTTP transport. */ - public static RawHeaders fromBytes(InputStream in) throws IOException { - RawHeaders headers; + public static RawHeaders readHttpHeaders(InputStream in) throws IOException { + Builder builder; do { - headers = new RawHeaders(); - headers.setStatusLine(Util.readAsciiLine(in)); - readHeaders(in, headers); - } while (headers.getResponseCode() == HttpEngine.HTTP_CONTINUE); - return headers; - } - - /** Reads headers or trailers into {@code out}. */ - public static void readHeaders(InputStream in, RawHeaders out) throws IOException { - // parse the result headers until the first blank line - String line; - while ((line = Util.readAsciiLine(in)).length() != 0) { - out.addLine(line); - } + builder = new Builder(); + builder.set(ResponseHeaders.SELECTED_TRANSPORT, "http/1.1"); + builder.setStatusLine(Util.readAsciiLine(in)); + builder.readHeaders(in); + } while (builder.responseCode == HttpEngine.HTTP_CONTINUE); + return builder.build(); } /** @@ -349,29 +218,6 @@ public final class RawHeaders { return Collections.unmodifiableMap(result); } - /** - * Creates a new instance from the given map of fields to values. If - * present, the null field's last element will be used to set the status - * line. - */ - public static RawHeaders fromMultimap(Map> map, boolean response) - throws IOException { - if (!response) throw new UnsupportedOperationException(); - RawHeaders result = new RawHeaders(); - for (Entry> entry : map.entrySet()) { - String fieldName = entry.getKey(); - List values = entry.getValue(); - if (fieldName != null) { - for (String value : values) { - result.addLenient(fieldName, value); - } - } else if (!values.isEmpty()) { - result.setStatusLine(values.get(values.size() - 1)); - } - } - return result; - } - /** * Returns a list of alternating names and values. Names are all lower case. * No names are repeated. If any name has multiple values, they are @@ -418,7 +264,8 @@ public final class RawHeaders { } String status = null; String version = null; - RawHeaders result = new RawHeaders(); + Builder builder = new Builder(); + builder.set(ResponseHeaders.SELECTED_TRANSPORT, "spdy/3"); for (int i = 0; i < nameValueBlock.size(); i += 2) { String name = nameValueBlock.get(i); String values = nameValueBlock.get(i + 1); @@ -433,15 +280,170 @@ public final class RawHeaders { } else if (":version".equals(name)) { version = value; } else { - result.namesAndValues.add(name); - result.namesAndValues.add(value); + builder.namesAndValues.add(name); + builder.namesAndValues.add(value); } start = end + 1; } } if (status == null) throw new ProtocolException("Expected ':status' header not present"); if (version == null) throw new ProtocolException("Expected ':version' header not present"); - result.setStatusLine(version + " " + status); + builder.setStatusLine(version + " " + status); + return builder.build(); + } + + public Builder newBuilder() { + Builder result = new Builder(); + result.namesAndValues.addAll(namesAndValues); + result.requestLine = requestLine; + result.statusLine = statusLine; + result.httpMinorVersion = httpMinorVersion; + result.responseCode = responseCode; + result.responseMessage = responseMessage; return result; } + + private static String get(List namesAndValues, String fieldName) { + for (int i = namesAndValues.size() - 2; i >= 0; i -= 2) { + if (fieldName.equalsIgnoreCase(namesAndValues.get(i))) { + return namesAndValues.get(i + 1); + } + } + return null; + } + + public static class Builder { + private final List namesAndValues = new ArrayList(20); + private String requestLine; + private String statusLine; + private int httpMinorVersion = 1; + private int responseCode = -1; + private String responseMessage; + private String transport; + + /** Sets the request line (like "GET / HTTP/1.1"). */ + public Builder setRequestLine(String requestLine) { + this.requestLine = requestLine.trim(); + return this; + } + + /** Equivalent to {@code build().get(fieldName)}, but potentially faster. */ + public String get(String fieldName) { + return RawHeaders.get(namesAndValues, fieldName); + } + + /** Equivalent to {@code build().getResponseCode()}, but potentially faster. */ + public int getResponseCode() { + return responseCode; + } + + /** Sets the response status line (like "HTTP/1.0 200 OK"). */ + public Builder setStatusLine(String statusLine) throws IOException { + // H T T P / 1 . 1 2 0 0 T e m p o r a r y R e d i r e c t + // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 + if (this.responseMessage != null) { + throw new IllegalStateException("statusLine is already set"); + } + // We allow empty message without leading white space since some servers + // do not send the white space when the message is empty. + boolean hasMessage = statusLine.length() > 13; + if (!statusLine.startsWith("HTTP/1.") + || statusLine.length() < 12 + || statusLine.charAt(8) != ' ' + || (hasMessage && statusLine.charAt(12) != ' ')) { + throw new ProtocolException("Unexpected status line: " + statusLine); + } + int httpMinorVersion = statusLine.charAt(7) - '0'; + if (httpMinorVersion < 0 || httpMinorVersion > 9) { + throw new ProtocolException("Unexpected status line: " + statusLine); + } + int responseCode; + try { + responseCode = Integer.parseInt(statusLine.substring(9, 12)); + } catch (NumberFormatException e) { + throw new ProtocolException("Unexpected status line: " + statusLine); + } + this.responseMessage = hasMessage ? statusLine.substring(13) : ""; + this.responseCode = responseCode; + this.statusLine = statusLine; + this.httpMinorVersion = httpMinorVersion; + return this; + } + + /** + * 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). + */ + 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(":")) { + return addLenient("", line.substring(1)); // Empty header name. + } else { + return addLenient("", line); // No header name. + } + } + + /** Add a field with the specified value. */ + public Builder add(String fieldName, String value) { + if (fieldName == null) throw new IllegalArgumentException("fieldname == null"); + if (value == null) throw new IllegalArgumentException("value == null"); + if (fieldName.length() == 0 || fieldName.indexOf('\0') != -1 || value.indexOf('\0') != -1) { + throw new IllegalArgumentException("Unexpected header: " + fieldName + ": " + value); + } + return addLenient(fieldName, value); + } + + /** + * Add a field with the specified value without any validation. Only + * appropriate for headers from the remote peer. + */ + private Builder addLenient(String fieldName, String value) { + namesAndValues.add(fieldName); + namesAndValues.add(value.trim()); + return this; + } + + public Builder removeAll(String fieldName) { + for (int i = 0; i < namesAndValues.size(); i += 2) { + if (fieldName.equalsIgnoreCase(namesAndValues.get(i))) { + namesAndValues.remove(i); // field name + namesAndValues.remove(i); // value + } + } + return this; + } + + public Builder addAll(String fieldName, List headerFields) { + for (String value : headerFields) { + add(fieldName, value); + } + return this; + } + + /** + * Set a field with the specified value. If the field is not found, it is + * added. If the field is found, the existing values are replaced. + */ + public Builder set(String fieldName, String value) { + removeAll(fieldName); + add(fieldName, value); + 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; + } + + public RawHeaders build() { + return new RawHeaders(this); + } + } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RequestHeaders.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RequestHeaders.java index 71c3cd0f2..701921468 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/RequestHeaders.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/RequestHeaders.java @@ -197,91 +197,6 @@ public final class RequestHeaders { return proxyAuthorization; } - public void setChunked() { - if (this.transferEncoding != null) { - headers.removeAll("Transfer-Encoding"); - } - headers.add("Transfer-Encoding", "chunked"); - this.transferEncoding = "chunked"; - } - - public void setContentLength(long contentLength) { - if (this.contentLength != -1) { - headers.removeAll("Content-Length"); - } - headers.add("Content-Length", Long.toString(contentLength)); - this.contentLength = contentLength; - } - - /** - * Remove the Content-Length headers. Call this when dropping the body on a - * request or response, such as when a redirect changes the method from POST - * to GET. - */ - public void removeContentLength() { - if (contentLength != -1) { - headers.removeAll("Content-Length"); - contentLength = -1; - } - } - - public void setUserAgent(String userAgent) { - if (this.userAgent != null) { - headers.removeAll("User-Agent"); - } - headers.add("User-Agent", userAgent); - this.userAgent = userAgent; - } - - public void setHost(String host) { - if (this.host != null) { - headers.removeAll("Host"); - } - headers.add("Host", host); - this.host = host; - } - - public void setConnection(String connection) { - if (this.connection != null) { - headers.removeAll("Connection"); - } - headers.add("Connection", connection); - this.connection = connection; - } - - public void setAcceptEncoding(String acceptEncoding) { - if (this.acceptEncoding != null) { - headers.removeAll("Accept-Encoding"); - } - headers.add("Accept-Encoding", acceptEncoding); - this.acceptEncoding = acceptEncoding; - } - - public void setContentType(String contentType) { - if (this.contentType != null) { - headers.removeAll("Content-Type"); - } - headers.add("Content-Type", contentType); - this.contentType = contentType; - } - - public void setIfModifiedSince(Date date) { - if (ifModifiedSince != null) { - headers.removeAll("If-Modified-Since"); - } - String formattedDate = HttpDate.format(date); - headers.add("If-Modified-Since", formattedDate); - ifModifiedSince = formattedDate; - } - - public void setIfNoneMatch(String ifNoneMatch) { - if (this.ifNoneMatch != null) { - headers.removeAll("If-None-Match"); - } - headers.add("If-None-Match", ifNoneMatch); - this.ifNoneMatch = ifNoneMatch; - } - /** * Returns true if the request contains conditions that save the server from * sending a response that the client has locally. When the caller adds @@ -291,27 +206,115 @@ public final class RequestHeaders { return ifModifiedSince != null || ifNoneMatch != null; } - public void addCookies(Map> allCookieHeaders) { - for (Map.Entry> entry : allCookieHeaders.entrySet()) { - String key = entry.getKey(); - if (("Cookie".equalsIgnoreCase(key) || "Cookie2".equalsIgnoreCase(key)) - && !entry.getValue().isEmpty()) { - headers.add(key, buildCookieHeader(entry.getValue())); - } - } + public Builder newBuilder() { + return new Builder(uri, headers); } - /** - * 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)); + static class Builder { + private final URI uri; + private final RawHeaders.Builder headers; + + public Builder(URI uri, RawHeaders headers) { + this.uri = uri; + this.headers = headers.newBuilder(); + } + + public Builder setRequestLine(String requestLine) { + headers.setRequestLine(requestLine); + return this; + } + + public Builder setChunked() { + headers.set("Transfer-Encoding", "chunked"); + return this; + } + + public Builder setContentLength(long contentLength) { + headers.set("Content-Length", Long.toString(contentLength)); + return this; + } + + /** + * Remove the Content-Length headers. Call this when dropping the body on a + * request or response, such as when a redirect changes the method from POST + * to GET. + */ + public void removeContentLength() { + headers.removeAll("Content-Length"); + } + + public void setUserAgent(String userAgent) { + headers.set("User-Agent", userAgent); + } + + public void setHost(String host) { + headers.set("Host", host); + } + + public void setConnection(String connection) { + headers.set("Connection", connection); + } + + public void setAcceptEncoding(String acceptEncoding) { + headers.set("Accept-Encoding", acceptEncoding); + } + + public void setContentType(String contentType) { + headers.set("Content-Type", contentType); + } + + 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> allCookieHeaders) { + for (Map.Entry> entry : allCookieHeaders.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(); + } + + /** + * @param method like "GET", "POST", "HEAD", etc. + * @param path like "/foo/bar.html" + * @param version like "HTTP/1.1" + * @param host like "www.android.com:1234" + * @param scheme like "https" + */ + public Builder addSpdyRequestHeaders( + String method, String path, String version, String host, String scheme) { + // TODO: populate the statusLine for the client's benefit? + headers.add(":method", method); + headers.add(":scheme", scheme); + headers.add(":path", path); + headers.add(":version", version); + headers.add(":host", host); + return this; + } + + public RequestHeaders build() { + return new RequestHeaders(uri, headers.build()); } - return sb.toString(); } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseHeaders.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseHeaders.java index 31e7869d0..d347a6f6f 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseHeaders.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseHeaders.java @@ -26,7 +26,6 @@ import java.util.Collections; import java.util.Date; import java.util.Set; import java.util.TreeSet; -import java.util.concurrent.TimeUnit; import static com.squareup.okhttp.internal.Util.equal; @@ -45,32 +44,32 @@ public final class ResponseHeaders { /** HTTP synthetic header with the selected transport (spdy/3, http/1.1, etc). */ static final String SELECTED_TRANSPORT = Platform.get().getPrefix() + "-Selected-Transport"; - private final URI uri; - private final RawHeaders headers; + final URI uri; + final RawHeaders headers; /** The server's time when this response was served, if known. */ - private Date servedDate; + Date servedDate; /** The last modified date of the response, if known. */ - private Date lastModified; + Date lastModified; /** * The expiration date of the response, if known. If both this field and the * max age are set, the max age is preferred. */ - private Date expires; + Date expires; /** * Extension header set by HttpURLConnectionImpl specifying the timestamp * when the HTTP request was first initiated. */ - private long sentRequestMillis; + long sentRequestMillis; /** * Extension header set by HttpURLConnectionImpl specifying the timestamp * when the HTTP response was first received. */ - private long receivedResponseMillis; + long receivedResponseMillis; /** * In the response, this field's name "no-cache" is misleading. It doesn't @@ -78,23 +77,23 @@ public final class ResponseHeaders { * the response with the origin server before returning it. We can do this * with a conditional get. */ - private boolean noCache; + boolean noCache; /** If true, this response should not be cached. */ - private boolean noStore; + boolean noStore; /** * The duration past the response's served date that it can be served * without validation. */ - private int maxAgeSeconds = -1; + int maxAgeSeconds = -1; /** * The "s-maxage" directive is the max age for shared caches. Not to be * confused with "max-age" for non-shared caches, As in Firefox and Chrome, * this directive is not honored by this cache. */ - private int sMaxAgeSeconds = -1; + int sMaxAgeSeconds = -1; /** * This request header field's name "only-if-cached" is misleading. It @@ -103,10 +102,10 @@ public final class ResponseHeaders { * Cached responses that would require validation (ie. conditional gets) are * not permitted if this header is set. */ - private boolean isPublic; - private boolean mustRevalidate; - private String etag; - private int ageSeconds = -1; + boolean isPublic; + boolean mustRevalidate; + String etag; + int ageSeconds = -1; /** Case-insensitive set of field names. */ private Set varyFields = Collections.emptySet(); @@ -191,16 +190,6 @@ public final class ResponseHeaders { return "gzip".equalsIgnoreCase(contentEncoding); } - public void stripContentEncoding() { - contentEncoding = null; - headers.removeAll("Content-Encoding"); - } - - public void stripContentLength() { - contentLength = -1; - headers.removeAll("Content-Length"); - } - public boolean isChunked() { return "chunked".equalsIgnoreCase(transferEncoding); } @@ -277,97 +266,6 @@ public final class ResponseHeaders { return connection; } - public void setLocalTimestamps(long sentRequestMillis, long receivedResponseMillis) { - this.sentRequestMillis = sentRequestMillis; - headers.add(SENT_MILLIS, Long.toString(sentRequestMillis)); - this.receivedResponseMillis = receivedResponseMillis; - headers.add(RECEIVED_MILLIS, Long.toString(receivedResponseMillis)); - } - - public void setResponseSource(ResponseSource responseSource) { - headers.set(RESPONSE_SOURCE, responseSource.toString() + " " + headers.getResponseCode()); - } - - public void setTransport(String transport) { - headers.set(SELECTED_TRANSPORT, transport); - } - - /** - * Returns the current age of the response, in milliseconds. The calculation - * is specified by RFC 2616, 13.2.3 Age Calculations. - */ - private long computeAge(long nowMillis) { - long apparentReceivedAge = - servedDate != null ? Math.max(0, receivedResponseMillis - servedDate.getTime()) : 0; - long receivedAge = - ageSeconds != -1 ? Math.max(apparentReceivedAge, TimeUnit.SECONDS.toMillis(ageSeconds)) - : apparentReceivedAge; - long responseDuration = receivedResponseMillis - sentRequestMillis; - long residentDuration = nowMillis - receivedResponseMillis; - return receivedAge + responseDuration + residentDuration; - } - - /** - * Returns the number of milliseconds that the response was fresh for, - * starting from the served date. - */ - private long computeFreshnessLifetime() { - if (maxAgeSeconds != -1) { - return TimeUnit.SECONDS.toMillis(maxAgeSeconds); - } else if (expires != null) { - long servedMillis = servedDate != null ? servedDate.getTime() : receivedResponseMillis; - long delta = expires.getTime() - servedMillis; - return delta > 0 ? delta : 0; - } else if (lastModified != null && uri.getRawQuery() == null) { - // As recommended by the HTTP RFC and implemented in Firefox, the - // max age of a document should be defaulted to 10% of the - // document's age at the time it was served. Default expiration - // dates aren't used for URIs containing a query. - long servedMillis = servedDate != null ? servedDate.getTime() : sentRequestMillis; - long delta = servedMillis - lastModified.getTime(); - return delta > 0 ? (delta / 10) : 0; - } - return 0; - } - - /** - * Returns true if computeFreshnessLifetime used a heuristic. If we used a - * heuristic to serve a cached response older than 24 hours, we are required - * to attach a warning. - */ - private boolean isFreshnessLifetimeHeuristic() { - return maxAgeSeconds == -1 && expires == null; - } - - /** - * Returns true if this response can be stored to later serve another - * request. - */ - public boolean isCacheable(RequestHeaders request) { - // Always go to network for uncacheable response codes (RFC 2616, 13.4), - // This implementation doesn't support caching partial content. - int responseCode = headers.getResponseCode(); - if (responseCode != HttpURLConnection.HTTP_OK - && responseCode != HttpURLConnection.HTTP_NOT_AUTHORITATIVE - && responseCode != HttpURLConnection.HTTP_MULT_CHOICE - && responseCode != HttpURLConnection.HTTP_MOVED_PERM - && responseCode != HttpURLConnection.HTTP_GONE) { - return false; - } - - // Responses to authorized requests aren't cacheable unless they include - // a 'public', 'must-revalidate' or 's-maxage' directive. - if (request.hasAuthorization() && !isPublic && !mustRevalidate && sMaxAgeSeconds == -1) { - return false; - } - - if (noStore) { - return false; - } - - return true; - } - /** * Returns true if a Vary header contains an asterisk. Such responses cannot * be cached. @@ -387,60 +285,6 @@ public final class ResponseHeaders { return true; } - /** Returns the source to satisfy {@code request} given this cached response. */ - public ResponseSource chooseResponseSource(long nowMillis, RequestHeaders request) { - // If this response shouldn't have been stored, it should never be used - // as a response source. This check should be redundant as long as the - // persistence store is well-behaved and the rules are constant. - if (!isCacheable(request)) { - return ResponseSource.NETWORK; - } - - if (request.isNoCache() || request.hasConditions()) { - return ResponseSource.NETWORK; - } - - long ageMillis = computeAge(nowMillis); - long freshMillis = computeFreshnessLifetime(); - - if (request.getMaxAgeSeconds() != -1) { - freshMillis = Math.min(freshMillis, TimeUnit.SECONDS.toMillis(request.getMaxAgeSeconds())); - } - - long minFreshMillis = 0; - if (request.getMinFreshSeconds() != -1) { - minFreshMillis = TimeUnit.SECONDS.toMillis(request.getMinFreshSeconds()); - } - - long maxStaleMillis = 0; - if (!mustRevalidate && request.getMaxStaleSeconds() != -1) { - maxStaleMillis = TimeUnit.SECONDS.toMillis(request.getMaxStaleSeconds()); - } - - if (!noCache && ageMillis + minFreshMillis < freshMillis + maxStaleMillis) { - if (ageMillis + minFreshMillis >= freshMillis) { - headers.add("Warning", "110 HttpURLConnection \"Response is stale\""); - } - long oneDayMillis = 24 * 60 * 60 * 1000L; - if (ageMillis > oneDayMillis && isFreshnessLifetimeHeuristic()) { - headers.add("Warning", "113 HttpURLConnection \"Heuristic expiration\""); - } - return ResponseSource.CACHE; - } - - if (lastModified != null) { - request.setIfModifiedSince(lastModified); - } else if (servedDate != null) { - request.setIfModifiedSince(servedDate); - } - - if (etag != null) { - request.setIfNoneMatch(etag); - } - - return request.hasConditions() ? ResponseSource.CONDITIONAL_CACHE : ResponseSource.NETWORK; - } - /** * Returns true if this cached response should be used; false if the * network response should be used. @@ -467,7 +311,7 @@ public final class ResponseHeaders { * 13.5.3. */ public ResponseHeaders combine(ResponseHeaders network) throws IOException { - RawHeaders result = new RawHeaders(); + RawHeaders.Builder result = new RawHeaders.Builder(); result.setStatusLine(headers.getStatusLine()); for (int i = 0; i < headers.length(); i++) { @@ -488,7 +332,7 @@ public final class ResponseHeaders { } } - return new ResponseHeaders(uri, result); + return new ResponseHeaders(uri, result.build()); } /** @@ -505,4 +349,48 @@ public final class ResponseHeaders { && !"Transfer-Encoding".equalsIgnoreCase(fieldName) && !"Upgrade".equalsIgnoreCase(fieldName); } + + public Builder newBuilder() { + return new Builder(uri, headers); + } + + static class Builder { + private final URI uri; + private final RawHeaders.Builder headers; + + public Builder(URI uri, RawHeaders headers) { + this.uri = uri; + this.headers = headers.newBuilder(); + } + + public Builder stripContentEncoding() { + headers.removeAll("Content-Encoding"); + return this; + } + + public Builder stripContentLength() { + headers.removeAll("Content-Length"); + return this; + } + + public Builder setLocalTimestamps(long sentRequestMillis, long receivedResponseMillis) { + headers.set(SENT_MILLIS, Long.toString(sentRequestMillis)); + headers.set(RECEIVED_MILLIS, Long.toString(receivedResponseMillis)); + return this; + } + + public Builder setResponseSource(ResponseSource responseSource) { + headers.set(RESPONSE_SOURCE, responseSource.toString() + " " + headers.getResponseCode()); + return this; + } + + public Builder addWarning(String message) { + headers.add("Warning", message); + return this; + } + + public ResponseHeaders build() { + return new ResponseHeaders(uri, headers.build()); + } + } } diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseStrategy.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseStrategy.java new file mode 100644 index 000000000..e7c420229 --- /dev/null +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/ResponseStrategy.java @@ -0,0 +1,172 @@ +package com.squareup.okhttp.internal.http; + +import com.squareup.okhttp.ResponseSource; +import java.net.HttpURLConnection; +import java.util.concurrent.TimeUnit; + +/** + * Given a request and cached response, this figures out the next action. It + * may also update the request to add conditions, or the response to add + * warnings. + */ +public final class ResponseStrategy { + public final RequestHeaders request; + public final ResponseHeaders response; + public final ResponseSource source; + + private ResponseStrategy( + RequestHeaders request, ResponseHeaders response, ResponseSource source) { + this.request = request; + this.response = response; + this.source = source; + } + + /** + * Returns the current age of the response, in milliseconds. The calculation + * is specified by RFC 2616, 13.2.3 Age Calculations. + */ + private static long computeAge(ResponseHeaders response, long nowMillis) { + long apparentReceivedAge = response.servedDate != null + ? Math.max(0, response.receivedResponseMillis - response.servedDate.getTime()) + : 0; + long receivedAge = response.ageSeconds != -1 + ? Math.max(apparentReceivedAge, TimeUnit.SECONDS.toMillis(response.ageSeconds)) + : apparentReceivedAge; + long responseDuration = response.receivedResponseMillis - response.sentRequestMillis; + long residentDuration = nowMillis - response.receivedResponseMillis; + return receivedAge + responseDuration + residentDuration; + } + + /** + * Returns the number of milliseconds that the response was fresh for, + * starting from the served date. + */ + private static long computeFreshnessLifetime(ResponseHeaders response) { + if (response.maxAgeSeconds != -1) { + return TimeUnit.SECONDS.toMillis(response.maxAgeSeconds); + } else if (response.expires != null) { + long servedMillis = response.servedDate != null + ? response.servedDate.getTime() + : response.receivedResponseMillis; + long delta = response.expires.getTime() - servedMillis; + return delta > 0 ? delta : 0; + } else if (response.lastModified != null && response.uri.getRawQuery() == null) { + // As recommended by the HTTP RFC and implemented in Firefox, the + // max age of a document should be defaulted to 10% of the + // document's age at the time it was served. Default expiration + // dates aren't used for URIs containing a query. + long servedMillis = response.servedDate != null + ? response.servedDate.getTime() + : response.sentRequestMillis; + long delta = servedMillis - response.lastModified.getTime(); + return delta > 0 ? (delta / 10) : 0; + } + return 0; + } + + /** + * Returns true if computeFreshnessLifetime used a heuristic. If we used a + * heuristic to serve a cached response older than 24 hours, we are required + * to attach a warning. + */ + private static boolean isFreshnessLifetimeHeuristic(ResponseHeaders response) { + return response.maxAgeSeconds == -1 && response.expires == null; + } + + /** + * Returns true if this response can be stored to later serve another + * request. + */ + public static boolean isCacheable(ResponseHeaders response, RequestHeaders request) { + // Always go to network for uncacheable response codes (RFC 2616, 13.4), + // This implementation doesn't support caching partial content. + int responseCode = response.headers.getResponseCode(); + if (responseCode != HttpURLConnection.HTTP_OK + && responseCode != HttpURLConnection.HTTP_NOT_AUTHORITATIVE + && responseCode != HttpURLConnection.HTTP_MULT_CHOICE + && responseCode != HttpURLConnection.HTTP_MOVED_PERM + && responseCode != HttpURLConnection.HTTP_GONE) { + return false; + } + + // Responses to authorized requests aren't cacheable unless they include + // a 'public', 'must-revalidate' or 's-maxage' directive. + if (request.hasAuthorization() + && !response.isPublic + && !response.mustRevalidate + && response.sMaxAgeSeconds == -1) { + return false; + } + + if (response.noStore) { + return false; + } + + return true; + } + + /** + * Returns a strategy to satisfy {@code request} using the a cached response + * {@code response}. + */ + public static ResponseStrategy get( + long nowMillis, ResponseHeaders response, RequestHeaders request) { + // If this response shouldn't have been stored, it should never be used + // as a response source. This check should be redundant as long as the + // persistence store is well-behaved and the rules are constant. + if (!isCacheable(response, request)) { + return new ResponseStrategy(request, response, ResponseSource.NETWORK); + } + + if (request.isNoCache() || request.hasConditions()) { + return new ResponseStrategy(request, response, ResponseSource.NETWORK); + } + + long ageMillis = computeAge(response, nowMillis); + long freshMillis = computeFreshnessLifetime(response); + + if (request.getMaxAgeSeconds() != -1) { + freshMillis = Math.min(freshMillis, TimeUnit.SECONDS.toMillis(request.getMaxAgeSeconds())); + } + + long minFreshMillis = 0; + if (request.getMinFreshSeconds() != -1) { + minFreshMillis = TimeUnit.SECONDS.toMillis(request.getMinFreshSeconds()); + } + + long maxStaleMillis = 0; + if (!response.mustRevalidate && request.getMaxStaleSeconds() != -1) { + maxStaleMillis = TimeUnit.SECONDS.toMillis(request.getMaxStaleSeconds()); + } + + if (!response.noCache && ageMillis + minFreshMillis < freshMillis + maxStaleMillis) { + ResponseHeaders.Builder builder = response.newBuilder(); + if (ageMillis + minFreshMillis >= freshMillis) { + builder.addWarning("110 HttpURLConnection \"Response is stale\""); + } + long oneDayMillis = 24 * 60 * 60 * 1000L; + if (ageMillis > oneDayMillis && isFreshnessLifetimeHeuristic(response)) { + builder.addWarning("113 HttpURLConnection \"Heuristic expiration\""); + } + return new ResponseStrategy(request, builder.build(), ResponseSource.CACHE); + } + + RequestHeaders.Builder conditionalRequestBuilder = request.newBuilder(); + + if (response.lastModified != null) { + conditionalRequestBuilder.setIfModifiedSince(response.lastModified); + } else if (response.servedDate != null) { + conditionalRequestBuilder.setIfModifiedSince(response.servedDate); + } + + if (response.etag != null) { + conditionalRequestBuilder.setIfNoneMatch(response.etag); + } + + RequestHeaders conditionalRequest = conditionalRequestBuilder.build(); + ResponseSource responseSource = conditionalRequest.hasConditions() + ? ResponseSource.CONDITIONAL_CACHE + : ResponseSource.NETWORK; + return new ResponseStrategy(conditionalRequest, response, responseSource); + } +} 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 471539a46..2483b9d0e 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 @@ -36,30 +36,38 @@ public final class SpdyTransport implements Transport { this.spdyConnection = spdyConnection; } - @Override public OutputStream createRequestBody() throws IOException { - long fixedContentLength = httpEngine.policy.getFixedContentLength(); - if (fixedContentLength != -1) { - httpEngine.requestHeaders.setContentLength(fixedContentLength); + @Override public RequestHeaders prepareRequestHeaders(RequestHeaders requestHeaders) { + RequestHeaders.Builder builder = requestHeaders.newBuilder(); + + String version = httpEngine.connection.getHttpMinorVersion() == 1 ? "HTTP/1.1" : "HTTP/1.0"; + URL url = httpEngine.policy.getURL(); + builder.addSpdyRequestHeaders(httpEngine.method, HttpEngine.requestPath(url), version, + HttpEngine.getOriginAddress(url), httpEngine.uri.getScheme()); + + if (httpEngine.hasRequestBody()) { + long fixedContentLength = httpEngine.policy.getFixedContentLength(); + if (fixedContentLength != -1) { + builder.setContentLength(fixedContentLength); + } } + + return builder.build(); + } + + @Override public OutputStream createRequestBody() throws IOException { // TODO: if we aren't streaming up to the server, we should buffer the whole request writeRequestHeaders(); return stream.getOutputStream(); } @Override public void writeRequestHeaders() throws IOException { - if (stream != null) { - return; - } + if (stream != null) return; + httpEngine.writingRequestHeaders(); - RawHeaders requestHeaders = httpEngine.requestHeaders.getHeaders(); - String version = httpEngine.connection.getHttpMinorVersion() == 1 ? "HTTP/1.1" : "HTTP/1.0"; - URL url = httpEngine.policy.getURL(); - requestHeaders.addSpdyRequestHeaders(httpEngine.method, HttpEngine.requestPath(url), version, - HttpEngine.getOriginAddress(url), httpEngine.uri.getScheme()); boolean hasRequestBody = httpEngine.hasRequestBody(); boolean hasResponseBody = true; - stream = spdyConnection.newStream(requestHeaders.toNameValueBlock(), hasRequestBody, - hasResponseBody); + stream = spdyConnection.newStream(httpEngine.requestHeaders.getHeaders().toNameValueBlock(), + hasRequestBody, hasResponseBody); stream.setReadTimeout(httpEngine.client.getReadTimeout()); } @@ -75,10 +83,7 @@ public final class SpdyTransport implements Transport { List nameValueBlock = stream.getResponseHeaders(); RawHeaders rawHeaders = RawHeaders.fromNameValueBlock(nameValueBlock); httpEngine.receiveHeaders(rawHeaders); - - ResponseHeaders headers = new ResponseHeaders(httpEngine.uri, rawHeaders); - headers.setTransport("spdy/3"); - return headers; + return new ResponseHeaders(httpEngine.uri, rawHeaders); } @Override public InputStream getTransferStream(CacheRequest cacheRequest) throws IOException { diff --git a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java index d408bfec0..e9235739f 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java +++ b/okhttp/src/main/java/com/squareup/okhttp/internal/http/Transport.java @@ -22,6 +22,13 @@ import java.io.OutputStream; import java.net.CacheRequest; interface Transport { + /** + * Returns headers equivalent to {@code requestHeaders} but with + * transport-specific changes. For example, this may set a Transfer-Encoding + * header if it is required but not present for the current transport. + */ + RequestHeaders prepareRequestHeaders(RequestHeaders requestHeaders); + /** * Returns an output stream where the request body can be written. The * returned stream will of one of two types: diff --git a/okhttp/src/test/java/com/squareup/okhttp/AsyncApiTest.java b/okhttp/src/test/java/com/squareup/okhttp/AsyncApiTest.java index 311529688..849fb8b58 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/AsyncApiTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/AsyncApiTest.java @@ -62,7 +62,7 @@ public final class AsyncApiTest { .build(); client.enqueue(request, receiver); - receiver.await(request) + receiver.await(request.url()) .assertCode(200) .assertContainsHeaders("Content-Type: text/plain") .assertBody("abc"); @@ -83,7 +83,7 @@ public final class AsyncApiTest { Request request = new Request.Builder(server.getUrl("/")).build(); client.enqueue(request, receiver); - receiver.await(request).assertHandshake(); + receiver.await(request.url()).assertHandshake(); } @Test public void post() throws Exception { @@ -95,7 +95,7 @@ public final class AsyncApiTest { .build(); client.enqueue(request, receiver); - receiver.await(request) + receiver.await(request.url()) .assertCode(200) .assertBody("abc"); @@ -114,12 +114,12 @@ public final class AsyncApiTest { Request request1 = new Request.Builder(server.getUrl("/")).build(); client.enqueue(request1, receiver); - receiver.await(request1).assertCode(200).assertBody("A"); + receiver.await(request1.url()).assertCode(200).assertBody("A"); assertNull(server.takeRequest().getHeader("If-None-Match")); Request request2 = new Request.Builder(server.getUrl("/")).build(); client.enqueue(request2, receiver); - receiver.await(request2).assertCode(200).assertBody("A"); + receiver.await(request2.url()).assertCode(200).assertBody("A"); assertEquals("v1", server.takeRequest().getHeader("If-None-Match")); } } diff --git a/okhttp/src/test/java/com/squareup/okhttp/RecordingReceiver.java b/okhttp/src/test/java/com/squareup/okhttp/RecordingReceiver.java index 018d0fb52..5d36c8d6c 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/RecordingReceiver.java +++ b/okhttp/src/test/java/com/squareup/okhttp/RecordingReceiver.java @@ -17,6 +17,7 @@ package com.squareup.okhttp; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.net.URL; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; @@ -29,8 +30,8 @@ import java.util.concurrent.TimeUnit; public class RecordingReceiver implements Response.Receiver { public static final long TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(10); - private final Map inFlightResponses - = new LinkedHashMap(); + private final Map inFlightResponses + = new LinkedHashMap(); private final List responses = new ArrayList(); @Override public synchronized void onFailure(Failure failure) { @@ -39,10 +40,10 @@ public class RecordingReceiver implements Response.Receiver { } @Override public synchronized boolean onResponse(Response response) throws IOException { - ByteArrayOutputStream out = inFlightResponses.get(response.request()); + ByteArrayOutputStream out = inFlightResponses.get(response); if (out == null) { out = new ByteArrayOutputStream(); - inFlightResponses.put(response.request(), out); + inFlightResponses.put(response, out); } byte[] buffer = new byte[1024]; @@ -52,7 +53,7 @@ public class RecordingReceiver implements Response.Receiver { int c = body.byteStream().read(buffer); if (c == -1) { - inFlightResponses.remove(response.request()); + inFlightResponses.remove(response); responses.add(new RecordedResponse( response.request(), response, out.toString("UTF-8"), null)); notifyAll(); @@ -69,11 +70,11 @@ public class RecordingReceiver implements Response.Receiver { * Returns the recorded response triggered by {@code request}. Throws if the * response isn't enqueued before the timeout. */ - public synchronized RecordedResponse await(Request request) throws Exception { + public synchronized RecordedResponse await(URL url) throws Exception { long timeoutMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime()) + TIMEOUT_MILLIS; while (true) { for (RecordedResponse recordedResponse : responses) { - if (recordedResponse.request == request) { + if (recordedResponse.request.url().equals(url)) { return recordedResponse; } } @@ -83,6 +84,6 @@ public class RecordingReceiver implements Response.Receiver { wait(timeoutMillis - nowMillis); } - throw new AssertionError("Timed out waiting for response to " + request); + throw new AssertionError("Timed out waiting for response to " + url); } } 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 82726dc03..a27ed239b 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 @@ -774,7 +774,7 @@ public final class HttpResponseCacheTest { URLConnection request2 = openConnection(url); if (expectCached) { - assertEquals("1", request1.getHeaderField("X-Response-ID")); + assertEquals("1", request2.getHeaderField("X-Response-ID")); } else { assertEquals("2", request2.getHeaderField("X-Response-ID")); } diff --git a/okhttp/src/test/java/com/squareup/okhttp/internal/http/RawHeadersTest.java b/okhttp/src/test/java/com/squareup/okhttp/internal/http/RawHeadersTest.java index 4ce80a5eb..2f6c92476 100644 --- a/okhttp/src/test/java/com/squareup/okhttp/internal/http/RawHeadersTest.java +++ b/okhttp/src/test/java/com/squareup/okhttp/internal/http/RawHeadersTest.java @@ -31,57 +31,63 @@ public final class RawHeadersTest { ":status", "200 OK", ":version", "HTTP/1.1"); RawHeaders rawHeaders = RawHeaders.fromNameValueBlock(nameValueBlock); - assertEquals(3, rawHeaders.length()); + assertEquals(4, rawHeaders.length()); assertEquals("HTTP/1.1 200 OK", rawHeaders.getStatusLine()); assertEquals("no-cache, no-store", rawHeaders.get("cache-control")); assertEquals("Cookie2", rawHeaders.get("set-cookie")); - assertEquals("cache-control", rawHeaders.getFieldName(0)); - assertEquals("no-cache, no-store", rawHeaders.getValue(0)); - assertEquals("set-cookie", rawHeaders.getFieldName(1)); - assertEquals("Cookie1", rawHeaders.getValue(1)); + assertEquals("spdy/3", rawHeaders.get(ResponseHeaders.SELECTED_TRANSPORT)); + assertEquals(ResponseHeaders.SELECTED_TRANSPORT, rawHeaders.getFieldName(0)); + assertEquals("spdy/3", rawHeaders.getValue(0)); + assertEquals("cache-control", rawHeaders.getFieldName(1)); + assertEquals("no-cache, no-store", rawHeaders.getValue(1)); assertEquals("set-cookie", rawHeaders.getFieldName(2)); - assertEquals("Cookie2", rawHeaders.getValue(2)); + assertEquals("Cookie1", rawHeaders.getValue(2)); + assertEquals("set-cookie", rawHeaders.getFieldName(3)); + assertEquals("Cookie2", rawHeaders.getValue(3)); assertNull(rawHeaders.get(":status")); assertNull(rawHeaders.get(":version")); } @Test public void toNameValueBlock() { - RawHeaders rawHeaders = new RawHeaders(); - rawHeaders.add("cache-control", "no-cache, no-store"); - rawHeaders.add("set-cookie", "Cookie1"); - rawHeaders.add("set-cookie", "Cookie2"); - rawHeaders.add(":status", "200 OK"); + RawHeaders.Builder builder = new RawHeaders.Builder(); + builder.add("cache-control", "no-cache, no-store"); + builder.add("set-cookie", "Cookie1"); + builder.add("set-cookie", "Cookie2"); + builder.add(":status", "200 OK"); // TODO: fromNameValueBlock should take the status line headers - List nameValueBlock = rawHeaders.toNameValueBlock(); - List expected = - Arrays.asList("cache-control", "no-cache, no-store", "set-cookie", "Cookie1\u0000Cookie2", - ":status", "200 OK"); + List nameValueBlock = builder.build().toNameValueBlock(); + List expected = Arrays.asList( + "cache-control", "no-cache, no-store", + "set-cookie", "Cookie1\u0000Cookie2", + ":status", "200 OK"); assertEquals(expected, nameValueBlock); } @Test public void toNameValueBlockDropsForbiddenHeaders() { - RawHeaders rawHeaders = new RawHeaders(); - rawHeaders.add("Connection", "close"); - rawHeaders.add("Transfer-Encoding", "chunked"); - assertEquals(Arrays.asList(), rawHeaders.toNameValueBlock()); + RawHeaders.Builder builder = new RawHeaders.Builder(); + builder.add("Connection", "close"); + builder.add("Transfer-Encoding", "chunked"); + assertEquals(Arrays.asList(), builder.build().toNameValueBlock()); } @Test public void statusMessage() throws IOException { - RawHeaders rawHeaders = new RawHeaders(); - final String message = "Temporary Redirect"; - final int version = 1; - final int code = 200; - rawHeaders.setStatusLine("HTTP/1." + version + " " + code + " " + message); + RawHeaders.Builder builder = new RawHeaders.Builder(); + String message = "Temporary Redirect"; + int version = 1; + int code = 200; + builder.setStatusLine("HTTP/1." + version + " " + code + " " + message); + RawHeaders rawHeaders = builder.build(); assertEquals(message, rawHeaders.getResponseMessage()); assertEquals(version, rawHeaders.getHttpMinorVersion()); assertEquals(code, rawHeaders.getResponseCode()); } @Test public void statusMessageWithEmptyMessage() throws IOException { - RawHeaders rawHeaders = new RawHeaders(); - final int version = 1; - final int code = 503; - rawHeaders.setStatusLine("HTTP/1." + version + " " + code + " "); + RawHeaders.Builder builder = new RawHeaders.Builder(); + int version = 1; + int code = 503; + builder.setStatusLine("HTTP/1." + version + " " + code + " "); + RawHeaders rawHeaders = builder.build(); assertEquals("", rawHeaders.getResponseMessage()); assertEquals(version, rawHeaders.getHttpMinorVersion()); assertEquals(code, rawHeaders.getResponseCode()); @@ -93,10 +99,11 @@ public final class RawHeadersTest { * http://www.w3.org/Protocols/rfc2616/rfc2616-sec6.html#sec6.1 */ @Test public void statusMessageWithEmptyMessageAndNoLeadingSpace() throws IOException { - RawHeaders rawHeaders = new RawHeaders(); - final int version = 1; - final int code = 503; - rawHeaders.setStatusLine("HTTP/1." + version + " " + code); + RawHeaders.Builder builder = new RawHeaders.Builder(); + int version = 1; + int code = 503; + builder.setStatusLine("HTTP/1." + version + " " + code); + RawHeaders rawHeaders = builder.build(); assertEquals("", rawHeaders.getResponseMessage()); assertEquals(version, rawHeaders.getHttpMinorVersion()); assertEquals(code, rawHeaders.getResponseCode()); 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 87cc0a372..f7f617882 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 @@ -213,12 +213,14 @@ public final class URLConnectionTest { fail("Modified an unmodifiable view."); } catch (UnsupportedOperationException expected) { } - assertEquals("A", urlConnection.getHeaderFieldKey(0)); - assertEquals("c", urlConnection.getHeaderField(0)); - assertEquals("B", urlConnection.getHeaderFieldKey(1)); - assertEquals("d", urlConnection.getHeaderField(1)); - assertEquals("A", urlConnection.getHeaderFieldKey(2)); - assertEquals("e", urlConnection.getHeaderField(2)); + assertEquals(ResponseHeaders.SELECTED_TRANSPORT, urlConnection.getHeaderFieldKey(0)); + assertEquals("http/1.1", urlConnection.getHeaderField(0)); + assertEquals("A", urlConnection.getHeaderFieldKey(1)); + assertEquals("c", urlConnection.getHeaderField(1)); + assertEquals("B", urlConnection.getHeaderFieldKey(2)); + assertEquals("d", urlConnection.getHeaderField(2)); + assertEquals("A", urlConnection.getHeaderFieldKey(3)); + assertEquals("e", urlConnection.getHeaderField(3)); } @Test public void serverSendsInvalidResponseHeaders() throws Exception {