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

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<Record>    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<String> failed = new ArrayList<String>();
 
 		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<String> failed) {
+	private void runWithArrayOfFiles(String[] files, String dir, List<String> 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<String> 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<String> failed = new ArrayList<String>();
+//		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<String> failed = new ArrayList<String>();
+		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"); 
 	};