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

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
 			}
 		}

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",

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 {

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, `

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.

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())
 

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
+				}
 			}
 		}
 	}

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
 		}

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]

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"`