This closes qax-os#1723, fix panic on read workbook in some cases

- Fix panic on read workbook with internal row element without r attribute - Update the unit tests
lujin1 · Nov 18, 2023 · 6b0a812 · 6b0a812
1 parent 068f7ec
commit 6b0a812
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 57 deletions.
diff --git 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
@@ -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
@@ -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
@@ -372,6 +372,16 @@ func TestGetCellValue(t *testing.T) {
 	}, rows)
 	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{}
+	cell, err = f.GetCellValue("Sheet1", "A3")
+	assert.Equal(t, "A3", cell)
+	assert.NoError(t, err)
+	rows, err = f.GetRows("Sheet1")
+	assert.Equal(t, [][]string{{"A1"}, nil, {"A3"}}, rows)
+	assert.NoError(t, err)
+
 	f.Sheet.Delete("xl/worksheets/sheet1.xml")
 	f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `
 	<row r="1"><c r="A1"><v>2422.3000000000002</v></c></row>

diff --git a/excelize.go b/excelize.go
@@ -303,64 +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
-		if len(sheetData.Row) > row-1 {
-			sheetData.Row[row-1] = r
+		if *r.R != 0 && *r.R > row {
+			row = *r.R
 		}
 	}
-	for i := 1; i <= row; i++ {
-		if len(sheetData.Row) > i-1 {
-			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/go.mod b/go.mod
@@ -1,4 +1,4 @@
-module github.com/lujin1/excelize
+module github.com/xuri/excelize/v2
 
 go 1.16
 

diff --git a/rows.go b/rows.go
@@ -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
@@ -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
@@ -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"`