Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't unzip if there isn't a response body. #474

Merged
merged 1 commit into from
Jan 22, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -401,16 +401,25 @@ public final void release(boolean streamCanceled) {
}
}

/**
* Initialize the response content stream from the response transfer stream.
* These two streams are the same unless we're doing transparent gzip, in
* which case the content stream is decompressed.
*
* <p>Whenever we do transparent gzip we also strip the corresponding headers.
* We strip the Content-Encoding header to prevent the application from
* attempting to double decompress. We strip the Content-Length header because
* it is the length of the compressed content, but the application is only
* interested in the length of the uncompressed content.
*
* <p>This method should only be used for non-empty response bodies. Response
* codes like "304 Not Modified" can include "Content-Encoding: gzip" without
* a response body and we will crash if we attempt to decompress the zero-byte
* stream.
*/
private void initContentStream(InputStream transferStream) throws IOException {
responseTransferIn = transferStream;
if (transparentGzip && "gzip".equalsIgnoreCase(response.header("Content-Encoding"))) {
// If the response was transparently gzipped, remove the gzip header field
// so clients don't double decompress. http://b/3009828
//
// Also remove the Content-Length in this case because it contains the
// 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.
response = response.newBuilder()
.removeHeader("Content-Encoding")
.removeHeader("Content-Length")
Expand Down Expand Up @@ -561,10 +570,14 @@ public final void readResponse() throws IOException {
}
}

if (hasResponseBody()) {
maybeCache();
if (!hasResponseBody()) {
// Don't call initContentStream() when the response doesn't have any content.
responseTransferIn = transport.getTransferStream(cacheRequest);
responseBodyIn = responseTransferIn;
return;
}

maybeCache();
initContentStream(transport.getTransferStream(cacheRequest));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.squareup.okhttp.internal.http;

import com.squareup.okhttp.HttpResponseCache;
import com.squareup.okhttp.OkAuthenticator.Credential;
import com.squareup.okhttp.OkHttpClient;
import com.squareup.okhttp.Protocol;
import com.squareup.okhttp.internal.RecordingAuthenticator;
Expand Down Expand Up @@ -75,7 +76,6 @@
import org.junit.Ignore;
import org.junit.Test;

import static com.squareup.okhttp.OkAuthenticator.Credential;
import static com.squareup.okhttp.internal.http.StatusLine.HTTP_TEMP_REDIRECT;
import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_END;
import static com.squareup.okhttp.mockwebserver.SocketPolicy.DISCONNECT_AT_START;
Expand Down Expand Up @@ -2572,6 +2572,38 @@ private void reusedConnectionFailsWithPost(TransferKind transferKind, int reques
assertEquals(Long.toString(contentLength), request.getHeader("Content-Length"));
}

/**
* We had a bug where we attempted to gunzip responses that didn't have a
* body. This only came up with 304s since that response code can include
* headers (like "Content-Encoding") without any content to go along with it.
* https://github.com/square/okhttp/issues/358
*/
@Test public void noTransparentGzipFor304NotModified() throws Exception {
server.enqueue(new MockResponse()
.clearHeaders()
.setResponseCode(HttpURLConnection.HTTP_NOT_MODIFIED)
.addHeader("Content-Encoding: gzip"));
server.enqueue(new MockResponse().setBody("b"));

server.play();

HttpURLConnection connection1 = client.open(server.getUrl("/"));
assertEquals(HttpURLConnection.HTTP_NOT_MODIFIED, connection1.getResponseCode());
assertContent("", connection1);
connection1.getInputStream().close();

HttpURLConnection connection2 = client.open(server.getUrl("/"));
assertEquals(HttpURLConnection.HTTP_OK, connection2.getResponseCode());
assertContent("b", connection2);
connection2.getInputStream().close();

RecordedRequest requestA = server.takeRequest();
assertEquals(0, requestA.getSequenceNumber());

RecordedRequest requestB = server.takeRequest();
assertEquals(1, requestB.getSequenceNumber());
}

/** Returns a gzipped copy of {@code bytes}. */
public byte[] gzip(byte[] bytes) throws IOException {
ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();
Expand Down