1
0
mirror of https://github.com/square/okhttp.git synced 2026-01-25 16:01:38 +03:00

Fix a bug in SPDY plus HTTP caching.

With spdy/3 the built-in headers were renamed from 'status'
to ':status' and 'version' to ':version'. This caused the
response cache to break, since it was parsing lines like
':status: 200 OK' by splitting on the first colon.

The fix drops these synthetic headers as soon as they're
received; converting them to the HTTP form immediately.
This commit is contained in:
jwilson
2013-07-20 12:04:53 -04:00
parent 4b70a92728
commit c2366b7658
4 changed files with 37 additions and 23 deletions

View File

@@ -123,23 +123,6 @@ public final class RawHeaders {
this.httpMinorVersion = httpMinorVersion;
}
public void computeResponseStatusLineFromSpdyHeaders() throws IOException {
String status = null;
String version = null;
for (int i = 0; i < namesAndValues.size(); i += 2) {
String name = namesAndValues.get(i);
if (":status".equals(name)) {
status = namesAndValues.get(i + 1);
} else if (":version".equals(name)) {
version = namesAndValues.get(i + 1);
}
}
if (status == null || version == null) {
throw new ProtocolException("Expected ':status' and ':version' headers not present");
}
setStatusLine(version + " " + status);
}
/**
* @param method like "GET", "POST", "HEAD", etc.
* @param path like "/foo/bar.html"
@@ -425,10 +408,13 @@ public final class RawHeaders {
return result;
}
public static RawHeaders fromNameValueBlock(List<String> nameValueBlock) {
/** Returns headers for a name value block containing a SPDY response. */
public static RawHeaders fromNameValueBlock(List<String> nameValueBlock) throws IOException {
if (nameValueBlock.size() % 2 != 0) {
throw new IllegalArgumentException("Unexpected name value block: " + nameValueBlock);
}
String status = null;
String version = null;
RawHeaders result = new RawHeaders();
for (int i = 0; i < nameValueBlock.size(); i += 2) {
String name = nameValueBlock.get(i);
@@ -438,11 +424,21 @@ public final class RawHeaders {
if (end == -1) {
end = values.length();
}
result.namesAndValues.add(name);
result.namesAndValues.add(values.substring(start, end));
String value = values.substring(start, end);
if (":status".equals(name)) {
status = value;
} else if (":version".equals(name)) {
version = value;
} else {
result.namesAndValues.add(name);
result.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);
return result;
}
}

View File

@@ -69,7 +69,6 @@ public final class SpdyTransport implements Transport {
@Override public ResponseHeaders readResponseHeaders() throws IOException {
List<String> nameValueBlock = stream.getResponseHeaders();
RawHeaders rawHeaders = RawHeaders.fromNameValueBlock(nameValueBlock);
rawHeaders.computeResponseStatusLineFromSpdyHeaders();
httpEngine.receiveHeaders(rawHeaders);
ResponseHeaders headers = new ResponseHeaders(httpEngine.uri, rawHeaders);

View File

@@ -23,7 +23,7 @@ import org.junit.Test;
import static org.junit.Assert.assertEquals;
public final class RawHeadersTest {
@Test public void parseNameValueBlock() {
@Test public void parseNameValueBlock() throws IOException {
List<String> nameValueBlock =
Arrays.asList("cache-control", "no-cache, no-store", "set-cookie", "Cookie1\u0000Cookie2",
":status", "200 OK");

View File

@@ -90,12 +90,14 @@ public final class HttpOverSpdyTest {
}
@Test public void get() throws Exception {
MockResponse response = new MockResponse().setBody("ABCDE");
MockResponse response = new MockResponse().setBody("ABCDE").setStatus("HTTP/1.1 200 Sweet");
server.enqueue(response);
server.play();
HttpURLConnection connection = client.open(server.getUrl("/foo"));
assertContent("ABCDE", connection, Integer.MAX_VALUE);
assertEquals(200, connection.getResponseCode());
assertEquals("Sweet", connection.getResponseMessage());
RecordedRequest request = server.takeRequest();
assertEquals("GET /foo HTTP/1.1", request.getRequestLine());
@@ -211,6 +213,23 @@ public final class HttpOverSpdyTest {
assertEquals(2, cache.getHitCount());
}
@Test public void conditionalCache() throws IOException {
client.setResponseCache(cache);
server.enqueue(new MockResponse().addHeader("ETag: v1").setBody("A"));
server.enqueue(new MockResponse().setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED));
server.play();
assertContent("A", client.open(server.getUrl("/")), Integer.MAX_VALUE);
assertEquals(1, cache.getRequestCount());
assertEquals(1, cache.getNetworkCount());
assertEquals(0, cache.getHitCount());
assertContent("A", client.open(server.getUrl("/")), Integer.MAX_VALUE);
assertEquals(2, cache.getRequestCount());
assertEquals(2, cache.getNetworkCount());
assertEquals(1, cache.getHitCount());
}
@Test public void acceptAndTransmitCookies() throws Exception {
CookieManager cookieManager = new CookieManager();
client.setCookieHandler(cookieManager);