Tweak comments, layout and exceptions in IOUtils and RawDataBlock. It should now be clearer exactly what they do, and when they become unhappy. Also include a test that ensures that when reading from a slow inputstream (as per bug #42834), we really are ok with the data dribbling in, and do not require it to all come in in blocksize chunks at one

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

src/java/org/apache/poi/poifs/storage/RawDataBlock.java

@@ -42,34 +42,44 @@ public class RawDataBlock
      * @param stream the InputStream from which the data will be read
      *
      * @exception IOException on I/O errors, and if an insufficient
-     *            amount of data is read
+     *            amount of data is read (the InputStream must
+     *            be an exact multiple of the block size)
      */
-
     public RawDataBlock(final InputStream stream)
-        throws IOException
-    {
-        _data = new byte[ POIFSConstants.BIG_BLOCK_SIZE ];
+    		throws IOException {
+    	this(stream, POIFSConstants.BIG_BLOCK_SIZE);
+    }
+    /**
+     * Constructor RawDataBlock
+     *
+     * @param stream the InputStream from which the data will be read
+     * @param blockSize the size of the POIFS blocks, normally 512 bytes {@link POIFSConstants#BIG_BLOCK_SIZE}
+     *
+     * @exception IOException on I/O errors, and if an insufficient
+     *            amount of data is read (the InputStream must
+     *            be an exact multiple of the block size)
+     */
+    public RawDataBlock(final InputStream stream, int blockSize)
+    		throws IOException {
+        _data = new byte[ blockSize ];
         int count = IOUtils.readFully(stream, _data);
 
-        if (count == -1)
-        {
+        if (count == -1) {
             _eof = true;
         }
-        else if (count != POIFSConstants.BIG_BLOCK_SIZE)
-        {
-        	if (count == -1)
-        		//Cant have -1 bytes read in the error message!
-        		count = 0;
-        	
+        else if (count != blockSize) {
+        	// IOUtils.readFully will always read the
+        	//  requested number of bytes, unless it hits
+        	//  an EOF
+            _eof = true;
             String type = " byte" + ((count == 1) ? ("")
                                                   : ("s"));
 
             throw new IOException("Unable to read entire block; " + count
-                                  + type + " read; expected "
-                                  + POIFSConstants.BIG_BLOCK_SIZE + " bytes");
+                                  + type + " read before EOF; expected "
+                                  + blockSize + " bytes");
         }
-        else
-        {
+        else {
             _eof = false;
         }
     }
@@ -82,7 +92,6 @@ public class RawDataBlock
      *
      * @exception IOException
      */
-
     public boolean eof()
         throws IOException
     {
@@ -98,7 +107,6 @@ public class RawDataBlock
      *
      * @exception IOException if there is no data
      */
-
     public byte [] getData()
         throws IOException
     {

src/java/org/apache/poi/util/IOUtils.java

@@ -58,11 +58,16 @@ public class IOUtils
     }
 
     /**
-     * Same as the normal <tt>in.read(b, off, len)</tt>, but tries to ensure that
-     * the entire len number of bytes is read.
+     * Same as the normal <tt>in.read(b, off, len)</tt>, but
+     *  tries to ensure that the entire len number of bytes
+     *  is read.
      * <p>
-     * If the end of file is reached before any bytes are read, returns -1.
-     * Otherwise, returns the number of bytes read.
+     * If the end of file is reached before any bytes
+     *  are read, returns -1.
+     * If the end of the file is reached after some bytes are
+     *  read, returns the number of bytes read.
+     * If the end of the file isn't reached before len
+     *  bytes have been read, will return len bytes.
      */
     public static int readFully(InputStream in, byte[] b, int off, int len)
     throws IOException
@@ -79,5 +84,4 @@ public class IOUtils
             }
         }
     }
-}
-
+}

src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java

@@ -20,6 +20,7 @@
 package org.apache.poi.poifs.storage;
 
 import java.io.*;
+import java.util.Random;
 
 import junit.framework.*;
 
@@ -124,6 +125,118 @@ public class TestRawDataBlock
             }
         }
     }
+    
+    /**
+     * Tests that when using a slow input stream, which
+     *  won't return a full block at a time, we don't
+     *  incorrectly think that there's not enough data
+     */
+    public void testSlowInputStream() throws Exception {
+        for (int k = 1; k < 512; k++) {
+            byte[] data = new byte[ 512 ];
+            for (int j = 0; j < data.length; j++) {
+                data[j] = (byte) j;
+            }
+            
+            // Shouldn't complain, as there is enough data,
+            //  even if it dribbles through
+            RawDataBlock block = 
+            	new RawDataBlock(new SlowInputStream(data, k));
+            assertFalse(block.eof());
+        }
+        
+        // But if there wasn't enough data available, will
+        //  complain
+        for (int k = 1; k < 512; k++) {
+            byte[] data = new byte[ 511 ];
+            for (int j = 0; j < data.length; j++) {
+                data[j] = (byte) j;
+            }
+            
+            // Shouldn't complain, as there is enough data
+            try {
+	            RawDataBlock block = 
+	            	new RawDataBlock(new SlowInputStream(data, k));
+	            fail();
+            } catch(IOException e) {
+            	// as expected
+            }
+        }
+    }
+    
+    /**
+     * An input stream which will return a maximum of
+     *  a given number of bytes to read, and often claims
+     *  not to have any data
+     */
+    public static class SlowInputStream extends InputStream {
+    	private Random rnd = new Random();
+    	private byte[] data;
+    	private int chunkSize;
+    	private int pos = 0;
+    	
+    	public SlowInputStream(byte[] data, int chunkSize) {
+    		this.chunkSize = chunkSize;
+    		this.data = data;
+    	}
+    	
+    	/**
+    	 * 75% of the time, claim there's no data available
+    	 */
+    	private boolean claimNoData() {
+    		if(rnd.nextFloat() < 0.25f) {
+    			return false;
+    		}
+    		return true;
+    	}
+    	
+		public int read() throws IOException {
+			if(pos >= data.length) {
+				return -1;
+			}
+			int ret = data[pos];
+			pos++;
+			
+			if(ret < 0) ret += 256;
+			return ret;
+		}
+
+		/**
+		 * Reads the requested number of bytes, or the chunk
+		 *  size, whichever is lower.
+		 * Quite often will simply claim to have no data
+		 */
+		public int read(byte[] b, int off, int len) throws IOException {
+			// Keep the length within the chunk size
+			if(len > chunkSize) {
+				len = chunkSize;
+			}
+			// Don't read off the end of the data
+			if(pos + len > data.length) {
+				len = data.length - pos;
+				
+				// Spot when we're out of data
+				if(len == 0) {
+					return -1;
+				}
+			}
+			
+			// 75% of the time, claim there's no data
+			if(claimNoData()) {
+				return 0;
+			}
+			
+			// Copy, and return what we read
+			System.arraycopy(data, pos, b, off, len);
+			pos += len;
+			return len;
+		}
+
+		public int read(byte[] b) throws IOException {
+			return read(b, 0, b.length);
+		}
+		
+    }
 
     /**
      * main method to run the unit tests