Merge pull request #119 from sirwart/chartsheets

Prevent workbooks with chartsheets from causing panics

Looks good to me. Test confirmed to break in `master` and be fixed by this branch.

file_test.go

@@ -31,6 +31,12 @@ func (l *FileSuite) TestOpenFileWithoutStyleAndSharedStrings(c *C) {
 	c.Assert(xlsxFile, NotNil)
 }
 
+func (l *FileSuite) TestOpenFileWithChartsheet(c *C) {
+	xlsxFile, error := OpenFile("./testdocs/testchartsheet.xlsx")
+	c.Assert(error, IsNil)
+	c.Assert(xlsxFile, NotNil)
+}
+
 // Test that we can correctly extract a reference table from the
 // sharedStrings.xml file embedded in the XLSX file and return a
 // reference table of string values from it.

lib.go

@@ -589,7 +589,16 @@ func readSheetsFromZipFile(f *zip.File, file *File, sheetXMLMap map[string]strin
 		return nil, nil, err
 	}
 	file.Date1904 = workbook.WorkbookPr.Date1904
-	sheetCount = len(workbook.Sheets.Sheet)
+
+	// Only try and read sheets that have corresponding files.
+	// Notably this excludes chartsheets don't right now
+	var workbookSheets []xlsxSheet
+	for _, sheet := range workbook.Sheets.Sheet {
+		if f := worksheetFileForSheet(sheet, file.worksheets, sheetXMLMap); f != nil {
+			workbookSheets = append(workbookSheets, sheet)
+		}
+	}
+	sheetCount = len(workbookSheets)
 	sheetsByName := make(map[string]*Sheet, sheetCount)
 	sheets := make([]*Sheet, sheetCount)
 	sheetChan := make(chan *indexedSheet, sheetCount)
@@ -604,7 +613,7 @@ func readSheetsFromZipFile(f *zip.File, file *File, sheetXMLMap map[string]strin
 			}
 		}()
 		err = nil
-		for i, rawsheet := range workbook.Sheets.Sheet {
+		for i, rawsheet := range workbookSheets {
 			readSheetFromFile(sheetChan, i, rawsheet, file, sheetXMLMap)
 		}
 	}()
@@ -614,7 +623,7 @@ func readSheetsFromZipFile(f *zip.File, file *File, sheetXMLMap map[string]strin
 		if sheet.Error != nil {
 			return nil, nil, sheet.Error
 		}
-		sheetName := workbook.Sheets.Sheet[sheet.Index].Name
+		sheetName := workbookSheets[sheet.Index].Name
 		sheetsByName[sheetName] = sheet.Sheet
 		sheet.Sheet.Name = sheetName
 		sheets[sheet.Index] = sheet.Sheet

xmlWorkbook.go

@@ -148,6 +148,19 @@ type xlsxCalcPr struct {
 	IterateDelta float64 `xml:"iterateDelta,attr,omitempty"`
 }
 
+// Helper function to lookup the file corresponding to a xlsxSheet object in the worksheets map
+func worksheetFileForSheet(sheet xlsxSheet, worksheets map[string]*zip.File, sheetXMLMap map[string]string) *zip.File {
+	sheetName, ok := sheetXMLMap[sheet.Id]
+	if !ok {
+		if sheet.SheetId != "" {
+			sheetName = fmt.Sprintf("sheet%s", sheet.SheetId)
+		} else {
+			sheetName = fmt.Sprintf("sheet%s", sheet.Id)
+		}
+	}
+	return worksheets[sheetName]
+}
+
 // getWorksheetFromSheet() is an internal helper function to open a
 // sheetN.xml file, refered to by an xlsx.xlsxSheet struct, from the XLSX
 // file and unmarshal it an xlsx.xlsxWorksheet struct
@@ -156,18 +169,12 @@ func getWorksheetFromSheet(sheet xlsxSheet, worksheets map[string]*zip.File, she
 	var decoder *xml.Decoder
 	var worksheet *xlsxWorksheet
 	var error error
-	var sheetName string
 	worksheet = new(xlsxWorksheet)
 
-	sheetName, ok := sheetXMLMap[sheet.Id]
-	if !ok {
-		if sheet.SheetId != "" {
-			sheetName = fmt.Sprintf("sheet%s", sheet.SheetId)
-		} else {
-			sheetName = fmt.Sprintf("sheet%s", sheet.Id)
-		}
+	f := worksheetFileForSheet(sheet, worksheets, sheetXMLMap)
+	if f == nil {
+		return nil, fmt.Errorf("Unable to find sheet '%s'", sheet)
 	}
-	f := worksheets[sheetName]
 	rc, error = f.Open()
 	if error != nil {
 		return nil, error