-
-
Notifications
You must be signed in to change notification settings - Fork 735
Disable transparent ungzip and add decompress interceptor #36
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
/* | ||
* Copyright (c) 2015-present, Parse, LLC. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
package com.parse; | ||
|
||
import java.io.IOException; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.zip.GZIPInputStream; | ||
|
||
/** package */ class ParseDecompressInterceptor implements ParseNetworkInterceptor { | ||
|
||
private static final String CONTENT_ENCODING_HEADER = "Content-Encoding"; | ||
private static final String CONTENT_LENGTH_HEADER = "Content-Length"; | ||
private static final String GZIP_ENCODING = "gzip"; | ||
|
||
@Override | ||
public ParseHttpResponse intercept(Chain chain) throws IOException { | ||
ParseHttpRequest request = chain.getRequest(); | ||
ParseHttpResponse response = chain.proceed(request); | ||
// If the response is gziped, we need to decompress the stream and remove the gzip header. | ||
if (GZIP_ENCODING.equalsIgnoreCase(response.getHeader(CONTENT_ENCODING_HEADER))) { | ||
Map<String, String > newHeaders = new HashMap<>(response.getAllHeaders()); | ||
newHeaders.remove(CONTENT_ENCODING_HEADER); | ||
// Since before we decompress the stream, we can not know the actual length of the stream. | ||
// In this situation, we follow the OkHttp library, set the content-length of the response | ||
// to -1 | ||
newHeaders.put(CONTENT_LENGTH_HEADER, "-1"); | ||
// TODO(mengyan): Add builder constructor based on an existing ParseHttpResponse | ||
response = new ParseHttpResponse.Builder() | ||
.setTotalSize(-1) | ||
.setContentType(response.getContentType()) | ||
.setHeaders(newHeaders) | ||
.setReasonPhase(response.getReasonPhrase()) | ||
.setStatusCode(response.getStatusCode()) | ||
.setContent(new GZIPInputStream(response.getContent())) | ||
.build(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, since almost all interceptor PRs need to use this constructor, I will do this after I land these PRs. |
||
} | ||
return response; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,6 @@ | |
private static final String MAX_CONNECTIONS_PROPERTY_NAME = "http.maxConnections"; | ||
private static final String KEEP_ALIVE_PROPERTY_NAME = "http.keepAlive"; | ||
|
||
|
||
public static ParseHttpClient createClient(int socketOperationTimeout, | ||
SSLSessionCache sslSessionCache) { | ||
String httpClientLibraryName; | ||
|
@@ -155,4 +154,14 @@ public ParseHttpResponse proceed(ParseHttpRequest request) throws IOException { | |
return executeInternal(request); | ||
} | ||
} | ||
|
||
/** | ||
* When we find developers use interceptors, since we need expose the raw | ||
* response(ungziped response) to interceptors, we need to disable the transparent ungzip. | ||
* | ||
* @return {@code true} if we should disable the http library level auto decompress. | ||
*/ | ||
/* package */ boolean disableHttpLibraryAutoDecompress() { | ||
return externalInterceptors != null && externalInterceptors.size() > 0; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add documentation for the reasoning behind this logic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kk |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,117 @@ | ||
/* | ||
* Copyright (c) 2015-present, Parse, LLC. | ||
* All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the | ||
* LICENSE file in the root directory of this source tree. An additional grant | ||
* of patent rights can be found in the PATENTS file in the same directory. | ||
*/ | ||
|
||
package com.parse; | ||
|
||
import org.junit.Test; | ||
import org.junit.runner.RunWith; | ||
import org.robolectric.RobolectricGradleTestRunner; | ||
import org.robolectric.annotation.Config; | ||
|
||
import java.io.ByteArrayInputStream; | ||
import java.io.ByteArrayOutputStream; | ||
import java.io.IOException; | ||
import java.util.HashMap; | ||
import java.util.Map; | ||
import java.util.zip.GZIPOutputStream; | ||
|
||
import static org.junit.Assert.assertArrayEquals; | ||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertNull; | ||
|
||
@RunWith(RobolectricGradleTestRunner.class) | ||
@Config(constants = BuildConfig.class, sdk = 21) | ||
public class ParseDecompressInterceptorTest { | ||
|
||
@Test | ||
public void testDecompressInterceptorWithNotGZIPResponse() throws Exception { | ||
ParseDecompressInterceptor interceptor = new ParseDecompressInterceptor(); | ||
|
||
final String responseContent = "content"; | ||
ParseHttpResponse interceptedResponse = | ||
interceptor.intercept(new ParseNetworkInterceptor.Chain() { | ||
@Override | ||
public ParseHttpRequest getRequest() { | ||
// Generate test request | ||
return new ParseHttpRequest.Builder() | ||
.setUrl("www.parse.com") | ||
.setMethod(ParseRequest.Method.GET) | ||
.build(); | ||
} | ||
|
||
@Override | ||
public ParseHttpResponse proceed(ParseHttpRequest request) throws IOException { | ||
// Generate test response | ||
return new ParseHttpResponse.Builder() | ||
.setStatusCode(200) | ||
.setTotalSize(responseContent.length()) | ||
.setReasonPhase("Success") | ||
.setContentType("text/plain") | ||
.setContent(new ByteArrayInputStream(responseContent.getBytes())) | ||
.build(); | ||
} | ||
}); | ||
|
||
// Verify response is correct | ||
assertEquals(200, interceptedResponse.getStatusCode()); | ||
assertEquals(responseContent.length(), interceptedResponse.getTotalSize()); | ||
assertEquals("Success", interceptedResponse.getReasonPhrase()); | ||
assertEquals("text/plain", interceptedResponse.getContentType()); | ||
byte[] content = ParseIOUtils.toByteArray(interceptedResponse.getContent()); | ||
assertArrayEquals(responseContent.getBytes(), content); | ||
} | ||
|
||
@Test | ||
public void testDecompressInterceptorWithGZIPResponse() throws Exception { | ||
ParseDecompressInterceptor interceptor = new ParseDecompressInterceptor(); | ||
|
||
final String responseContent = "content"; | ||
ParseHttpResponse interceptedResponse = | ||
interceptor.intercept(new ParseNetworkInterceptor.Chain() { | ||
@Override | ||
public ParseHttpRequest getRequest() { | ||
// Generate test request | ||
return new ParseHttpRequest.Builder() | ||
.setUrl("www.parse.com") | ||
.setMethod(ParseRequest.Method.GET) | ||
.build(); | ||
} | ||
|
||
@Override | ||
public ParseHttpResponse proceed(ParseHttpRequest request) throws IOException { | ||
// Make gzip response content | ||
ByteArrayOutputStream byteOut = new ByteArrayOutputStream(); | ||
GZIPOutputStream gzipOut = new GZIPOutputStream(byteOut); | ||
gzipOut.write(responseContent.getBytes()); | ||
gzipOut.close(); | ||
// Make gzip encoding headers | ||
Map<String, String> headers = new HashMap<>(); | ||
headers.put("Content-Encoding", "gzip"); | ||
// Generate test response | ||
return new ParseHttpResponse.Builder() | ||
.setStatusCode(200) | ||
.setTotalSize(byteOut.toByteArray().length) | ||
.setReasonPhase("Success") | ||
.setContentType("text/plain") | ||
.setContent(new ByteArrayInputStream(byteOut.toByteArray())) | ||
.setHeaders(headers) | ||
.build(); | ||
} | ||
}); | ||
|
||
// Verify response is correct | ||
assertEquals(200, interceptedResponse.getStatusCode()); | ||
assertEquals(-1, interceptedResponse.getTotalSize()); | ||
assertEquals("Success", interceptedResponse.getReasonPhrase()); | ||
assertEquals("text/plain", interceptedResponse.getContentType()); | ||
assertNull(interceptedResponse.getHeader("Content-Encoding")); | ||
byte[] content = ParseIOUtils.toByteArray(interceptedResponse.getContent()); | ||
assertArrayEquals(responseContent.getBytes(), content); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment somewhere why we set
Content-Length
andtotalSize
to -1?