[GitHub] [solr] madrob commented on a change in pull request #283: SOLR-14457: Fix closing of malformed GZIPInputStream

[GitBox <gi...@apache.org>]

madrob commented on a change in pull request #283:
URL: https://github.com/apache/solr/pull/283#discussion_r701264180



##########
File path: solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClientUtilTest.java
##########
@@ -109,4 +111,21 @@ public void testToBooleanObject() throws Exception {
     assertEquals(null, HttpClientUtil.toBooleanObject("foo"));
     assertEquals(null, HttpClientUtil.toBooleanObject(null));
   }
+
+  @Test
+  public void testNonRepeatableMalformedGzipEntityAutoClosed() throws IOException {
+    HttpEntity baseEntity = new InputStreamEntity(new ByteArrayInputStream("this is not compressed".getBytes(StandardCharsets.UTF_8)));
+    HttpClientUtil.GzipDecompressingEntity gzipDecompressingEntity = new HttpClientUtil.GzipDecompressingEntity(baseEntity);
+    expectThrows(IOException.class, "An IOException wrapping a ZIPException should be thrown when loading a malformed GZIP Entity Content", gzipDecompressingEntity::getContent);

Review comment:
       Save the exception and make an assertion about the cause?

##########
File path: solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java
##########
@@ -447,14 +448,39 @@ public void process(final HttpResponse response, final HttpContext context)
     }
   }
   
-  private static class GzipDecompressingEntity extends HttpEntityWrapper {
+  protected static class GzipDecompressingEntity extends HttpEntityWrapper {
+    private boolean gzipInputStreamCreated = false;
+    private InputStream gzipInputStream = null;
+
     public GzipDecompressingEntity(final HttpEntity entity) {
       super(entity);
     }
-    
+
+    /**
+     * Return a InputStream of uncompressed data.
+     * If there is an issue with the compression of the data, a null InputStream will be returned,
+     * and the underlying compressed InputStream will be closed.
+     *
+     * The same input stream will be returned if the underlying entity is not repeatable.
+     * If the underlying entity is repeatable, then a new input stream will be created.

Review comment:
       do we have a test that covers this? what is the use case for repeatable entities?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org