Fix bug #53984 - Support the ColInfoRecord coming after the cells, rather than before as is normal, and also handle things so that if an unsupported BofRecord is found for a sheet, warn and skip rather than breaking

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

src/java/org/apache/poi/hssf/model/InternalSheet.java

@@ -21,7 +21,42 @@ import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
-import org.apache.poi.hssf.record.*;
+import org.apache.poi.hssf.record.BOFRecord;
+import org.apache.poi.hssf.record.CFHeaderRecord;
+import org.apache.poi.hssf.record.CalcCountRecord;
+import org.apache.poi.hssf.record.CalcModeRecord;
+import org.apache.poi.hssf.record.CellValueRecordInterface;
+import org.apache.poi.hssf.record.ColumnInfoRecord;
+import org.apache.poi.hssf.record.DVALRecord;
+import org.apache.poi.hssf.record.DefaultColWidthRecord;
+import org.apache.poi.hssf.record.DefaultRowHeightRecord;
+import org.apache.poi.hssf.record.DeltaRecord;
+import org.apache.poi.hssf.record.DimensionsRecord;
+import org.apache.poi.hssf.record.DrawingRecord;
+import org.apache.poi.hssf.record.EOFRecord;
+import org.apache.poi.hssf.record.EscherAggregate;
+import org.apache.poi.hssf.record.FeatHdrRecord;
+import org.apache.poi.hssf.record.FeatRecord;
+import org.apache.poi.hssf.record.GridsetRecord;
+import org.apache.poi.hssf.record.GutsRecord;
+import org.apache.poi.hssf.record.IndexRecord;
+import org.apache.poi.hssf.record.IterationRecord;
+import org.apache.poi.hssf.record.MergeCellsRecord;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.record.PaneRecord;
+import org.apache.poi.hssf.record.PrintGridlinesRecord;
+import org.apache.poi.hssf.record.PrintHeadersRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.RecordBase;
+import org.apache.poi.hssf.record.RecordFormatException;
+import org.apache.poi.hssf.record.RefModeRecord;
+import org.apache.poi.hssf.record.RowRecord;
+import org.apache.poi.hssf.record.SCLRecord;
+import org.apache.poi.hssf.record.SaveRecalcRecord;
+import org.apache.poi.hssf.record.SelectionRecord;
+import org.apache.poi.hssf.record.UncalcedRecord;
+import org.apache.poi.hssf.record.WSBoolRecord;
+import org.apache.poi.hssf.record.WindowTwoRecord;
 import org.apache.poi.hssf.record.aggregates.ChartSubstreamRecordAggregate;
 import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.ConditionalFormattingTable;
@@ -131,11 +166,27 @@ public final class InternalSheet {
         if (rs.peekNextSid() != BOFRecord.sid) {
             throw new RuntimeException("BOF record expected");
         }
+        
         BOFRecord bof = (BOFRecord) rs.getNext();
-        if (bof.getType() != BOFRecord.TYPE_WORKSHEET) {
+        if (bof.getType() == BOFRecord.TYPE_WORKSHEET) {
-            // TODO - fix junit tests throw new RuntimeException("Bad BOF record type");
+            // Good, well supported
+        } else if (bof.getType() == BOFRecord.TYPE_CHART ||
+                   bof.getType() == BOFRecord.TYPE_EXCEL_4_MACRO) {
+            // These aren't really typical sheets... Let it go though,
+            //  we can handle them roughly well enough as a "normal" one
+        } else {
+            // Not a supported type
+            // Skip onto the EOF, then complain
+            while (rs.hasNext()) {
+                Record rec = rs.getNext();
+                if (rec instanceof EOFRecord) {
+                    break;
+                }
+            }
+            throw new UnsupportedBOFType(bof.getType());
         }
         records.add(bof);
+        
         while (rs.hasNext()) {
             int recSid = rs.peekNextSid();
 
@@ -319,6 +370,18 @@ public final class InternalSheet {
             }});
     }
     
