This closes #1723, fix panic on read workbook in some cases (#1692)

- Fix panic on read workbook with internal row element without r attribute
- Check worksheet XML before get all cell value by column or row
- Update the unit tests

diff --git a/adjust.go b/adjust.go
index 5cdb711..b5a99c5 100644 (file)
--- a/adjust.go
+++ b/adjust.go
@@ -201,13 +201,13 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
                return nil
        }
        lastRow := &ws.SheetData.Row[totalRows-1]
-       if newRow := lastRow.R + offset; lastRow.R >= row && newRow > 0 && newRow > TotalRows {
+       if newRow := *lastRow.R + offset; *lastRow.R >= row && newRow > 0 && newRow > TotalRows {
                return ErrMaxRows
        }
        numOfRows := len(ws.SheetData.Row)
        for i := 0; i < numOfRows; i++ {
                r := &ws.SheetData.Row[i]
-               if newRow := r.R + offset; r.R >= row && newRow > 0 {
+               if newRow := *r.R + offset; *r.R >= row && newRow > 0 {
                        r.adjustSingleRowDimensions(offset)
                }
                if err := f.adjustSingleRowFormulas(sheet, sheet, r, row, offset, false); err != nil {
@@ -219,10 +219,10 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
 
 // adjustSingleRowDimensions provides a function to adjust single row dimensions.
 func (r *xlsxRow) adjustSingleRowDimensions(offset int) {
-       r.R += offset
+       r.R = intPtr(*r.R + offset)
        for i, col := range r.C {
                colName, _, _ := SplitCellName(col.R)
-               r.C[i].R, _ = JoinCellName(colName, r.R)
+               r.C[i].R, _ = JoinCellName(colName, *r.R)
        }
 }
 
@@ -561,7 +561,7 @@ func (f *File) adjustAutoFilter(ws *xlsxWorksheet, sheet string, dir adjustDirec
                ws.AutoFilter = nil
                for rowIdx := range ws.SheetData.Row {
                        rowData := &ws.SheetData.Row[rowIdx]
-                       if rowData.R > y1 && rowData.R <= y2 {
+                       if rowData.R != nil && *rowData.R > y1 && *rowData.R <= y2 {
                                rowData.Hidden = false
                        }
                }

diff --git a/adjust_test.go b/adjust_test.go
index 769affe..a8dd2ff 100644 (file)
--- a/adjust_test.go
+++ b/adjust_test.go
@@ -289,7 +289,7 @@ func TestAdjustAutoFilter(t *testing.T) {
        f := NewFile()
        assert.NoError(t, f.adjustAutoFilter(&xlsxWorksheet{
                SheetData: xlsxSheetData{
-                       Row: []xlsxRow{{Hidden: true, R: 2}},
+                       Row: []xlsxRow{{Hidden: true, R: intPtr(2)}},
                },
                AutoFilter: &xlsxAutoFilter{
                        Ref: "A1:A3",

diff --git a/cell.go b/cell.go
index dd9980d..80d113a 100644 (file)
--- a/cell.go
+++ b/cell.go
@@ -1335,8 +1335,8 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
                return "", err
        }
        lastRowNum := 0
-       if l := len(ws.SheetData.Row); l > 0 {
-               lastRowNum = ws.SheetData.Row[l-1].R
+       if l := len(ws.SheetData.Row); l > 0 && ws.SheetData.Row[l-1].R != nil {
+               lastRowNum = *ws.SheetData.Row[l-1].R
        }
 
        // keep in mind: row starts from 1
@@ -1346,7 +1346,7 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
 
        for rowIdx := range ws.SheetData.Row {
                rowData := &ws.SheetData.Row[rowIdx]
-               if rowData.R != row {
+               if rowData.R != nil && *rowData.R != row {
                        continue
                }
                for colIdx := range rowData.C {

diff --git a/cell_test.go b/cell_test.go
index 1307688..ea802c0 100644 (file)
--- a/cell_test.go
+++ b/cell_test.go
@@ -358,9 +358,6 @@ func TestGetCellValue(t *testing.T) {
        f.Sheet.Delete("xl/worksheets/sheet1.xml")
        f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row r="0"><c r="H6" t="inlineStr"><is><t>H6</t></is></c><c r="A1" t="inlineStr"><is><t>r0A6</t></is></c><c r="F4" t="inlineStr"><is><t>F4</t></is></c></row><row><c r="A1" t="inlineStr"><is><t>A6</t></is></c><c r="B1" t="inlineStr"><is><t>B6</t></is></c><c r="C1" t="inlineStr"><is><t>C6</t></is></c></row><row r="3"><c r="A3"><v>100</v></c><c r="B3" t="inlineStr"><is><t>B3</t></is></c></row>`)))
        f.checked = sync.Map{}
-       cell, err = f.GetCellValue("Sheet1", "H6")
-       assert.Equal(t, "H6", cell)
-       assert.NoError(t, err)
        rows, err = f.GetRows("Sheet1")
        assert.Equal(t, [][]string{
                {"A6", "B6", "C6"},
@@ -371,6 +368,19 @@ func TestGetCellValue(t *testing.T) {
                {"", "", "", "", "", "", "", "H6"},
        }, rows)
        assert.NoError(t, err)
+       cell, err = f.GetCellValue("Sheet1", "H6")
+       assert.Equal(t, "H6", cell)
+       assert.NoError(t, err)
+
+       f.Sheet.Delete("xl/worksheets/sheet1.xml")
+       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `<row><c r="A1" t="inlineStr"><is><t>A1</t></is></c></row><row></row><row><c r="A3" t="inlineStr"><is><t>A3</t></is></c></row>`)))
+       f.checked = sync.Map{}
+       rows, err = f.GetRows("Sheet1")
+       assert.Equal(t, [][]string{{"A1"}, nil, {"A3"}}, rows)
+       assert.NoError(t, err)
+       cell, err = f.GetCellValue("Sheet1", "A3")
+       assert.Equal(t, "A3", cell)
+       assert.NoError(t, err)
 
        f.Sheet.Delete("xl/worksheets/sheet1.xml")
        f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `

diff --git a/col.go b/col.go
index 13bf139..c5be1b7 100644 (file)
--- a/col.go
+++ b/col.go
@@ -63,16 +63,16 @@ type Cols struct {
 //         fmt.Println()
 //     }
 func (f *File) GetCols(sheet string, opts ...Options) ([][]string, error) {
-       cols, err := f.Cols(sheet)
-       if err != nil {
+       if _, err := f.workSheetReader(sheet); err != nil {
                return nil, err
        }
+       cols, err := f.Cols(sheet)
        results := make([][]string, 0, 64)
        for cols.Next() {
                col, _ := cols.Rows(opts...)
                results = append(results, col)
        }
-       return results, nil
+       return results, err
 }
 
 // Next will return true if the next column is found.

diff --git a/col_test.go b/col_test.go
index 9af8ca0..ff3892e 100644 (file)
--- a/col_test.go
+++ b/col_test.go
@@ -1,6 +1,7 @@
 package excelize
 
 import (
+       "fmt"
        "path/filepath"
        "sync"
        "testing"
@@ -72,6 +73,17 @@ func TestCols(t *testing.T) {
        cols.Next()
        _, err = cols.Rows()
        assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
+
+       f = NewFile()
+       f.Sheet.Delete("xl/worksheets/sheet1.xml")
+       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`))
+       f.checked = sync.Map{}
+       _, err = f.Cols("Sheet1")
+       assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
+
+       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="2"><c r="A" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`))
+       _, err = f.Cols("Sheet1")
+       assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 }
 
 func TestColumnsIterator(t *testing.T) {
@@ -125,12 +137,12 @@ func TestGetColsError(t *testing.T) {
 
        f = NewFile()
        f.Sheet.Delete("xl/worksheets/sheet1.xml")
-       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`))
+       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(`<worksheet xmlns="%s"><sheetData><row r="A"><c r="2" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`, NameSpaceSpreadSheet.Value)))
        f.checked = sync.Map{}
        _, err = f.GetCols("Sheet1")
-       assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
+       assert.EqualError(t, err, `strconv.ParseInt: parsing "A": invalid syntax`)
 
-       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(`<worksheet><sheetData><row r="2"><c r="A" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`))
+       f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(`<worksheet xmlns="%s"><sheetData><row r="2"><c r="A" t="inlineStr"><is><t>B</t></is></c></row></sheetData></worksheet>`, NameSpaceSpreadSheet.Value)))
        _, err = f.GetCols("Sheet1")
        assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 
@@ -140,7 +152,7 @@ func TestGetColsError(t *testing.T) {
        cols.totalRows = 2
        cols.totalCols = 2
        cols.curCol = 1
-       cols.sheetXML = []byte(`<worksheet><sheetData><row r="1"><c r="A" t="inlineStr"><is><t>A</t></is></c></row></sheetData></worksheet>`)
+       cols.sheetXML = []byte(fmt.Sprintf(`<worksheet xmlns="%s"><sheetData><row r="1"><c r="A" t="inlineStr"><is><t>A</t></is></c></row></sheetData></worksheet>`, NameSpaceSpreadSheet.Value))
        _, err = cols.Rows()
        assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
 

diff --git a/excelize.go b/excelize.go
index ae80ce0..0b85760 100644 (file)
--- a/excelize.go
+++ b/excelize.go
@@ -303,60 +303,83 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
 // checkSheet provides a function to fill each row element and make that is
 // continuous in a worksheet of XML.
 func (ws *xlsxWorksheet) checkSheet() {
-       var row int
-       var r0 xlsxRow
-       for i, r := range ws.SheetData.Row {
-               if i == 0 && r.R == 0 {
-                       r0 = r
-                       ws.SheetData.Row = ws.SheetData.Row[1:]
+       row, r0 := ws.checkSheetRows()
+       sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
+       row = 0
+       for _, r := range ws.SheetData.Row {
+               if r.R == nil {
+                       row++
+                       r.R = intPtr(row)
+                       sheetData.Row[row-1] = r
                        continue
                }
-               if r.R != 0 && r.R > row {
-                       row = r.R
+               if *r.R == row && row > 0 {
+                       sheetData.Row[*r.R-1].C = append(sheetData.Row[*r.R-1].C, r.C...)
                        continue
                }
-               if r.R != row {
-                       row++
+               if *r.R != 0 {
+                       sheetData.Row[*r.R-1] = r
+                       row = *r.R
                }
        }
-       sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
-       row = 0
-       for _, r := range ws.SheetData.Row {
-               if r.R == row && row > 0 {
-                       sheetData.Row[r.R-1].C = append(sheetData.Row[r.R-1].C, r.C...)
+       for i := 1; i <= len(sheetData.Row); i++ {
+               sheetData.Row[i-1].R = intPtr(i)
+       }
+       ws.checkSheetR0(&sheetData, r0)
+}
+
+// checkSheetRows returns the last row number of the worksheet and rows element
+// with r="0" attribute.
+func (ws *xlsxWorksheet) checkSheetRows() (int, []xlsxRow) {
+       var (
+               row, max  int
+               r0        []xlsxRow
+               maxRowNum = func(num int, c []xlsxC) int {
+                       for _, cell := range c {
+                               if _, n, err := CellNameToCoordinates(cell.R); err == nil && n > num {
+                                       num = n
+                               }
+                       }
+                       return num
+               }
+       )
+       for i, r := range ws.SheetData.Row {
+               if r.R == nil {
+                       row++
                        continue
                }
-               if r.R != 0 {
-                       sheetData.Row[r.R-1] = r
-                       row = r.R
+               if i == 0 && *r.R == 0 {
+                       if num := maxRowNum(row, r.C); num > max {
+                               max = num
+                       }
+                       r0 = append(r0, r)
                        continue
                }
-               row++
-               r.R = row
-               sheetData.Row[row-1] = r
+               if *r.R != 0 && *r.R > row {
+                       row = *r.R
+               }
        }
-       for i := 1; i <= row; i++ {
-               sheetData.Row[i-1].R = i
+       if max > row {
+               row = max
        }
-       ws.checkSheetR0(&sheetData, &r0)
+       return row, r0
 }
 
 // checkSheetR0 handle the row element with r="0" attribute, cells in this row
 // could be disorderly, the cell in this row can be used as the value of
 // which cell is empty in the normal rows.
-func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0 *xlsxRow) {
-       for _, cell := range r0.C {
-               if col, row, err := CellNameToCoordinates(cell.R); err == nil {
-                       rows, rowIdx := len(sheetData.Row), row-1
-                       for r := rows; r < row; r++ {
-                               sheetData.Row = append(sheetData.Row, xlsxRow{R: r + 1})
-                       }
-                       columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
-                       for c := columns; c < col; c++ {
-                               sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
-                       }
-                       if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
-                               sheetData.Row[rowIdx].C[colIdx] = cell
+func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0s []xlsxRow) {
+       for _, r0 := range r0s {
+               for _, cell := range r0.C {
+                       if col, row, err := CellNameToCoordinates(cell.R); err == nil {
+                               rowIdx := row - 1
+                               columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
+                               for c := columns; c < col; c++ {
+                                       sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
+                               }
+                               if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
+                                       sheetData.Row[rowIdx].C[colIdx] = cell
+                               }
                        }
                }
        }

diff --git a/rows.go b/rows.go
index 027d44c..9c24057 100644 (file)
--- a/rows.go
+++ b/rows.go
@@ -45,10 +45,10 @@ import (
 //         fmt.Println()
 //     }
 func (f *File) GetRows(sheet string, opts ...Options) ([][]string, error) {
-       rows, err := f.Rows(sheet)
-       if err != nil {
+       if _, err := f.workSheetReader(sheet); err != nil {
                return nil, err
        }
+       rows, _ := f.Rows(sheet)
        results, cur, max := make([][]string, 0, 64), 0, 0
        for rows.Next() {
                cur++
@@ -368,7 +368,7 @@ func (f *File) getRowHeight(sheet string, row int) int {
        defer ws.mu.Unlock()
        for i := range ws.SheetData.Row {
                v := &ws.SheetData.Row[i]
-               if v.R == row && v.Ht != nil {
+               if v.R != nil && *v.R == row && v.Ht != nil {
                        return int(convertRowHeightToPixels(*v.Ht))
                }
        }
@@ -399,7 +399,7 @@ func (f *File) GetRowHeight(sheet string, row int) (float64, error) {
                return ht, nil // it will be better to use 0, but we take care with BC
        }
        for _, v := range ws.SheetData.Row {
-               if v.R == row && v.Ht != nil {
+               if v.R != nil && *v.R == row && v.Ht != nil {
                        return *v.Ht, nil
                }
        }
@@ -554,7 +554,7 @@ func (f *File) RemoveRow(sheet string, row int) error {
        keep := 0
        for rowIdx := 0; rowIdx < len(ws.SheetData.Row); rowIdx++ {
                v := &ws.SheetData.Row[rowIdx]
-               if v.R != row {
+               if v.R != nil && *v.R != row {
                        ws.SheetData.Row[keep] = *v
                        keep++
                }
@@ -625,7 +625,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
        var rowCopy xlsxRow
 
        for i, r := range ws.SheetData.Row {
-               if r.R == row {
+               if *r.R == row {
                        rowCopy = deepcopy.Copy(ws.SheetData.Row[i]).(xlsxRow)
                        ok = true
                        break
@@ -642,7 +642,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
 
        idx2 := -1
        for i, r := range ws.SheetData.Row {
-               if r.R == row2 {
+               if *r.R == row2 {
                        idx2 = i
                        break
                }

diff --git a/sheet.go b/sheet.go
index b9201a3..83ad73e 100644 (file)
--- a/sheet.go
+++ b/sheet.go
@@ -1951,7 +1951,7 @@ func (ws *xlsxWorksheet) prepareSheetXML(col int, row int) {
        if rowCount < row {
                // append missing rows
                for rowIdx := rowCount; rowIdx < row; rowIdx++ {
-                       ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: rowIdx + 1, CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
+                       ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: intPtr(rowIdx + 1), CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
                }
        }
        rowData := &ws.SheetData.Row[row-1]

diff --git a/xmlWorksheet.go b/xmlWorksheet.go
index 6eb860f..177f136 100644 (file)
--- a/xmlWorksheet.go
+++ b/xmlWorksheet.go
@@ -308,7 +308,7 @@ type xlsxSheetData struct {
 // particular row in the worksheet.
 type xlsxRow struct {
        C            []xlsxC  `xml:"c"`
-       R            int      `xml:"r,attr,omitempty"`
+       R            *int     `xml:"r,attr"`
        Spans        string   `xml:"spans,attr,omitempty"`
        S            int      `xml:"s,attr,omitempty"`
        CustomFormat bool     `xml:"customFormat,attr,omitempty"`