From 1a824f3c644fbc7f014a910d3c786677a260cf7a Mon Sep 17 00:00:00 2001 From: jwilson Date: Wed, 28 Aug 2013 22:45:37 -0700 Subject: [PATCH] Continue to read old bad cache responses. --- .../com/squareup/okhttp/internal/Util.java | 30 ++++++++ .../squareup/okhttp/HttpResponseCache.java | 27 +------- .../okhttp/internal/http/RawHeaders.java | 13 ++-- .../internal/http/HttpResponseCacheTest.java | 69 +++++++++++++++++++ 4 files changed, 108 insertions(+), 31 deletions(-) diff --git a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java index d1b285708..29c6c255f 100644 --- a/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java +++ b/okhttp-protocols/src/main/java/com/squareup/okhttp/internal/Util.java @@ -24,11 +24,14 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.Reader; import java.io.StringWriter; +import java.io.UnsupportedEncodingException; import java.net.Socket; import java.net.URI; import java.net.URL; import java.nio.ByteOrder; import java.nio.charset.Charset; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -50,6 +53,9 @@ public final class Util { public static final Charset UTF_8 = Charset.forName("UTF-8"); private static AtomicReference skipBuffer = new AtomicReference(); + private static final char[] DIGITS = + { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + private Util() { } @@ -331,6 +337,30 @@ public final class Util { return result.toString(); } + /** Returns a 32 character string containing a hash of {@code s}. */ + public static String hash(String s) { + try { + MessageDigest messageDigest = MessageDigest.getInstance("MD5"); + byte[] md5bytes = messageDigest.digest(s.getBytes("UTF-8")); + return bytesToHexString(md5bytes); + } catch (NoSuchAlgorithmException e) { + throw new AssertionError(e); + } catch (UnsupportedEncodingException e) { + throw new AssertionError(e); + } + } + + private static String bytesToHexString(byte[] bytes) { + char[] digits = DIGITS; + char[] buf = new char[bytes.length * 2]; + int c = 0; + for (byte b : bytes) { + buf[c++] = digits[(b >> 4) & 0xf]; + buf[c++] = digits[b & 0xf]; + } + return new String(buf); + } + /** Returns an immutable copy of {@code list}. */ public static List immutableList(List list) { return Collections.unmodifiableList(new ArrayList(list)); diff --git a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java index a1c653c14..821031827 100644 --- a/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java +++ b/okhttp/src/main/java/com/squareup/okhttp/HttpResponseCache.java @@ -35,7 +35,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.OutputStreamWriter; -import java.io.UnsupportedEncodingException; import java.io.Writer; import java.net.CacheRequest; import java.net.CacheResponse; @@ -44,8 +43,6 @@ import java.net.ResponseCache; import java.net.SecureCacheResponse; import java.net.URI; import java.net.URLConnection; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; import java.security.Principal; import java.security.cert.Certificate; import java.security.cert.CertificateEncodingException; @@ -119,9 +116,6 @@ import static com.squareup.okhttp.internal.Util.UTF_8; * } */ public final class HttpResponseCache extends ResponseCache { - private static final char[] DIGITS = - { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - // TODO: add APIs to iterate the cache? private static final int VERSION = 201105; private static final int ENTRY_METADATA = 0; @@ -176,26 +170,7 @@ public final class HttpResponseCache extends ResponseCache { } private String uriToKey(URI uri) { - try { - MessageDigest messageDigest = MessageDigest.getInstance("MD5"); - byte[] md5bytes = messageDigest.digest(uri.toString().getBytes("UTF-8")); - return bytesToHexString(md5bytes); - } catch (NoSuchAlgorithmException e) { - throw new AssertionError(e); - } catch (UnsupportedEncodingException e) { - throw new AssertionError(e); - } - } - - private static String bytesToHexString(byte[] bytes) { - char[] digits = DIGITS; - char[] buf = new char[bytes.length * 2]; - int c = 0; - for (byte b : bytes) { - buf[c++] = digits[(b >> 4) & 0xf]; - buf[c++] = digits[b & 0xf]; - } - return new String(buf); + return Util.hash(uri.toString()); } @Override public CacheResponse get(URI uri, String requestMethod, 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 e1fdcf457..8b4532070 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 @@ -164,14 +164,17 @@ public final class RawHeaders { /** * Add an HTTP header line containing a field name, a literal colon, and a - * value. + * 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(":"); - if (index == -1) { - addLenient("", line); - } else { + 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. } } 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 8a2e900a2..aa3948b19 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 @@ -20,6 +20,7 @@ import com.squareup.okhttp.HttpResponseCache; import com.squareup.okhttp.OkHttpClient; import com.squareup.okhttp.ResponseSource; import com.squareup.okhttp.internal.SslContextBuilder; +import com.squareup.okhttp.internal.Util; import com.squareup.okhttp.mockwebserver.MockResponse; import com.squareup.okhttp.mockwebserver.MockWebServer; import com.squareup.okhttp.mockwebserver.RecordedRequest; @@ -27,6 +28,7 @@ import java.io.BufferedReader; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -1733,6 +1735,73 @@ public final class HttpResponseCacheTest { assertEquals("A", connection.getHeaderField("")); } + /** + * Old implementations of OkHttp's response cache wrote header fields like + * ":status: 200 OK". This broke our cached response parser because it split + * on the first colon. This regression test exists to help us read these old + * bad cache entries. + * + * https://github.com/square/okhttp/issues/227 + */ + @Test public void testGoldenCacheResponse() throws Exception { + cache.close(); + server.enqueue(new MockResponse() + .clearHeaders() + .setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED)); + server.play(); + + URL url = server.getUrl("/"); + String urlKey = Util.hash(url.toString()); + String entryMetadata = "" + + "" + url + "\n" + + "GET\n" + + "0\n" + + "HTTP/1.1 200 OK\n" + + "7\n" + + ":status: 200 OK\n" + + ":version: HTTP/1.1\n" + + "etag: foo\n" + + "content-length: 3\n" + + "OkHttp-Received-Millis: " + System.currentTimeMillis() + "\n" + + "X-Android-Response-Source: NETWORK 200\n" + + "OkHttp-Sent-Millis: " + System.currentTimeMillis() + "\n" + + "\n" + + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA\n" + + "1\n" + + "MIIBpDCCAQ2gAwIBAgIBATANBgkqhkiG9w0BAQsFADAYMRYwFAYDVQQDEw1qd2lsc29uLmxvY2FsMB4XDTEzMDgy" + + "OTA1MDE1OVoXDTEzMDgzMDA1MDE1OVowGDEWMBQGA1UEAxMNandpbHNvbi5sb2NhbDCBnzANBgkqhkiG9w0BAQEF" + + "AAOBjQAwgYkCgYEAlFW+rGo/YikCcRghOyKkJanmVmJSce/p2/jH1QvNIFKizZdh8AKNwojt3ywRWaDULA/RlCUc" + + "ltF3HGNsCyjQI/+Lf40x7JpxXF8oim1E6EtDoYtGWAseelawus3IQ13nmo6nWzfyCA55KhAWf4VipelEy8DjcuFK" + + "v6L0xwXnI0ECAwEAATANBgkqhkiG9w0BAQsFAAOBgQAuluNyPo1HksU3+Mr/PyRQIQS4BI7pRXN8mcejXmqyscdP" + + "7S6J21FBFeRR8/XNjVOp4HT9uSc2hrRtTEHEZCmpyoxixbnM706ikTmC7SN/GgM+SmcoJ1ipJcNcl8N0X6zym4dm" + + "yFfXKHu2PkTo7QFdpOJFvP3lIigcSZXozfmEDg==\n" + + "-1\n"; + String entryBody = "abc"; + String journalBody = "" + + "libcore.io.DiskLruCache\n" + + "1\n" + + "201105\n" + + "2\n" + + "\n" + + "CLEAN " + urlKey + " " + entryMetadata.length() + " " + entryBody.length() + "\n"; + writeFile(cache.getDirectory(), urlKey + ".0", entryMetadata); + writeFile(cache.getDirectory(), urlKey + ".1", entryBody); + writeFile(cache.getDirectory(), "journal", journalBody); + cache = new HttpResponseCache(cache.getDirectory(), Integer.MAX_VALUE); + client.setResponseCache(cache); + + HttpURLConnection connection = client.open(url); + assertEquals(entryBody, readAscii(connection)); + assertEquals("3", connection.getHeaderField("Content-Length")); + assertEquals("foo", connection.getHeaderField("etag")); + } + + private void writeFile(File directory, String file, String content) throws IOException { + OutputStream out = new FileOutputStream(new File(directory, file)); + out.write(content.getBytes(Util.UTF_8)); + out.close(); + } + /** * @param delta the offset from the current date to use. Negative * values yield dates in the past; positive values yield dates in the