From bf27275eb2421a2ac8dc79799f9d0f2232b29197 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Wed, 29 Dec 2010 03:00:46 +0000 Subject: [PATCH] More NPOIFS BAT vs XBAT confusion fixes. Also fixes recent POIFS regression on big files, and adds a POIFS unit test for XBAT containing files (previously there wasn't one) git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1053511 13f79535-47bb-0310-9956-ffa450edef68 --- .../poifs/filesystem/NPOIFSFileSystem.java | 6 +- .../apache/poi/poifs/storage/BATBlock.java | 29 +++---- .../storage/BlockAllocationTableReader.java | 26 ++++--- .../apache/poi/poifs/storage/HeaderBlock.java | 13 +--- src/java/org/apache/poi/util/IntList.java | 4 +- .../poifs/filesystem/TestPOIFSFileSystem.java | 76 +++++++++++++++++-- .../poi/poifs/storage/TestBATBlock.java | 33 ++++---- .../TestBlockAllocationTableReader.java | 6 +- 8 files changed, 124 insertions(+), 69 deletions(-) diff --git a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java index 6cc5730164..b506e4b9aa 100644 --- a/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java +++ b/src/java/org/apache/poi/poifs/filesystem/NPOIFSFileSystem.java @@ -45,6 +45,7 @@ import org.apache.poi.poifs.nio.FileBackedDataSource; import org.apache.poi.poifs.property.DirectoryProperty; import org.apache.poi.poifs.property.NPropertyTable; import org.apache.poi.poifs.storage.BATBlock; +import org.apache.poi.poifs.storage.BlockAllocationTableReader; import org.apache.poi.poifs.storage.BlockAllocationTableWriter; import org.apache.poi.poifs.storage.HeaderBlock; import org.apache.poi.poifs.storage.HeaderBlockConstants; @@ -179,6 +180,9 @@ public class NPOIFSFileSystem extends BlockStore // Have the header processed _header = new HeaderBlock(headerBuffer); + + // Sanity check the block count + BlockAllocationTableReader.sanityCheckBlockCount(_header.getBATCount()); // We need to buffer the whole file into memory when // working with an InputStream. @@ -455,8 +459,8 @@ public class NPOIFSFileSystem extends BlockStore System.arraycopy(_header.getBATArray(), 0, newBATs, 0, newBATs.length-1); newBATs[newBATs.length-1] = offset; _header.setBATArray(newBATs); - _header.setBATCount(newBATs.length); } + _header.setBATCount(_bat_blocks.size()); // The current offset stores us, but the next one is free return offset+1; diff --git a/src/java/org/apache/poi/poifs/storage/BATBlock.java b/src/java/org/apache/poi/poifs/storage/BATBlock.java index 3319839f16..661cd12dc6 100644 --- a/src/java/org/apache/poi/poifs/storage/BATBlock.java +++ b/src/java/org/apache/poi/poifs/storage/BATBlock.java @@ -233,39 +233,30 @@ public final class BATBlock extends BigBlock { /** * Calculates the maximum size of a file which is addressable given the - * number of FAT (BAT and XBAT) sectors specified. + * number of FAT (BAT) sectors specified. (We don't care if those BAT + * blocks come from the 109 in the header, or from header + XBATS, it + * won't affect the calculation) * - * For files with 109 or fewer BATs: - * The actual file size will be between [size of fatCount-1 blocks] and + * The actual file size will be between [size of fatCount-1 blocks] and * [size of fatCount blocks]. * For 512 byte block sizes, this means we may over-estimate by up to 65kb. * For 4096 byte block sizes, this means we may over-estimate by up to 4mb - * - * For files with more than 109 BATs (i.e. has XBATs): - * Each XBAT can hold 127/1023 BATs, which in turn address 128/1024 blocks. - * For 512 byte block sizes, this means we may over-estimate by up to 8mb - * For 4096 byte block sizes, this means we may over-estimate by up to 4gb, - * but only for files of more than 436mb in size */ public static int calculateMaximumSize(final POIFSBigBlockSize bigBlockSize, - final int numBAT, final int numXBAT) { + final int numBATs) { int size = 1; // Header isn't FAT addressed - // The header contains up to 109 BATs, each of which can - // address 128/1024 blocks - size += (numBAT * bigBlockSize.getBATEntriesPerBlock()); - - // Each XBAT holds up to 127/1024 BATs, each of which can - // address 128/1024 blocks - size += (numXBAT * bigBlockSize.getXBATEntriesPerBlock() * - bigBlockSize.getBATEntriesPerBlock()); + // The header has up to 109 BATs, and extra ones are referenced + // from XBATs + // However, all BATs can contain 128/1024 blocks + size += (numBATs * bigBlockSize.getBATEntriesPerBlock()); // So far we've been in sector counts, turn into bytes return size * bigBlockSize.getBigBlockSize(); } public static int calculateMaximumSize(final HeaderBlock header) { - return calculateMaximumSize(header.getBigBlockSize(), header.getBATCount(), header.getXBATCount()); + return calculateMaximumSize(header.getBigBlockSize(), header.getBATCount()); } /** diff --git a/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java b/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java index f2c5112007..0d1b86dd4f 100644 --- a/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java +++ b/src/java/org/apache/poi/poifs/storage/BlockAllocationTableReader.java @@ -81,16 +81,7 @@ public final class BlockAllocationTableReader { int xbat_count, int xbat_index, BlockList raw_block_list) throws IOException { this(bigBlockSize); - if (block_count <= 0) { - throw new IOException( - "Illegal block count; minimum count is 1, got " + block_count - + " instead"); - } - - if (block_count > MAX_BLOCK_COUNT) { - throw new IOException("Block count " + block_count - + " is too high. POI maximum is " + MAX_BLOCK_COUNT + "."); - } + sanityCheckBlockCount(block_count); // We want to get the whole of the FAT table // To do this: @@ -186,6 +177,21 @@ public final class BlockAllocationTableReader { this.bigBlockSize = bigBlockSize; _entries = new IntList(); } + + public static void sanityCheckBlockCount(int block_count) throws IOException { + if (block_count <= 0) { + throw new IOException( + "Illegal block count; minimum count is 1, got " + + block_count + " instead" + ); + } + if (block_count > MAX_BLOCK_COUNT) { + throw new IOException( + "Block count " + block_count + + " is too high. POI maximum is " + MAX_BLOCK_COUNT + "." + ); + } + } /** * walk the entries from a specified point and return the diff --git a/src/java/org/apache/poi/poifs/storage/HeaderBlock.java b/src/java/org/apache/poi/poifs/storage/HeaderBlock.java index d9098749ee..5b9e071d6c 100644 --- a/src/java/org/apache/poi/poifs/storage/HeaderBlock.java +++ b/src/java/org/apache/poi/poifs/storage/HeaderBlock.java @@ -50,8 +50,8 @@ public final class HeaderBlock implements HeaderBlockConstants { private final POIFSBigBlockSize bigBlockSize; /** - * number of big block allocation table blocks (int). - * (Number of FAT Sectors in Microsoft parlance) + * Number of big block allocation table blocks (int). + * (Number of FAT Sectors in Microsoft parlance). */ private int _bat_count; @@ -159,13 +159,6 @@ public final class HeaderBlock implements HeaderBlockConstants { _sbat_count = new IntegerField(_sbat_block_count_offset, _data).get(); _xbat_start = new IntegerField(_xbat_start_offset, _data).get(); _xbat_count = new IntegerField(_xbat_count_offset, _data).get(); - - // Sanity check values - if(_bat_count > _max_bats_in_header) { - _logger.log(POILogger.WARN, "Too many BAT blocks listed in header, found " - + _bat_count + " but the maximum is " + _max_bats_in_header); - _bat_count = _max_bats_in_header; - } } /** @@ -306,7 +299,7 @@ public final class HeaderBlock implements HeaderBlockConstants { // Read them in int[] result = new int[ Math.min(_bat_count,_max_bats_in_header) ]; int offset = _bat_array_offset; - for (int j = 0; j < _bat_count; j++) { + for (int j = 0; j < result.length; j++) { result[ j ] = LittleEndian.getInt(_data, offset); offset += LittleEndianConsts.INT_SIZE; } diff --git a/src/java/org/apache/poi/util/IntList.java b/src/java/org/apache/poi/util/IntList.java index 3fee8dd572..08f024082e 100644 --- a/src/java/org/apache/poi/util/IntList.java +++ b/src/java/org/apache/poi/util/IntList.java @@ -343,7 +343,9 @@ public class IntList { if (index >= _limit) { - throw new IndexOutOfBoundsException(); + throw new IndexOutOfBoundsException( + index + " not accessible in a list of length " + _limit + ); } return _array[ index ]; } diff --git a/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java b/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java index 0389a18a1b..aa610bce65 100644 --- a/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java +++ b/src/testcases/org/apache/poi/poifs/filesystem/TestPOIFSFileSystem.java @@ -17,11 +17,11 @@ package org.apache.poi.poifs.filesystem; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.ByteBuffer; import java.util.Iterator; import junit.framework.TestCase; @@ -29,6 +29,9 @@ import junit.framework.TestCase; import org.apache.poi.POIDataSamples; import org.apache.poi.hssf.HSSFTestDataSamples; import org.apache.poi.poifs.common.POIFSBigBlockSize; +import org.apache.poi.poifs.common.POIFSConstants; +import org.apache.poi.poifs.storage.BATBlock; +import org.apache.poi.poifs.storage.BlockAllocationTableReader; import org.apache.poi.poifs.storage.HeaderBlock; import org.apache.poi.poifs.storage.RawDataBlockList; @@ -38,6 +41,8 @@ import org.apache.poi.poifs.storage.RawDataBlockList; * @author Josh Micich */ public final class TestPOIFSFileSystem extends TestCase { + private POIDataSamples _samples = POIDataSamples.getPOIFSInstance(); + /** * Mock exception used to ensure correct error handling @@ -98,7 +103,6 @@ public final class TestPOIFSFileSystem extends TestCase { * POIFSFileSystem was not closing the input stream. */ public void testAlwaysClose() { - TestIS testIS; // Normal case - read until EOF and close @@ -139,9 +143,7 @@ public final class TestPOIFSFileSystem extends TestCase { "ShortLastBlock.qwp", "ShortLastBlock.wps" }; - POIDataSamples _samples = POIDataSamples.getPOIFSInstance(); for(int i=0; i