From 924975f0e9f7858b017d73e79dc3aeb98505fe09 Mon Sep 17 00:00:00 2001 From: Nick Burch Date: Thu, 6 Mar 2008 15:54:06 +0000 Subject: [PATCH] Change the behaviour on short last blocks to be a warning not an exception, as some people seem to have "real" valid files that trigger this. Fixed bug #28231 git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@634318 13f79535-47bb-0310-9956-ffa450edef68 --- src/documentation/content/xdocs/changes.xml | 1 + src/documentation/content/xdocs/status.xml | 3 + .../poi/poifs/storage/RawDataBlock.java | 12 ++- .../poi/poifs/storage/TestRawDataBlock.java | 87 ++++++++++++++----- .../poifs/storage/TestRawDataBlockList.java | 32 ++++--- .../org/apache/poi/util/DummyPOILogger.java | 46 ++++++++++ 6 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 src/testcases/org/apache/poi/util/DummyPOILogger.java diff --git a/src/documentation/content/xdocs/changes.xml b/src/documentation/content/xdocs/changes.xml index f49993944..99d81b0ae 100644 --- a/src/documentation/content/xdocs/changes.xml +++ b/src/documentation/content/xdocs/changes.xml @@ -36,6 +36,7 @@ + 28231 - For apparently truncated files, which are somehow still valid, now issue a truncation warning but carry on, rather than giving an exception as before 44504 - Added initial support for recognising external functions like YEARFRAC and ISEVEN (using NameXPtg), via LinkTable support 44504 - Improvements to FormulaParser - operators, precedence, error literals, quotes in string literals, range checking on IntPtg, formulas with extra un-parsed stuff at the end, improved parse error handling 44504 - Fixed number conversion inconsistencies in many functions, and improved RefEval diff --git a/src/documentation/content/xdocs/status.xml b/src/documentation/content/xdocs/status.xml index 082d30e6b..6fcbfee45 100644 --- a/src/documentation/content/xdocs/status.xml +++ b/src/documentation/content/xdocs/status.xml @@ -33,6 +33,9 @@ + 44504 - Added initial support for recognising external functions like YEARFRAC and ISEVEN (using NameXPtg), via LinkTable support + 44504 - Improvements to FormulaParser - operators, precedence, error literals, quotes in string literals, range checking on IntPtg, formulas with extra un-parsed stuff at the end, improved parse error handling + 44504 - Fixed number conversion inconsistencies in many functions, and improved RefEval 44504 - Added initial support for recognising external functions like YEARFRAC and ISEVEN (using NameXPtg), via LinkTable support 44504 - Improvements to FormulaParser - operators, precedence, error literals, quotes in string literals, range checking on IntPtg, formulas with extra un-parsed stuff at the end, improved parse error handling 44504 - Fixed number conversion inconsistencies in many functions, and improved RefEval diff --git a/src/java/org/apache/poi/poifs/storage/RawDataBlock.java b/src/java/org/apache/poi/poifs/storage/RawDataBlock.java index d55429840..472fd8b8b 100644 --- a/src/java/org/apache/poi/poifs/storage/RawDataBlock.java +++ b/src/java/org/apache/poi/poifs/storage/RawDataBlock.java @@ -21,6 +21,8 @@ package org.apache.poi.poifs.storage; import org.apache.poi.poifs.common.POIFSConstants; import org.apache.poi.util.IOUtils; +import org.apache.poi.util.POILogFactory; +import org.apache.poi.util.POILogger; import java.io.*; @@ -35,6 +37,7 @@ public class RawDataBlock { private byte[] _data; private boolean _eof; + private static POILogger log = POILogFactory.getLogger(RawDataBlock.class); /** * Constructor RawDataBlock @@ -75,9 +78,12 @@ public class RawDataBlock String type = " byte" + ((count == 1) ? ("") : ("s")); - throw new IOException("Unable to read entire block; " + count - + type + " read before EOF; expected " - + blockSize + " bytes"); + log.log(POILogger.ERROR, + "Unable to read entire block; " + count + + type + " read before EOF; expected " + + blockSize + " bytes. Your document" + + " has probably been truncated!" + ); } else { _eof = false; diff --git a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java index 1473fa82e..4c84f04b7 100644 --- a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java +++ b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlock.java @@ -22,6 +22,9 @@ package org.apache.poi.poifs.storage; import java.io.*; import java.util.Random; +import org.apache.poi.util.DummyPOILogger; +import org.apache.poi.util.POILogFactory; + import junit.framework.*; /** @@ -43,6 +46,13 @@ public class TestRawDataBlock public TestRawDataBlock(String name) { super(name); + + // We always want to use our own + // logger + System.setProperty( + "org.apache.poi.util.POILogger", + "org.apache.poi.util.DummyPOILogger" + ); } /** @@ -99,11 +109,19 @@ public class TestRawDataBlock /** * Test creating a short RawDataBlock + * Will trigger a warning, but no longer an IOException, + * as people seem to have "valid" truncated files */ - - public void testShortConstructor() + public void testShortConstructor() throws Exception { - for (int k = 1; k < 512; k++) + // Get the logger to be used + DummyPOILogger logger = (DummyPOILogger)POILogFactory.getLogger( + RawDataBlock.class + ); + assertEquals(0, logger.logged.size()); + + // Test for various data sizes + for (int k = 1; k <= 512; k++) { byte[] data = new byte[ k ]; @@ -112,16 +130,33 @@ public class TestRawDataBlock data[ j ] = ( byte ) j; } RawDataBlock block = null; - - try - { - block = new RawDataBlock(new ByteArrayInputStream(data)); - fail("Should have thrown IOException creating short block"); - } - catch (IOException ignored) - { - - // as expected + + logger.reset(); + assertEquals(0, logger.logged.size()); + + // Have it created + block = new RawDataBlock(new ByteArrayInputStream(data)); + assertNotNull(block); + + // Check for the warning is there for <512 + if(k < 512) { + assertEquals( + "Warning on " + k + " byte short block", + 1, logger.logged.size() + ); + + // Build the expected warning message, and check + String bts = k + " byte"; + if(k > 1) { + bts += "s"; + } + + assertEquals( + "7 - Unable to read entire block; "+bts+" read before EOF; expected 512 bytes. Your document has probably been truncated!", + (String)(logger.logged.get(0)) + ); + } else { + assertEquals(0, logger.logged.size()); } } } @@ -132,6 +167,13 @@ public class TestRawDataBlock * incorrectly think that there's not enough data */ public void testSlowInputStream() throws Exception { + // Get the logger to be used + DummyPOILogger logger = (DummyPOILogger)POILogFactory.getLogger( + RawDataBlock.class + ); + assertEquals(0, logger.logged.size()); + + // Test for various ok data sizes for (int k = 1; k < 512; k++) { byte[] data = new byte[ 512 ]; for (int j = 0; j < data.length; j++) { @@ -153,14 +195,17 @@ public class TestRawDataBlock 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 - } + logger.reset(); + assertEquals(0, logger.logged.size()); + + // Should complain, as there isn't enough data + RawDataBlock block = + new RawDataBlock(new SlowInputStream(data, k)); + assertNotNull(block); + assertEquals( + "Warning on " + k + " byte short block", + 1, logger.logged.size() + ); } } diff --git a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java index ee63825e2..ac6fc08c0 100644 --- a/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java +++ b/src/testcases/org/apache/poi/poifs/storage/TestRawDataBlockList.java @@ -21,6 +21,9 @@ package org.apache.poi.poifs.storage; import java.io.*; +import org.apache.poi.util.DummyPOILogger; +import org.apache.poi.util.POILogFactory; + import junit.framework.*; /** @@ -42,6 +45,13 @@ public class TestRawDataBlockList public TestRawDataBlockList(String name) { super(name); + + // We always want to use our own + // logger + System.setProperty( + "org.apache.poi.util.POILogger", + "org.apache.poi.util.DummyPOILogger" + ); } /** @@ -78,8 +88,15 @@ public class TestRawDataBlockList * Test creating a short RawDataBlockList */ - public void testShortConstructor() + public void testShortConstructor() throws Exception { + // Get the logger to be used + DummyPOILogger logger = (DummyPOILogger)POILogFactory.getLogger( + RawDataBlock.class + ); + assertEquals(0, logger.logged.size()); + + // Test for various short sizes for (int k = 2049; k < 2560; k++) { byte[] data = new byte[ k ]; @@ -88,16 +105,11 @@ public class TestRawDataBlockList { data[ j ] = ( byte ) j; } - try - { - new RawDataBlockList(new ByteArrayInputStream(data)); - fail("Should have thrown IOException creating short block"); - } - catch (IOException ignored) - { - // as expected - } + // Check we logged the error + logger.reset(); + new RawDataBlockList(new ByteArrayInputStream(data)); + assertEquals(1, logger.logged.size()); } } diff --git a/src/testcases/org/apache/poi/util/DummyPOILogger.java b/src/testcases/org/apache/poi/util/DummyPOILogger.java new file mode 100644 index 000000000..7efbfac29 --- /dev/null +++ b/src/testcases/org/apache/poi/util/DummyPOILogger.java @@ -0,0 +1,46 @@ +/* ==================================================================== + 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.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * POILogger which logs into an ArrayList, so that + * tests can see what got logged + */ +public class DummyPOILogger extends POILogger { + public List logged = new ArrayList(); + + public void reset() { + logged = new ArrayList(); + } + + public boolean check(int level) { + return true; + } + + public void initialize(String cat) {} + + public void log(int level, Object obj1) { + logged.add(new String(level + " - " + obj1)); + } + + public void log(int level, Object obj1, Throwable exception) { + logged.add(new String(level + " - " + obj1 + " - " + exception)); + } +}