[commons-compress] branch master updated: Removed incorrect use of InputStream.available() in ArArchiveInputStream

[bo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bodewig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git


The following commit(s) were added to refs/heads/master by this push:
     new 8e5338c  Removed incorrect use of InputStream.available() in ArArchiveInputStream
     new 575f0eb  Merge pull request #74 from akbertram/patch-1
8e5338c is described below

commit 8e5338cd2d6edc347cfe2bcd72f15bf23b178f5b
Author: Alex Bertram <al...@bedatadriven.com>
AuthorDate: Mon Feb 11 20:22:48 2019 +0100

    Removed incorrect use of InputStream.available() in ArArchiveInputStream
    
    The original code appears to be checking for end-of-file
    using the InputStream.available() method.
    
    This however, misunderstands the InputStream API. The available()
    method only returns an estimate, and cannot be used
    to check for the remaining bytes in the file. From the documentation:
    
    > Returns an estimate of the number of bytes that can be read (or
    > skipped over) from this input stream without blocking by the next
    > invocation of a method for this input stream. The next invocation
    > might be the same thread or another thread.  A single read or skip of this
    > many bytes will not block, but may read or skip fewer bytes.
    > Note that while some implementations of InputStream will return
    > the total number of bytes in the stream, many will not.  It is
    > never correct to use the return value of this method to allocate
    > a buffer intended to hold all data in this stream.
    
    This patch includes a unit test that demonstrates the bug and
    verifies the fix.
---
 .../archivers/ar/ArArchiveInputStream.java         |  7 ++---
 .../archivers/ar/ArArchiveInputStreamTest.java     | 32 ++++++++++++++++++++--
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
index 206d388..38d13d9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
@@ -121,13 +121,12 @@ public class ArArchiveInputStream extends ArchiveInputStream {
             trackReadBytes(1);
         }
 
-        if (input.available() == 0) {
-            return null;
-        }
-
         {
             final int read = IOUtils.readFully(input, metaData);
             trackReadBytes(read);
+            if (read == 0) {
+                return null;
+            }
             if (read < metaData.length) {
                 throw new IOException("truncated ar archive");
             }
diff --git a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
index e665ff9..14c4315 100644
--- a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
@@ -18,10 +18,10 @@
 
 package org.apache.commons.compress.archivers.ar;
 
+import static org.hamcrest.CoreMatchers.*;
 import static org.junit.Assert.*;
 
-import java.io.BufferedInputStream;
-import java.io.FileInputStream;
+import java.io.*;
 
 import org.apache.commons.compress.AbstractTestCase;
 import org.apache.commons.compress.archivers.ArchiveEntry;
@@ -83,6 +83,34 @@ public class ArArchiveInputStreamTest extends AbstractTestCase {
         }
     }
 
+    @Test
+    public void simpleInputStream() throws IOException {
+        try (final FileInputStream fileInputStream = new FileInputStream(getFile("bla.ar"))) {
+
+            // This default implementation of InputStream.available() always returns zero,
+            // and there are many streams in practice where the total length of the stream is not known.
+
+            final InputStream simpleInputStream = new InputStream() {
+                @Override
+                public int read() throws IOException {
+                    return fileInputStream.read();
+                }
+            };
+
+            ArArchiveInputStream archiveInputStream = new ArArchiveInputStream(simpleInputStream);
+            ArArchiveEntry entry1 = archiveInputStream.getNextArEntry();
+            assertThat(entry1, not(nullValue()));
+            assertThat(entry1.getName(), equalTo("test1.xml"));
+            assertThat(entry1.getLength(), equalTo(610L));
+
+            ArArchiveEntry entry2 = archiveInputStream.getNextArEntry();
+            assertThat(entry2.getName(), equalTo("test2.xml"));
+            assertThat(entry2.getLength(), equalTo(82L));
+
+            assertThat(archiveInputStream.getNextArEntry(), nullValue());
+        }
+    }
+
     @Test(expected=IllegalStateException.class)
     public void cantReadWithoutOpeningAnEntry() throws Exception {
         try (FileInputStream in = new FileInputStream(getFile("bla.ar"));