More NPOIFSFileSystem and NPOIFSStream read unit tests, along with details of a few...

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

diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
index 28147433636857057768f29545bca014b65be30d..9a3aed8c9fb2a2cb06248ab98a054403e939785c 100644 (file)
--- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
+++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java
@@ -653,12 +653,21 @@ public class NPOIFSFileSystem
      *  we can bail out with an error rather than
      *  spinning away for ever... 
      */
-    private class ChainLoopDetector {
+    protected class ChainLoopDetector {
        private boolean[] used_blocks;
-       private ChainLoopDetector() throws IOException {
-          used_blocks = new boolean[(int)(_data.size()/bigBlockSize.getBigBlockSize())];
+       protected ChainLoopDetector() throws IOException {
+          int numBlocks = (int)Math.ceil(_data.size()/bigBlockSize.getBigBlockSize());
+          used_blocks = new boolean[numBlocks];
        }
-       private void claim(int offset) {
+       protected void claim(int offset) {
+          if(offset >= used_blocks.length) {
+             // They're writing, and have had new blocks requested
+             //  for the write to proceed. That means they're into
+             //  blocks we've allocated for them, so are safe
+             return;
+          }
+          
+          // Claiming an existing block, ensure there's no loop
           if(used_blocks[offset]) {
              throw new IllegalStateException(
                    "Potential loop detected - Block " + offset + 

diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSStream.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSStream.java
index ac9f918dfc6961eee1ee34d5a65a2de32f1c1eea..1739532631c53f0f3ca5ed96cc999e1277e0df65 100644 (file)
--- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSStream.java
+++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSStream.java
@@ -24,6 +24,7 @@ import java.nio.ByteBuffer;
 import java.util.Iterator;
 
 import org.apache.poi.poifs.common.POIFSConstants;
+import org.apache.poi.poifs.filesystem.NPOIFSFileSystem.ChainLoopDetector;
 import org.apache.poi.poifs.property.Property;
 import org.apache.poi.poifs.storage.HeaderBlock;
 
@@ -38,6 +39,8 @@ import org.apache.poi.poifs.storage.HeaderBlock;
  * This only works on big block streams, it doesn't
  *  handle small block ones.
  * This uses the new NIO code
+ * 
+ * TODO Add loop checking on read and on write
  */
 
 public class NPOIFSStream implements Iterable<ByteBuffer>
@@ -100,11 +103,16 @@ public class NPOIFSStream implements Iterable<ByteBuffer>
       // How many blocks are we going to need?
       int blocks = (int)Math.ceil(contents.length / filesystem.getBigBlockSize());
       
+      // Make sure we don't encounter a loop whilst overwriting
+      //  the existing blocks
+      ChainLoopDetector loopDetector = filesystem.new ChainLoopDetector();
+      
       // Start writing
       int prevBlock = POIFSConstants.END_OF_CHAIN;
       int nextBlock = startBlock;
       for(int i=0; i<blocks; i++) {
          int thisBlock = nextBlock;
+         loopDetector.claim(thisBlock);
          
          // Allocate a block if needed, otherwise figure
          //  out what the next block will be
@@ -128,6 +136,9 @@ public class NPOIFSStream implements Iterable<ByteBuffer>
          prevBlock = thisBlock;
       }
       
+      // If we're overwriting, free any remaining blocks
+      // TODO
+      
       // Mark the end of the stream
       filesystem.setNextBlock(nextBlock, POIFSConstants.END_OF_CHAIN);
    }
@@ -138,9 +149,16 @@ public class NPOIFSStream implements Iterable<ByteBuffer>
     * Class that handles a streaming read of one stream
     */
    protected class StreamBlockByteBufferIterator implements Iterator<ByteBuffer> {
+      private ChainLoopDetector loopDetector;
       private int nextBlock;
+      
       protected StreamBlockByteBufferIterator(int firstBlock) {
-         nextBlock = firstBlock;
+         this.nextBlock = firstBlock;
+         try {
+            this.loopDetector = filesystem.new ChainLoopDetector();
+         } catch(IOException e) {
+            throw new RuntimeException(e);
+         }
       }
 
       public boolean hasNext() {
@@ -156,6 +174,7 @@ public class NPOIFSStream implements Iterable<ByteBuffer>
          }
          
          try {
+            loopDetector.claim(nextBlock);
             ByteBuffer data = filesystem.getBlockAt(nextBlock);
             nextBlock = filesystem.getNextBlock(nextBlock);
             return data;

diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
index 62ce21a890127579f1eeeab77bbb8e8f8332e6a1..c511df3741dcc3f3f25ab43c19ee07981a600f90 100644 (file)
--- a/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSFileSystem.java
@@ -17,6 +17,8 @@
 
 package org.apache.poi.poifs.filesystem;
 
+import java.nio.ByteBuffer;
+
 import junit.framework.TestCase;
 
 import org.apache.poi.POIDataSamples;
@@ -116,6 +118,32 @@ public final class TestNPOIFSFileSystem extends TestCase {
     * Check we get the right data back for each block
     */
    public void testGetBlock() throws Exception {
-      // TODO
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      ByteBuffer b;
+      
+      // The 0th block is the first data block
+      b = fs.getBlockAt(0);
+      assertEquals((byte)0x9e, b.get());
+      assertEquals((byte)0x75, b.get());
+      assertEquals((byte)0x97, b.get());
+      assertEquals((byte)0xf6, b.get());
+      
+      // And the next block
+      b = fs.getBlockAt(1);
+      assertEquals((byte)0x86, b.get());
+      assertEquals((byte)0x09, b.get());
+      assertEquals((byte)0x22, b.get());
+      assertEquals((byte)0xfb, b.get());
+      
+      // Check the final block too
+      b = fs.getBlockAt(99);
+      assertEquals((byte)0x01, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x02, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
    }
 }

diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSStream.java b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSStream.java
new file mode 100644 (file)
index 0000000..ef792e7
--- /dev/null
+++ b/src/testcases/org/apache/poi/poifs/filesystem/TestNPOIFSStream.java
@@ -0,0 +1,281 @@
+/* ====================================================================
+   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.
+==================================================================== */
+
+package org.apache.poi.poifs.filesystem;
+
+import java.nio.ByteBuffer;
+import java.util.Iterator;
+
+import junit.framework.TestCase;
+
+import org.apache.poi.POIDataSamples;
+
+/**
+ * Tests {@link NPOIFSStream}
+ */
+public final class TestNPOIFSStream extends TestCase {
+   private static final POIDataSamples _inst = POIDataSamples.getPOIFSInstance();
+
+   /**
+    * Read a single block stream
+    */
+   public void testReadTinyStream() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+
+      // 98 is actually the last block in a two block stream...
+      NPOIFSStream stream = new NPOIFSStream(fs, 98);
+      Iterator<ByteBuffer> i = stream.getBlockIterator();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b = i.next();
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      
+      // Check the contents
+      assertEquals((byte)0x81, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x82, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+      assertEquals((byte)0x00, b.get());
+   }
+
+   /**
+    * Read a stream with only two blocks in it 
+    */
+   public void testReadShortStream() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // 97 -> 98 -> end
+      NPOIFSStream stream = new NPOIFSStream(fs, 97);
+      Iterator<ByteBuffer> i = stream.getBlockIterator();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b97 = i.next();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b98 = i.next();
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      
+      // Check the contents of the 1st block
+      assertEquals((byte)0x01, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      assertEquals((byte)0x02, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      assertEquals((byte)0x00, b97.get());
+      
+      // Check the contents of the 2nd block
+      assertEquals((byte)0x81, b98.get());
+      assertEquals((byte)0x00, b98.get());
+      assertEquals((byte)0x00, b98.get());
+      assertEquals((byte)0x00, b98.get());
+      assertEquals((byte)0x82, b98.get());
+      assertEquals((byte)0x00, b98.get());
+      assertEquals((byte)0x00, b98.get());
+      assertEquals((byte)0x00, b98.get());
+   }
+   
+   /**
+    * Read a stream with many blocks 
+    */
+   public void testReadLongerStream() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      ByteBuffer b0 = null;
+      ByteBuffer b1 = null;
+      ByteBuffer b22 = null;
+      
+      // The stream at 0 has 23 blocks in it
+      NPOIFSStream stream = new NPOIFSStream(fs, 0);
+      Iterator<ByteBuffer> i = stream.getBlockIterator();
+      int count = 0;
+      while(i.hasNext()) {
+         ByteBuffer b = i.next();
+         if(count == 0) {
+            b0 = b;
+         }
+         if(count == 1) {
+            b1 = b;
+         }
+         if(count == 22) {
+            b22 = b;
+         }
+         
+         count++;
+      }
+      assertEquals(23, count);
+      
+      // Check the contents
+      //  1st block is at 0
+      assertEquals((byte)0x9e, b0.get());
+      assertEquals((byte)0x75, b0.get());
+      assertEquals((byte)0x97, b0.get());
+      assertEquals((byte)0xf6, b0.get());
+            
+      //  2nd block is at 1
+      assertEquals((byte)0x86, b1.get());
+      assertEquals((byte)0x09, b1.get());
+      assertEquals((byte)0x22, b1.get());
+      assertEquals((byte)0xfb, b1.get());
+      
+      //  last block is at 89
+      assertEquals((byte)0xfe, b22.get());
+      assertEquals((byte)0xff, b22.get());
+      assertEquals((byte)0x00, b22.get());
+      assertEquals((byte)0x00, b22.get());
+      assertEquals((byte)0x05, b22.get());
+      assertEquals((byte)0x01, b22.get());
+      assertEquals((byte)0x02, b22.get());
+      assertEquals((byte)0x00, b22.get());
+   }
+
+   /**
+    * Read a stream with several blocks in a 4096 byte block file 
+    */
+   public void testReadStream4096() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize4096.zvi"));
+      
+      // 0 -> 1 -> 2 -> end
+      NPOIFSStream stream = new NPOIFSStream(fs, 0);
+      Iterator<ByteBuffer> i = stream.getBlockIterator();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b0 = i.next();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b1 = i.next();
+      assertEquals(true, i.hasNext());
+      assertEquals(true, i.hasNext());
+      ByteBuffer b2 = i.next();
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      assertEquals(false, i.hasNext());
+      
+      // Check the contents of the 1st block
+      assertEquals((byte)0x9E, b0.get());
+      assertEquals((byte)0x75, b0.get());
+      assertEquals((byte)0x97, b0.get());
+      assertEquals((byte)0xF6, b0.get());
+      assertEquals((byte)0xFF, b0.get());
+      assertEquals((byte)0x21, b0.get());
+      assertEquals((byte)0xD2, b0.get());
+      assertEquals((byte)0x11, b0.get());
+      
+      // Check the contents of the 2nd block
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x03, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      assertEquals((byte)0x00, b1.get());
+      
+      // Check the contents of the 3rd block
+      assertEquals((byte)0x6D, b2.get());
+      assertEquals((byte)0x00, b2.get());
+      assertEquals((byte)0x00, b2.get());
+      assertEquals((byte)0x00, b2.get());
+      assertEquals((byte)0x03, b2.get());
+      assertEquals((byte)0x00, b2.get());
+      assertEquals((byte)0x46, b2.get());
+      assertEquals((byte)0x00, b2.get());
+   }
+   
+   /**
+    * Craft a nasty file with a loop, and ensure we don't get stuck
+    */
+   public void testReadFailsOnLoop() throws Exception {
+      // TODO
+   }
+
+   /**
+    * Writing the same amount of data as before
+    */
+   public void testReplaceStream() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Writes less data than before, some blocks will need
+    *  to be freed
+    */
+   public void testReplaceStreamWithLess() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Writes more data than before, new blocks will be needed
+    */
+   public void testReplaceStreamWithMore() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Writes to a new stream in the file
+    */
+   public void testWriteNewStream() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Writes to a new stream in the file, where we've not enough
+    *  free blocks so new FAT segments will need to be allocated
+    *  to support this
+    */
+   public void testWriteNewStreamExtraFATs() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize512.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Replaces data in an existing stream, with a bit
+    *  more data than before, in a 4096 byte block file
+    */
+   public void testWriteStream4096() throws Exception {
+      NPOIFSFileSystem fs = new NPOIFSFileSystem(_inst.getFile("BlockSize4096.zvi"));
+      
+      // TODO
+   }
+   
+   /**
+    * Craft a nasty file with a loop, and ensure we don't get stuck
+    */
+   public void testWriteFailsOnLoop() throws Exception {
+      // TODO
+   }
+}