From 8812a4ea04c71ccb498a09b2a9d3558662aade59 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Sun, 27 Oct 2013 08:16:29 +0000 Subject: [PATCH] Re-use functionality from HSSFWorkbook in dev-tools to find the dir-entry-name to make special names like WORKBOOK or BOOK work, handle a special case in ExternalNameRecord when the flag states that there is data, but no data remains in the stream, slightly improve error-output in DirectoryNode, adjust all dev-tools-tests to not report stack-traces any more for expected failures and try with HSSFWorkbook to decide if it is an expected failure or now. git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1536062 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/poi/hssf/dev/BiffViewer.java | 11 ++++- .../org/apache/poi/hssf/dev/EFBiffViewer.java | 20 +++----- .../apache/poi/hssf/dev/FormulaViewer.java | 7 +-- .../org/apache/poi/hssf/dev/RecordLister.java | 7 +-- .../poi/hssf/record/ExternalNameRecord.java | 16 ++++--- .../poi/hssf/usermodel/HSSFWorkbook.java | 21 ++------- .../poi/poifs/filesystem/DirectoryNode.java | 27 +++++------ .../poi/hssf/dev/BaseXLSIteratingTest.java | 16 ++++++- .../apache/poi/hssf/dev/TestBiffViewer.java | 47 +++++++++---------- .../apache/poi/hssf/dev/TestEFBiffViewer.java | 12 +++-- .../org/apache/poi/hssf/dev/TestReSave.java | 26 ++++++++-- .../apache/poi/hssf/dev/TestRecordLister.java | 6 +-- 12 files changed, 114 insertions(+), 102 deletions(-) diff --git a/src/java/org/apache/poi/hssf/dev/BiffViewer.java b/src/java/org/apache/poi/hssf/dev/BiffViewer.java index 10d70048d7..827ccab19f 100644 --- a/src/java/org/apache/poi/hssf/dev/BiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/BiffViewer.java @@ -31,6 +31,7 @@ import org.apache.poi.hssf.record.pivottable.StreamIDRecord; import org.apache.poi.hssf.record.pivottable.ViewDefinitionRecord; import org.apache.poi.hssf.record.pivottable.ViewFieldsRecord; import org.apache.poi.hssf.record.pivottable.ViewSourceRecord; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.util.HexDump; import org.apache.poi.util.LittleEndian; @@ -397,8 +398,7 @@ public final class BiffViewer { ps = System.out; } - POIFSFileSystem fs = new POIFSFileSystem(new FileInputStream(cmdArgs.getFile())); - InputStream is = fs.createDocumentInputStream("Workbook"); + InputStream is = getPOIFSInputStream(cmdArgs.getFile()); if (cmdArgs.shouldOutputRawHexOnly()) { int size = is.available(); @@ -419,6 +419,13 @@ public final class BiffViewer { } } + protected static InputStream getPOIFSInputStream(File file) + throws IOException, FileNotFoundException { + POIFSFileSystem fs = new POIFSFileSystem(new FileInputStream(file)); + String workbookName = HSSFWorkbook.getWorkbookDirEntryName(fs.getRoot()); + return fs.createDocumentInputStream(workbookName); + } + protected static void runBiffViewer(PrintStream ps, InputStream is, boolean dumpInterpretedRecords, boolean dumpHex, boolean zeroAlignHexDump, boolean suppressHeader) { diff --git a/src/java/org/apache/poi/hssf/dev/EFBiffViewer.java b/src/java/org/apache/poi/hssf/dev/EFBiffViewer.java index 6dc36daa4f..2893f31ed9 100644 --- a/src/java/org/apache/poi/hssf/dev/EFBiffViewer.java +++ b/src/java/org/apache/poi/hssf/dev/EFBiffViewer.java @@ -17,16 +17,14 @@ package org.apache.poi.hssf.dev; -import java.io.FileInputStream; -import java.io.InputStream; +import java.io.File; import java.io.IOException; +import java.io.InputStream; -import org.apache.poi.poifs.filesystem.POIFSFileSystem; -import org.apache.poi.hssf.record.Record; - -import org.apache.poi.hssf.eventusermodel.HSSFRequest; -import org.apache.poi.hssf.eventusermodel.HSSFListener; import org.apache.poi.hssf.eventusermodel.HSSFEventFactory; +import org.apache.poi.hssf.eventusermodel.HSSFListener; +import org.apache.poi.hssf.eventusermodel.HSSFRequest; +import org.apache.poi.hssf.record.Record; /** * @@ -43,12 +41,8 @@ public class EFBiffViewer { } - public void run() - throws IOException - { - FileInputStream fin = new FileInputStream(file); - POIFSFileSystem poifs = new POIFSFileSystem(fin); - InputStream din = poifs.createDocumentInputStream("Workbook"); + public void run() throws IOException { + InputStream din = BiffViewer.getPOIFSInputStream(new File(file)); HSSFRequest req = new HSSFRequest(); req.addListenerForAllRecords(new HSSFListener() diff --git a/src/java/org/apache/poi/hssf/dev/FormulaViewer.java b/src/java/org/apache/poi/hssf/dev/FormulaViewer.java index c273d256da..69e0c3005b 100644 --- a/src/java/org/apache/poi/hssf/dev/FormulaViewer.java +++ b/src/java/org/apache/poi/hssf/dev/FormulaViewer.java @@ -17,7 +17,7 @@ package org.apache.poi.hssf.dev; -import java.io.FileInputStream; +import java.io.File; import java.util.List; import org.apache.poi.hssf.model.HSSFFormulaParser; @@ -25,7 +25,6 @@ import org.apache.poi.hssf.record.FormulaRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.RecordFactory; import org.apache.poi.hssf.usermodel.HSSFWorkbook; -import org.apache.poi.poifs.filesystem.POIFSFileSystem; import org.apache.poi.ss.formula.ptg.ExpPtg; import org.apache.poi.ss.formula.ptg.FuncPtg; import org.apache.poi.ss.formula.ptg.OperationPtg; @@ -60,11 +59,9 @@ public class FormulaViewer public void run() throws Exception { - POIFSFileSystem fs = - new POIFSFileSystem(new FileInputStream(file)); List records = RecordFactory - .createRecords(fs.createDocumentInputStream("Workbook")); + .createRecords(BiffViewer.getPOIFSInputStream(new File(file))); for (int k = 0; k < records.size(); k++) { diff --git a/src/java/org/apache/poi/hssf/dev/RecordLister.java b/src/java/org/apache/poi/hssf/dev/RecordLister.java index 4cd7d246b1..4031167872 100644 --- a/src/java/org/apache/poi/hssf/dev/RecordLister.java +++ b/src/java/org/apache/poi/hssf/dev/RecordLister.java @@ -17,7 +17,7 @@ package org.apache.poi.hssf.dev; -import java.io.FileInputStream; +import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -25,7 +25,6 @@ import org.apache.poi.hssf.record.ContinueRecord; import org.apache.poi.hssf.record.Record; import org.apache.poi.hssf.record.RecordFactory; import org.apache.poi.hssf.record.RecordInputStream; -import org.apache.poi.poifs.filesystem.POIFSFileSystem; /** * This is a low-level debugging class, which simply prints @@ -50,9 +49,7 @@ public class RecordLister public void run() throws IOException { - FileInputStream fin = new FileInputStream(file); - POIFSFileSystem poifs = new POIFSFileSystem(fin); - InputStream din = poifs.createDocumentInputStream("Workbook"); + InputStream din = BiffViewer.getPOIFSInputStream(new File(file)); RecordInputStream rinp = new RecordInputStream(din); while(rinp.hasNextRecord()) { diff --git a/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java b/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java index 934cf431d4..0990009f05 100644 --- a/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java +++ b/src/java/org/apache/poi/hssf/record/ExternalNameRecord.java @@ -17,8 +17,8 @@ package org.apache.poi.hssf.record; -import org.apache.poi.ss.formula.constant.ConstantValueParser; import org.apache.poi.ss.formula.Formula; +import org.apache.poi.ss.formula.constant.ConstantValueParser; import org.apache.poi.ss.formula.ptg.Ptg; import org.apache.poi.util.LittleEndianOutput; import org.apache.poi.util.StringUtil; @@ -130,8 +130,10 @@ public final class ExternalNameRecord extends StandardRecord { if(!isOLELink() && !isStdDocumentNameIdentifier()){ if(isAutomaticLink()){ - result += 3; // byte, short - result += ConstantValueParser.getEncodedSize(_ddeValues); + if(_ddeValues != null) { + result += 3; // byte, short + result += ConstantValueParser.getEncodedSize(_ddeValues); + } } else { result += field_5_name_definition.getEncodedSize(); } @@ -149,9 +151,11 @@ public final class ExternalNameRecord extends StandardRecord { if(!isOLELink() && !isStdDocumentNameIdentifier()){ if(isAutomaticLink()){ - out.writeByte(_nColumns-1); - out.writeShort(_nRows-1); - ConstantValueParser.encode(out, _ddeValues); + if(_ddeValues != null) { + out.writeByte(_nColumns-1); + out.writeShort(_nRows-1); + ConstantValueParser.encode(out, _ddeValues); + } } else { field_5_name_definition.serialize(out); } diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java index 9434dbb90b..9ad7f87354 100644 --- a/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java +++ b/src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java @@ -48,19 +48,7 @@ import org.apache.poi.hssf.model.HSSFFormulaParser; import org.apache.poi.hssf.model.InternalSheet; import org.apache.poi.hssf.model.InternalWorkbook; import org.apache.poi.hssf.model.RecordStream; -import org.apache.poi.hssf.record.AbstractEscherHolderRecord; -import org.apache.poi.hssf.record.BackupRecord; -import org.apache.poi.hssf.record.DrawingGroupRecord; -import org.apache.poi.hssf.record.ExtendedFormatRecord; -import org.apache.poi.hssf.record.FontRecord; -import org.apache.poi.hssf.record.LabelRecord; -import org.apache.poi.hssf.record.LabelSSTRecord; -import org.apache.poi.hssf.record.NameRecord; -import org.apache.poi.hssf.record.RecalcIdRecord; -import org.apache.poi.hssf.record.Record; -import org.apache.poi.hssf.record.RecordFactory; -import org.apache.poi.hssf.record.SSTRecord; -import org.apache.poi.hssf.record.UnknownRecord; +import org.apache.poi.hssf.record.*; import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor; import org.apache.poi.hssf.record.common.UnicodeString; import org.apache.poi.hssf.util.CellReference; @@ -222,11 +210,10 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss }; - private static String getWorkbookDirEntryName(DirectoryNode directory) { + public static String getWorkbookDirEntryName(DirectoryNode directory) { - String[] potentialNames = WORKBOOK_DIR_ENTRY_NAMES; - for (int i = 0; i < potentialNames.length; i++) { - String wbName = potentialNames[i]; + for (int i = 0; i < WORKBOOK_DIR_ENTRY_NAMES.length; i++) { + String wbName = WORKBOOK_DIR_ENTRY_NAMES[i]; try { directory.getEntry(wbName); return wbName; diff --git a/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java b/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java index 20effd8bdb..7f8e14e58c 100644 --- a/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java +++ b/src/java/org/apache/poi/poifs/filesystem/DirectoryNode.java @@ -289,13 +289,13 @@ public class DirectoryNode if (rval) { _entries.remove(entry); - _byname.remove(entry.getName()); + _byname.remove(entry.getName()); - if(_ofilesystem != null) { + if(_ofilesystem != null) { _ofilesystem.remove(entry); - } else { - _nfilesystem.remove(entry); - } + } else { + _nfilesystem.remove(entry); + } } return rval; } @@ -359,21 +359,16 @@ public class DirectoryNode * name exists in this DirectoryEntry */ - public Entry getEntry(final String name) - throws FileNotFoundException - { + public Entry getEntry(final String name) throws FileNotFoundException { Entry rval = null; - if (name != null) - { + if (name != null) { rval = _byname.get(name); } - if (rval == null) - { - + if (rval == null) { // either a null name was given, or there is no such name throw new FileNotFoundException("no such entry: \"" + name - + "\""); + + "\", had: " + _byname.keySet()); } return rval; } @@ -479,6 +474,7 @@ public class DirectoryNode * @return true if the Entry is a DirectoryEntry, else false */ + @Override public boolean isDirectoryEntry() { return true; @@ -495,6 +491,7 @@ public class DirectoryNode * false */ + @Override protected boolean isDeleteOK() { @@ -524,7 +521,7 @@ public class DirectoryNode * @return an Iterator; may not be null, but may have an empty * back end store */ - @SuppressWarnings("unchecked") + @SuppressWarnings({ "unchecked", "rawtypes" }) public Iterator getViewableIterator() { List components = new ArrayList(); diff --git a/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java b/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java index d0b8708903..9433f13454 100644 --- a/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java +++ b/src/testcases/org/apache/poi/hssf/dev/BaseXLSIteratingTest.java @@ -1,14 +1,17 @@ package org.apache.poi.hssf.dev; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.File; +import java.io.FileInputStream; import java.io.FilenameFilter; import java.io.IOException; import java.io.OutputStream; import java.util.ArrayList; import java.util.List; +import org.apache.poi.hssf.usermodel.HSSFWorkbook; import org.junit.Test; /** @@ -29,7 +32,7 @@ public abstract class BaseXLSIteratingTest { System.out.println("Had " + count + " files"); } - private int runWithDir(String dir) { + private int runWithDir(String dir) throws IOException { List failed = new ArrayList(); String[] files = new File(dir).list(new FilenameFilter() { @@ -46,7 +49,7 @@ public abstract class BaseXLSIteratingTest { return files.length; } - private void runWithArrayOfFiles(String[] files, String dir, List failed) { + private void runWithArrayOfFiles(String[] files, String dir, List failed) throws IOException { for(String file : files) { try { runOneFile(dir, file, failed); @@ -57,6 +60,15 @@ public abstract class BaseXLSIteratingTest { } e.printStackTrace(); + + // try to read it in HSSFWorkbook to quickly fail if we cannot read the file there at all and thus probably can use SILENT_EXCLUDED instead + FileInputStream stream = new FileInputStream(new File(dir, file)); + try { + assertNotNull(new HSSFWorkbook(stream)); + } finally { + stream.close(); + } + if(!EXCLUDED.contains(file)) { failed.add(file); } diff --git a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java index 19a915cfa2..6f73b7fa7c 100644 --- a/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java +++ b/src/testcases/org/apache/poi/hssf/dev/TestBiffViewer.java @@ -1,45 +1,44 @@ package org.apache.poi.hssf.dev; import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; import java.util.List; -import org.apache.poi.poifs.filesystem.POIFSFileSystem; - public class TestBiffViewer extends BaseXLSIteratingTest { static { - // TODO: is it ok to fail these? // Look at the output of the test for the detailed stacktrace of the failures... - EXCLUDED.add("WORKBOOK_in_capitals.xls"); - EXCLUDED.add("password.xls"); - EXCLUDED.add("NoGutsRecords.xls"); - EXCLUDED.add("BOOK_in_capitals.xls"); - EXCLUDED.add("XRefCalc.xls"); - EXCLUDED.add("50833.xls"); // probably a problem in BiffViewer - EXCLUDED.add("43493.xls"); - EXCLUDED.add("51832.xls"); - EXCLUDED.add("OddStyleRecord.xls"); - + //EXCLUDED.add(""); + + // these are likely ok to fail + SILENT_EXCLUDED.add("XRefCalc.xls"); // "Buffer overrun" + SILENT_EXCLUDED.add("50833.xls"); // "Name is too long" when setting username + SILENT_EXCLUDED.add("OddStyleRecord.xls"); + SILENT_EXCLUDED.add("NoGutsRecords.xls"); + SILENT_EXCLUDED.add("51832.xls"); // password + SILENT_EXCLUDED.add("43493.xls"); // HSSFWorkbook cannot open it as well + SILENT_EXCLUDED.add("password.xls"); SILENT_EXCLUDED.add("46904.xls"); }; @Override void runOneFile(String dir, String file, List failed) throws IOException { - FileInputStream inStream = new FileInputStream(new File(dir, file)); + InputStream is = BiffViewer.getPOIFSInputStream(new File(dir, file)); try { - POIFSFileSystem fs = new POIFSFileSystem(inStream); - InputStream is = fs.createDocumentInputStream("Workbook"); - try { - // use a NullOutputStream to not write the bytes anywhere for best runtime - BiffViewer.runBiffViewer(new PrintStream(NULL_OUTPUT_STREAM), is, true, true, true, false); - } finally { - is.close(); - } + // use a NullOutputStream to not write the bytes anywhere for best runtime + BiffViewer.runBiffViewer(new PrintStream(NULL_OUTPUT_STREAM), is, true, true, true, false); } finally { - inStream.close(); + is.close(); } } + +// @Test +// public void testOneFile() throws Exception { +// List failed = new ArrayList(); +// runOneFile("test-data/spreadsheet", "WORKBOOK_in_capitals.xls", failed); +// +// assertTrue("Expected to have no failed except the ones excluded, but had: " + failed, +// failed.isEmpty()); +// } } diff --git a/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java b/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java index 23807074cd..df3efb5ec4 100644 --- a/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java +++ b/src/testcases/org/apache/poi/hssf/dev/TestEFBiffViewer.java @@ -7,12 +7,14 @@ import java.util.List; public class TestEFBiffViewer extends BaseXLSIteratingTest { static { - // TODO: is it ok to fail these? // Look at the output of the test for the detailed stacktrace of the failures... - EXCLUDED.add("password.xls"); - EXCLUDED.add("XRefCalc.xls"); - EXCLUDED.add("43493.xls"); - EXCLUDED.add("51832.xls"); + //EXCLUDED.add(""); + + // these are likely ok to fail + SILENT_EXCLUDED.add("XRefCalc.xls"); + SILENT_EXCLUDED.add("password.xls"); + SILENT_EXCLUDED.add("51832.xls"); // password + SILENT_EXCLUDED.add("43493.xls"); // HSSFWorkbook cannot open it as well }; @Override diff --git a/src/testcases/org/apache/poi/hssf/dev/TestReSave.java b/src/testcases/org/apache/poi/hssf/dev/TestReSave.java index 4cdf01591e..33e4f6aa05 100644 --- a/src/testcases/org/apache/poi/hssf/dev/TestReSave.java +++ b/src/testcases/org/apache/poi/hssf/dev/TestReSave.java @@ -1,20 +1,25 @@ package org.apache.poi.hssf.dev; +import static org.junit.Assert.assertTrue; + import java.io.File; import java.io.PrintStream; +import java.util.ArrayList; import java.util.List; +import org.junit.Test; + public class TestReSave extends BaseXLSIteratingTest { static { // TODO: is it ok to fail these? // Look at the output of the test for the detailed stacktrace of the failures... - EXCLUDED.add("password.xls"); - EXCLUDED.add("43493.xls"); - EXCLUDED.add("51832.xls"); - EXCLUDED.add("49219.xls"); EXCLUDED.add("49931.xls"); + // these are likely ok to fail + SILENT_EXCLUDED.add("password.xls"); + SILENT_EXCLUDED.add("43493.xls"); // HSSFWorkbook cannot open it as well SILENT_EXCLUDED.add("46904.xls"); + SILENT_EXCLUDED.add("51832.xls"); // password }; @Override @@ -31,6 +36,10 @@ public class TestReSave extends BaseXLSIteratingTest { try { ReSave.main(new String[] { new File(dir, file).getAbsolutePath() }); + + // also try BiffViewer on the saved file + new TestBiffViewer().runOneFile(dir, file.replace(".xls", "-saved.xls"), failed); + try { // had one case where the re-saved could not be re-saved! ReSave.main(new String[] { new File(dir, file.replace(".xls", "-saved.xls")).getAbsolutePath() }); @@ -47,4 +56,13 @@ public class TestReSave extends BaseXLSIteratingTest { System.setOut(save); } } + + @Test + public void testOneFile() throws Exception { + List failed = new ArrayList(); + runOneFile("test-data/spreadsheet", "49219.xls", failed); + + assertTrue("Expected to have no failed except the ones excluded, but had: " + failed, + failed.isEmpty()); + } } diff --git a/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java b/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java index 2afe6399b0..405c43f160 100644 --- a/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java +++ b/src/testcases/org/apache/poi/hssf/dev/TestRecordLister.java @@ -9,11 +9,9 @@ public class TestRecordLister extends BaseXLSIteratingTest { static { // TODO: is it ok to fail these? // Look at the output of the test for the detailed stacktrace of the failures... - EXCLUDED.add("WORKBOOK_in_capitals.xls"); - EXCLUDED.add("NoGutsRecords.xls"); - EXCLUDED.add("BOOK_in_capitals.xls"); - EXCLUDED.add("OddStyleRecord.xls"); + //EXCLUDED.add(""); + // these are likely ok to fail SILENT_EXCLUDED.add("46904.xls"); }; -- 2.39.5