+    public static class UnsupportedBOFType extends RecordFormatException {
+        private final int type;
+        protected UnsupportedBOFType(int type) {
+            super("BOF not of a supported type, found " + type);
+            this.type = type;
+        }
+        
+        public int getType() {
+            return type;
+        }
+    }
+
     private static final class RecordCloner implements RecordVisitor {
 
         private final List<Record> _destList;

src/java/org/apache/poi/hssf/model/RecordOrderer.java

@@ -25,6 +25,7 @@ import org.apache.poi.hssf.record.BlankRecord;
 import org.apache.poi.hssf.record.BoolErrRecord;
 import org.apache.poi.hssf.record.CalcCountRecord;
 import org.apache.poi.hssf.record.CalcModeRecord;
+import org.apache.poi.hssf.record.ColumnInfoRecord;
 import org.apache.poi.hssf.record.DVALRecord;
 import org.apache.poi.hssf.record.DateWindow1904Record;
 import org.apache.poi.hssf.record.DefaultColWidthRecord;
@@ -415,7 +416,7 @@ final class RecordOrderer {
 			case DrawingSelectionRecord.sid:
 			case ObjRecord.sid:
 			case TextObjectRecord.sid:
-
+            case ColumnInfoRecord.sid: // See Bugzilla 53984
             case GutsRecord.sid:   // see Bugzilla 50426
 			case WindowOneRecord.sid:
 				// should really be part of workbook stream, but some apps seem to put this before WINDOW2

src/java/org/apache/poi/hssf/record/EscherAggregate.java

@@ -19,7 +19,12 @@ package org.apache.poi.hssf.record;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 
 import org.apache.poi.ddf.DefaultEscherRecordFactory;
 import org.apache.poi.ddf.EscherClientDataRecord;
@@ -755,7 +760,14 @@ public final class EscherAggregate extends AbstractEscherHolderRecord {
      * @return sid of the record with selected location
      */
     private static short sid(List<RecordBase> records, int loc) {
-        return ((Record) records.get(loc)).getSid();
+        RecordBase record = records.get(loc);
+        if (record instanceof Record) {
+            return ((Record)record).getSid();
+        } else {
+            // Aggregates don't have a sid
+            // We could step into them, but for these needs we don't care
+            return -1;
+        }
     }
 
     /**

src/java/org/apache/poi/hssf/usermodel/HSSFWorkbook.java

@@ -46,6 +46,7 @@ import org.apache.poi.hssf.OldExcelFormatException;
 import org.apache.poi.hssf.model.DrawingManager2;
 import org.apache.poi.hssf.model.HSSFFormulaParser;
 import org.apache.poi.hssf.model.InternalSheet;
+import org.apache.poi.hssf.model.InternalSheet.UnsupportedBOFType;
 import org.apache.poi.hssf.model.InternalWorkbook;
 import org.apache.poi.hssf.model.RecordStream;
 import org.apache.poi.hssf.record.AbstractEscherHolderRecord;
@@ -321,8 +322,13 @@ public final class HSSFWorkbook extends POIDocument implements org.apache.poi.ss
         convertLabelRecords(records, recOffset);
         RecordStream rs = new RecordStream(records, recOffset);
         while (rs.hasNext()) {
+            try {
                 InternalSheet sheet = InternalSheet.createSheet(rs);
                 _sheets.add(new HSSFSheet(this, sheet));
+            } catch (UnsupportedBOFType eb) {
+                // Hopefully there's a supported one after this!
+                log.log(POILogger.WARN, "Unsupported BOF found of type " + eb.getType());
+            }
         }
 
         for (int i = 0 ; i < workbook.getNumNames() ; ++i){

src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java

@@ -2731,4 +2731,15 @@ public final class TestBugs extends BaseTestBugzillaIssues {
             assertEquals("A$1+B$1", s.getRow(row).getCell(2).getCellFormula());
         }        
     }
+
+    /**
+     * Unexpected record type (org.apache.poi.hssf.record.ColumnInfoRecord)
+     */
+    @Test
+    public void bug53984() {
+        Workbook wb = openSample("53984.xls");
+        Sheet s = wb.getSheetAt(0);
+        assertEquals("International Communication Services SA", s.getRow(2).getCell(0).getStringCellValue());
+        assertEquals("Saudi Arabia-Riyadh", s.getRow(210).getCell(0).getStringCellValue());
+    }
 }