Ensure we read streams fully

Read method makes an assumption that the stream will return the
amount of data specified by the length parameter which isn't the case for
streams.

The length parameter indicates how much it may read up to. A stream will
block until it has read at least one byte, after that it may return at
any point indicating how much it actually read.

Also fixing a bug in the EntryInputStream which made the same assumption
that the number of bytes read will always be the same as the length
requested.

Author: Ciaran Evans

loop-fs-iso-impl/src/main/java/com/github/stephenc/javaisotools/loopfs/iso9660/EntryInputStream.java

@@ -1,17 +1,17 @@
 /*
  * Copyright (c) 2010. Stephen Connolly.
  * Copyright (c) 2006-2007. loopy project (http://loopy.sourceforge.net).
- *  
+ *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
  * License as published by the Free Software Foundation; either
  * version 2.1 of the License, or (at your option) any later version.
- *  
+ *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Lesser General Public License for more details.
- *  
+ *
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
@@ -72,8 +72,8 @@ public int read(final byte b[], final int off, final int len) throws IOException
         }
 
         if (read > 0) {
-            this.pos += len;
-            this.rem -= len;
+            this.pos += read;
+            this.rem -= read;
         }
 
         return read;
@@ -127,4 +127,4 @@ private void ensureOpen() {
             throw new IllegalStateException("stream has been closed");
         }
     }
-}
+}

loop-fs-iso-impl/src/test/java/com/github/stephenc/javaisotools/loopfs/iso9660/Iso9660FileSystemTest.java

@@ -1,39 +1,42 @@
 /*
  * Copyright (c) 2010. Stephen Connolly.
- *  
+ *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
  * License as published by the Free Software Foundation; either
  * version 2.1 of the License, or (at your option) any later version.
- *  
+ *
  * This library is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  * Lesser General Public License for more details.
- *  
+ *
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
  */
 
 package com.github.stephenc.javaisotools.loopfs.iso9660;
 
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.InputStream;
-import java.util.Properties;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assume.assumeTrue;
 
+import com.github.stephenc.javaisotools.loopfs.spi.SeekableInputFile;
 import com.github.stephenc.javaisotools.loopfs.spi.SeekableInputFileHadoop;
+import com.google.common.collect.Iterables;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
-import org.junit.*;
-
 import org.codehaus.plexus.util.IOUtil;
+import org.junit.BeforeClass;
+import org.junit.Test;
 
-import static org.hamcrest.CoreMatchers.*;
-import static org.junit.Assert.*;
-import static org.junit.Assume.assumeTrue;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.Properties;
 
 /**
  * Tests the Iso9660 implementation.
@@ -65,13 +68,25 @@ public void smokes() throws Exception {
         this.runCheck(image);
     }
 
+    @Test
+    public void shouldReadAllBytesWhenSeekableInputPartiallyReads() throws IOException {
+        // Create seekeable input which does not read up to specified length
+        SeekableInputFile input = new PartiallyReadSeekableInput();
+        Iso9660FileSystem fs = new Iso9660FileSystem(input, true);
+        Iso9660FileEntry entry = Iterables.getLast(fs);
+
+        byte[] bytes = fs.getBytes(entry);
+
+        assertThat("All bytes should have been read", new String(bytes), is("Goodbye"));
+    }
+
     @Test
     public void hdfsSmokes() throws Exception {
         assumeTrue(isNotWindows());
         //Creating a Mini DFS Cluster as the default File System does not return a Seekable Stream
         MiniDFSCluster.Builder builder = new MiniDFSCluster.Builder(new Configuration());
         MiniDFSCluster hdfsCluster = builder.build();
-        String hdfsTestFile = "hdfs://127.0.0.1:"+ hdfsCluster.getNameNodePort() + "/test/" + filePath;
+        String hdfsTestFile = "hdfs://127.0.0.1:" + hdfsCluster.getNameNodePort() + "/test/" + filePath;
         hdfsCluster.getFileSystem()
                 .copyFromLocalFile(new Path(filePath), new Path(hdfsTestFile));
         InputStream is = hdfsCluster.getFileSystem().open(new Path(hdfsTestFile));
@@ -97,4 +112,22 @@ private boolean isNotWindows() {
         String os = System.getProperty("os.name");
         return !os.startsWith("Windows");
     }
+
+    private static class PartiallyReadSeekableInput extends SeekableInputFile {
+
+        private byte[] bytes;
+
+        public PartiallyReadSeekableInput() throws IOException {
+            super(new File(filePath));
+        }
+
+        @Override
+        public int read(byte[] b, int off, int len) throws IOException {
+            // Deliberately miss last byte on first pass
+            boolean firstPass = b != bytes;
+            int length = firstPass ? len - 1 : len;
+            bytes = b;
+            return super.read(b, off, length);
+        }
+    }
 }

loop-fs-spi/src/main/java/com/github/stephenc/javaisotools/loopfs/spi/AbstractFileSystem.java

@@ -85,7 +85,21 @@ protected final void seek(long pos) throws IOException {
      */
     protected final int read(byte[] buffer, int offset, int length) throws IOException {
         ensureOpen();
-        return this.channel.read(buffer, offset, length);
+        return readFully(buffer, offset, length);
+    }
+
+    private int readFully(byte[] buffer, int offset, int length) throws IOException {
+        int bytesRead;
+        int remaining = length;
+
+        // read doesn't guarantee a full buffer is read. Reading until we have a full buffer or end of stream
+        while (remaining != 0 &&
+                (bytesRead = this.channel.read(buffer, offset, remaining)) != -1) {
+            offset += bytesRead;
+            remaining -= bytesRead;
+        }
+        int totalBytesRead = length - remaining;
+        return totalBytesRead;
     }
 
 }