From 357f77221cd4a41d06c6c84b7bb26cd79b307190 Mon Sep 17 00:00:00 2001 From: aahlenst Date: Mon, 16 Sep 2013 12:21:17 +0200 Subject: [PATCH] Bug fixed that caused gzipped responses to be returned from cache after cache validation (#298). --- .../com/squareup/okhttp/internal/http/HttpEngine.java | 9 ++++++++- .../okhttp/internal/http/HttpResponseCacheTest.java | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-) 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 142def466..df5af0c47 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 @@ -649,10 +649,17 @@ public class HttpEngine { if (cachedResponseHeaders.validate(responseHeaders)) { release(false); ResponseHeaders combinedHeaders = cachedResponseHeaders.combine(responseHeaders); - setResponse(combinedHeaders, cachedResponseBody); + this.responseHeaders = combinedHeaders; + + // Update the cache after applying the combined headers but before initializing the content + // stream, otherwise the Content-Encoding header (if present) will be stripped from the + // combined headers and not end up in the cache file if transparent gzip compression is + // turned on. OkResponseCache responseCache = client.getOkResponseCache(); responseCache.trackConditionalCacheHit(); responseCache.update(cacheResponse, policy.getHttpConnectionToCache()); + + initContentStream(cachedResponseBody); return; } else { Util.closeQuietly(cachedResponseBody); 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 aa3948b19..cbbeb987a 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 @@ -946,8 +946,14 @@ public final class HttpResponseCacheTest { server.enqueue( response.setBody(gzip("ABCABCABC".getBytes("UTF-8"))).addHeader("Content-Encoding: gzip")); server.enqueue(new MockResponse().setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED)); + server.enqueue(new MockResponse().setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED)); server.play(); + + // At least three request/response pairs are required because after the first request is cached + // a different execution path might be taken. Thus modifications to the cache applied during + // the second request might not be visible until another request is performed. + assertEquals("ABCABCABC", readAscii(openConnection(server.getUrl("/")))); assertEquals("ABCABCABC", readAscii(openConnection(server.getUrl("/")))); assertEquals("ABCABCABC", readAscii(openConnection(server.getUrl("/")))); }