HTTPCLIENT-1075: Decompressing entities (DeflateDecompressingEntity, GzipDecompressingEntity) do not correctly handle content streaming

Contributed by James Abley <james.abley at gmail.com> 


git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@1088000 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,4 +1,16 @@
-Changes since 4.1
+Changes since 4.1.1
+
+* [HTTPCLIENT-1075] Decompressing entities (DeflateDecompressingEntity, GzipDecompressingEntity) 
+  do not correctly handle content streaming.
+  Contributed by James Abley <james.abley at gmail.com> 
+
+
+Release 4.1.1
+-------------------
+
+The HttpClient 4.1.1 is a bug fix release that addresses a number of issues reported since 
+release 4.1, including one critical security issue (HTTPCLIENT-1061). All users of HttpClient 4.0.x 
+and 4.1 are strongly encouraged to upgrade.
 
 * [HTTPCLIENT-1069] HttpHostConnectException not correctly retried for direct and non-tunnelled 
   proxy connections. 

httpclient/src/main/java/org/apache/http/client/entity/DecompressingEntity.java

@@ -44,6 +44,12 @@ abstract class DecompressingEntity extends HttpEntityWrapper {
      */
     private static final int BUFFER_SIZE = 1024 * 2;
 
+    /**
+     * DecompressingEntities are not repeatable, so they will return the same
+     * InputStream instance when {@link #getContent()} is called.
+     */
+    private InputStream content;
+
     /**
      * Creates a new {@link DecompressingEntity}.
      *
@@ -54,6 +60,23 @@ abstract class DecompressingEntity extends HttpEntityWrapper {
         super(wrapped);
     }
 
+    abstract InputStream getDecompressingInputStream(final InputStream wrapped) throws IOException;
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public InputStream getContent() throws IOException {
+        if (wrappedEntity.isStreaming()) {
+            if (content == null) {
+                content = getDecompressingInputStream(wrappedEntity.getContent());
+            }
+            return content;
+        } else {
+            return getDecompressingInputStream(wrappedEntity.getContent());
+        }
+    }
+
     /**
      * {@inheritDoc}
      */

httpclient/src/main/java/org/apache/http/client/entity/DeflateDecompressingEntity.java

@@ -63,12 +63,14 @@ public class DeflateDecompressingEntity extends DecompressingEntity {
     }
 
     /**
-     * {@inheritDoc}
+     * Returns the non-null InputStream that should be returned to by all requests to
+     * {@link #getContent()}.
+     *
+     * @return a non-null InputStream
+     * @throws IOException if there was a problem
      */
     @Override
-    public InputStream getContent() throws IOException {
+    InputStream getDecompressingInputStream(final InputStream wrapped) throws IOException {
-        InputStream wrapped = this.wrappedEntity.getContent();
-
         /*
          * A zlib stream will have a header.
          *

httpclient/src/main/java/org/apache/http/client/entity/GzipDecompressingEntity.java

@@ -51,16 +51,9 @@ public class GzipDecompressingEntity extends DecompressingEntity {
         super(entity);
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
-    public InputStream getContent() throws IOException, IllegalStateException {
+    InputStream getDecompressingInputStream(final InputStream wrapped) throws IOException {
-
+        return new GZIPInputStream(wrapped);
-        // the wrapped entity's getContent() decides about repeatability
-        InputStream wrappedin = wrappedEntity.getContent();
-
-        return new GZIPInputStream(wrappedin);
     }
 
     /**

httpclient/src/test/java/org/apache/http/client/entity/TestDecompressingEntity.java

@@ -0,0 +1,109 @@
+/*
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * <http://www.apache.org/>.
+ *
+ */
+
+package org.apache.http.client.entity;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.zip.CRC32;
+import java.util.zip.CheckedInputStream;
+import java.util.zip.Checksum;
+
+import org.apache.http.HttpEntity;
+import org.apache.http.entity.InputStreamEntity;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.util.EntityUtils;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestDecompressingEntity {
+
+    @Test
+    public void testNonStreaming() throws Exception {
+        CRC32 crc32 = new CRC32();
+        StringEntity wrapped = new StringEntity("1234567890", "ASCII");
+        ChecksumEntity entity = new ChecksumEntity(wrapped, crc32);
+        Assert.assertFalse(entity.isStreaming());
+        String s = EntityUtils.toString(entity);
+        Assert.assertEquals("1234567890", s);
+        Assert.assertEquals(639479525L, crc32.getValue());
+        InputStream in1 = entity.getContent();
+        InputStream in2 = entity.getContent();
+        Assert.assertTrue(in1 != in2);
+    }
+
+    @Test
+    public void testStreaming() throws Exception {
+        CRC32 crc32 = new CRC32();
+        ByteArrayInputStream in = new ByteArrayInputStream("1234567890".getBytes("ASCII"));
+        InputStreamEntity wrapped = new InputStreamEntity(in, -1);
+        ChecksumEntity entity = new ChecksumEntity(wrapped, crc32);
+        Assert.assertTrue(entity.isStreaming());
+        String s = EntityUtils.toString(entity);
+        Assert.assertEquals("1234567890", s);
+        Assert.assertEquals(639479525L, crc32.getValue());
+        InputStream in1 = entity.getContent();
+        InputStream in2 = entity.getContent();
+        Assert.assertTrue(in1 == in2);
+        EntityUtils.consume(entity);
+        EntityUtils.consume(entity);
+    }
+
+    @Test
+    public void testWriteToStream() throws Exception {
+        CRC32 crc32 = new CRC32();
+        StringEntity wrapped = new StringEntity("1234567890", "ASCII");
+        ChecksumEntity entity = new ChecksumEntity(wrapped, crc32);
+        Assert.assertFalse(entity.isStreaming());
+
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        entity.writeTo(out);
+
+        String s = new String(out.toByteArray(), "ASCII");
+        Assert.assertEquals("1234567890", s);
+        Assert.assertEquals(639479525L, crc32.getValue());
+    }
+
+    static class ChecksumEntity extends DecompressingEntity {
+
+        private final Checksum checksum;
+
+        public ChecksumEntity(final HttpEntity wrapped, final Checksum checksum) {
+            super(wrapped);
+            this.checksum = checksum;
+        }
+
+        @Override
+        InputStream getDecompressingInputStream(final InputStream wrapped) throws IOException {
+            return new CheckedInputStream(wrapped, this.checksum);
+        }
+
+    }
+
+}

httpclient/src/test/java/org/apache/http/impl/client/TestContentCodings.java

@@ -313,6 +313,34 @@ public class TestContentCodings extends ServerTestBase {
         client.getConnectionManager().shutdown();
     }
 
+    @Test
+    public void gzipResponsesWorkWithBasicResponseHandler() throws Exception {
+        final String entityText = "Hello, this is some plain text coming back.";
+
+        this.localServer.register("*", createGzipEncodingRequestHandler(entityText));
+
+        DefaultHttpClient client = createHttpClient();
+        HttpGet request = new HttpGet("/some-resource");
+        String response = client.execute(getServerHttp(), request, new BasicResponseHandler());
+        Assert.assertEquals("The entity text is correctly transported", entityText, response);
+
+        client.getConnectionManager().shutdown();
+    }
+
+    @Test
+    public void deflateResponsesWorkWithBasicResponseHandler() throws Exception {
+        final String entityText = "Hello, this is some plain text coming back.";
+
+        this.localServer.register("*", createDeflateEncodingRequestHandler(entityText, false));
+
+        DefaultHttpClient client = createHttpClient();
+        HttpGet request = new HttpGet("/some-resource");
+        String response = client.execute(getServerHttp(), request, new BasicResponseHandler());
+        Assert.assertEquals("The entity text is correctly transported", entityText, response);
+
+        client.getConnectionManager().shutdown();
+    }
+
     /**
      * Creates a new {@link HttpRequestHandler} that will attempt to provide a deflate stream
      * Content-Coding.