Skip to content

Re-open: GZIP content not inflated #932

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

Closed
phleemac opened this issue Aug 22, 2015 · 8 comments
Closed

Re-open: GZIP content not inflated #932

phleemac opened this issue Aug 22, 2015 · 8 comments
Assignees
Labels
Milestone

Comments

@phleemac
Copy link

I encounter exactly the same problem as described in #641 using version 1.4.8

This is caused by a bug in com.loopj.android.http.AsyncHttpClient#isInputStreamGZIPCompressed: in the statement int readStatus = inputStream.read(signature); where signature might only be partially filled (see javadoc http://docs.oracle.com/javase/7/docs/api/java/io/PushbackInputStream.html#read(byte[],%20int,%20int)

Should check the returned byte count and re-read to fetch the 2nd byte if necessary

@phleemac
Copy link
Author

I temporarily workaround the problem by inserting another response interceptor to pre-fetch the first two bytes.

My Scala code just to illustrate the idea:

  private lazy val client = {
    val c = new AsyncHttpClient
    // TODO: make sure at least 2 bytes are available in the content input stream to workaround 
    // https://github.com/loopj/android-async-http/issues/932 (android-async-http 1.4.8)
    c.getHttpClient match {
      case httpClient: DefaultHttpClient =>
        httpClient.addResponseInterceptor(new HttpResponseInterceptor {
          override def process(response: HttpResponse, httpContext: HttpContext): Unit = {
            val entity: HttpEntity = response.getEntity
            if (entity == null) {
              return
            } else {
              response.setEntity(new HttpEntityWrapper(entity) {
                override def getContent: InputStream = {
                  val wrappedStream = wrappedEntity.getContent
                  val pushbackStream = new PushbackInputStream(wrappedStream, 2)
                  def preRead(cnt: Int, buf: Seq[Int]): Unit = {
                    lazy val b = pushbackStream.read()
                    if ((cnt > 0) && (b >= 0)) {
                      preRead(cnt - 1, b +: buf)
                    } else {
                      buf.foreach(pushbackStream.unread(_))
                    }
                  }
                  preRead(2, Seq.empty)
                  pushbackStream
                }
              })
            }
          }
        }, 0)
      case _ => // skip
    }
    c
  }

@Queatz
Copy link

Queatz commented Aug 23, 2015

+1 Running into this as well with the Google Autocomplete API.

@smarek smarek added this to the 1.4.9 milestone Aug 24, 2015
@smarek smarek self-assigned this Aug 24, 2015
@smarek
Copy link
Member

smarek commented Aug 24, 2015

Thank you, I'm able to reproduce this issue, and I'll push the fix asap

@smarek
Copy link
Member

smarek commented Aug 24, 2015

Simple fix is patch like this

diff --git a/library/src/main/java/com/loopj/android/http/AsyncHttpClient.java b/library/src/main/java/com/loopj/android/http/AsyncHttpClient.java
index b9ee313..b10ea6f 100755
--- a/library/src/main/java/com/loopj/android/http/AsyncHttpClient.java
+++ b/library/src/main/java/com/loopj/android/http/AsyncHttpClient.java
@@ -1608,7 +1608,8 @@ public class AsyncHttpClient {
         public InputStream getContent() throws IOException {
             wrappedStream = wrappedEntity.getContent();
             pushbackStream = new PushbackInputStream(wrappedStream, 2);
-            if (isInputStreamGZIPCompressed(pushbackStream)) {
+            Header enc = wrappedEntity.getContentEncoding();
+            if ((enc != null && "gzip".equalsIgnoreCase(enc.getValue())) || isInputStreamGZIPCompressed(pushbackStream)) {
                 gzippedStream = new GZIPInputStream(pushbackStream);
                 return gzippedStream;
             } else {

But this is not final solution, and will be pushed in better form

@SergeyA
Copy link

SergeyA commented Aug 25, 2015

You can disable gzip as a temporary fix:

httpClient = new AsyncHttpClient();
httpClient.addHeader("Accept-Encoding", "identity"); // disable gzip
httpClient.get(...)

@phleemac
Copy link
Author

Thanks @smarek. But your patch is doing redundant check on the contentEncoding. It's already checked at around line 280:

...
                final Header encoding = entity.getContentEncoding();
                if (encoding != null) {
                    for (HeaderElement element : encoding.getElements()) {
                        if (element.getName().equalsIgnoreCase(ENCODING_GZIP)) {
                            response.setEntity(new InflatingEntity(entity));
                            break;
                        }
                    }
                }
...

The patch effectively disable the call to isInputStreamGZIPCompressed such that it will never be invoked. In that case, better remove the call to isInputStreamGZIPCompressed altogether.

If we want to play safe and keep the isInputStreamGZIPCompressed checking (e.g. contentEncoding indicates gzip, but the actual content isn't... will it happen?), better remove the redundant contentEncoding checking from your patch and fix isInputStreamGZIPCompressed like:

    public static boolean isInputStreamGZIPCompressed(final PushbackInputStream inputStream) throws IOException {
        if (inputStream == null)
            return false;
        byte[] signature = new byte[2];
        int count = 0;
        try {
            while (count < 2) {
                int readCount = inputStream.read(signature, count, 2 - count);
                if (readCount < 0) return false;
                count = count + readCount;
            }
        } finally {
            inputStream.unread(signature, 0, count);
        }    
        int streamHeader = ((int) signature[0] & 0xff) | ((signature[1] << 8) & 0xff00);
        return readStatus == 2 && GZIPInputStream.GZIP_MAGIC == streamHeader;
    }

(Sorry for not creating a patch / pull-request, I haven't write Java for a long time and don't have a Java development environment on hand)

@smarek
Copy link
Member

smarek commented Sep 3, 2015

@SergeyA thanks for adding workaround til the fix is published in stable
@phleemac yes, that's correct, it does assume that stream is gzip compressed without checking the starting two-byte signature, however the check was introduced with #583 which is meaningfull for most usages, so I'll try to implement that wait for two bytes on input procedure as correct solution

smarek added a commit that referenced this issue Sep 12, 2015
@smarek
Copy link
Member

smarek commented Sep 12, 2015

Thank you nicely for provided patch @phleemac, I've included that in commit 4bffe5f, which will be released in 1.4.9 as soon as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants