Skip to content

Commit 6220a79

Browse files
lujin1lujin1
authored
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
1 parent 57faaf2 commit 6220a79

10 files changed

+110
-65
lines changed

‎adjust.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,13 @@ func (f *File) adjustRowDimensions(sheet string, ws *xlsxWorksheet, row, offset
201201
return nil
202202
}
203203
lastRow := &ws.SheetData.Row[totalRows-1]
204-
if newRow := lastRow.R + offset; lastRow.R >= row && newRow > 0 && newRow > TotalRows {
204+
if newRow := *lastRow.R + offset; *lastRow.R >= row && newRow > 0 && newRow > TotalRows {
205205
return ErrMaxRows
206206
}
207207
numOfRows := len(ws.SheetData.Row)
208208
for i := 0; i < numOfRows; i++ {
209209
r := &ws.SheetData.Row[i]
210-
if newRow := r.R + offset; r.R >= row && newRow > 0 {
210+
if newRow := *r.R + offset; *r.R >= row && newRow > 0 {
211211
r.adjustSingleRowDimensions(offset)
212212
}
213213
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
219219

220220
// adjustSingleRowDimensions provides a function to adjust single row dimensions.
221221
func (r *xlsxRow) adjustSingleRowDimensions(offset int) {
222-
r.R += offset
222+
r.R = intPtr(*r.R + offset)
223223
for i, col := range r.C {
224224
colName, _, _ := SplitCellName(col.R)
225-
r.C[i].R, _ = JoinCellName(colName, r.R)
225+
r.C[i].R, _ = JoinCellName(colName, *r.R)
226226
}
227227
}
228228

@@ -561,7 +561,7 @@ func (f *File) adjustAutoFilter(ws *xlsxWorksheet, sheet string, dir adjustDirec
561561
ws.AutoFilter = nil
562562
for rowIdx := range ws.SheetData.Row {
563563
rowData := &ws.SheetData.Row[rowIdx]
564-
if rowData.R > y1 && rowData.R <= y2 {
564+
if rowData.R != nil && *rowData.R > y1 && *rowData.R <= y2 {
565565
rowData.Hidden = false
566566
}
567567
}

‎adjust_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ func TestAdjustAutoFilter(t *testing.T) {
289289
f := NewFile()
290290
assert.NoError(t, f.adjustAutoFilter(&xlsxWorksheet{
291291
SheetData: xlsxSheetData{
292-
Row: []xlsxRow{{Hidden: true, R: 2}},
292+
Row: []xlsxRow{{Hidden: true, R: intPtr(2)}},
293293
},
294294
AutoFilter: &xlsxAutoFilter{
295295
Ref: "A1:A3",

‎cell.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -1335,8 +1335,8 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
13351335
return "", err
13361336
}
13371337
lastRowNum := 0
1338-
if l := len(ws.SheetData.Row); l > 0 {
1339-
lastRowNum = ws.SheetData.Row[l-1].R
1338+
if l := len(ws.SheetData.Row); l > 0 && ws.SheetData.Row[l-1].R != nil {
1339+
lastRowNum = *ws.SheetData.Row[l-1].R
13401340
}
13411341

13421342
// keep in mind: row starts from 1
@@ -1346,7 +1346,7 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
13461346

13471347
for rowIdx := range ws.SheetData.Row {
13481348
rowData := &ws.SheetData.Row[rowIdx]
1349-
if rowData.R != row {
1349+
if rowData.R != nil && *rowData.R != row {
13501350
continue
13511351
}
13521352
for colIdx := range rowData.C {

‎cell_test.go

+13-3
Original file line numberDiff line numberDiff line change
@@ -358,9 +358,6 @@ func TestGetCellValue(t *testing.T) {
358358
f.Sheet.Delete("xl/worksheets/sheet1.xml")
359359
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>`)))
360360
f.checked = sync.Map{}
361-
cell, err = f.GetCellValue("Sheet1", "H6")
362-
assert.Equal(t, "H6", cell)
363-
assert.NoError(t, err)
364361
rows, err = f.GetRows("Sheet1")
365362
assert.Equal(t, [][]string{
366363
{"A6", "B6", "C6"},
@@ -371,6 +368,19 @@ func TestGetCellValue(t *testing.T) {
371368
{"", "", "", "", "", "", "", "H6"},
372369
}, rows)
373370
assert.NoError(t, err)
371+
cell, err = f.GetCellValue("Sheet1", "H6")
372+
assert.Equal(t, "H6", cell)
373+
assert.NoError(t, err)
374+
375+
f.Sheet.Delete("xl/worksheets/sheet1.xml")
376+
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>`)))
377+
f.checked = sync.Map{}
378+
rows, err = f.GetRows("Sheet1")
379+
assert.Equal(t, [][]string{{"A1"}, nil, {"A3"}}, rows)
380+
assert.NoError(t, err)
381+
cell, err = f.GetCellValue("Sheet1", "A3")
382+
assert.Equal(t, "A3", cell)
383+
assert.NoError(t, err)
374384

375385
f.Sheet.Delete("xl/worksheets/sheet1.xml")
376386
f.Pkg.Store("xl/worksheets/sheet1.xml", []byte(fmt.Sprintf(sheetData, `

‎col.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,16 @@ type Cols struct {
6363
// fmt.Println()
6464
// }
6565
func (f *File) GetCols(sheet string, opts ...Options) ([][]string, error) {
66-
cols, err := f.Cols(sheet)
67-
if err != nil {
66+
if _, err := f.workSheetReader(sheet); err != nil {
6867
return nil, err
6968
}
69+
cols, err := f.Cols(sheet)
7070
results := make([][]string, 0, 64)
7171
for cols.Next() {
7272
col, _ := cols.Rows(opts...)
7373
results = append(results, col)
7474
}
75-
return results, nil
75+
return results, err
7676
}
7777

7878
// Next will return true if the next column is found.

‎col_test.go

+16-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package excelize
22

33
import (
4+
"fmt"
45
"path/filepath"
56
"sync"
67
"testing"
@@ -72,6 +73,17 @@ func TestCols(t *testing.T) {
7273
cols.Next()
7374
_, err = cols.Rows()
7475
assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
76+
77+
f = NewFile()
78+
f.Sheet.Delete("xl/worksheets/sheet1.xml")
79+
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>`))
80+
f.checked = sync.Map{}
81+
_, err = f.Cols("Sheet1")
82+
assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
83+
84+
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>`))
85+
_, err = f.Cols("Sheet1")
86+
assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
7587
}
7688

7789
func TestColumnsIterator(t *testing.T) {
@@ -125,12 +137,12 @@ func TestGetColsError(t *testing.T) {
125137

126138
f = NewFile()
127139
f.Sheet.Delete("xl/worksheets/sheet1.xml")
128-
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>`))
140+
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)))
129141
f.checked = sync.Map{}
130142
_, err = f.GetCols("Sheet1")
131-
assert.EqualError(t, err, `strconv.Atoi: parsing "A": invalid syntax`)
143+
assert.EqualError(t, err, `strconv.ParseInt: parsing "A": invalid syntax`)
132144

133-
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>`))
145+
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)))
134146
_, err = f.GetCols("Sheet1")
135147
assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
136148

@@ -140,7 +152,7 @@ func TestGetColsError(t *testing.T) {
140152
cols.totalRows = 2
141153
cols.totalCols = 2
142154
cols.curCol = 1
143-
cols.sheetXML = []byte(`<worksheet><sheetData><row r="1"><c r="A" t="inlineStr"><is><t>A</t></is></c></row></sheetData></worksheet>`)
155+
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))
144156
_, err = cols.Rows()
145157
assert.EqualError(t, err, newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error())
146158

‎excelize.go

+60-37
Original file line numberDiff line numberDiff line change
@@ -303,60 +303,83 @@ func (f *File) workSheetReader(sheet string) (ws *xlsxWorksheet, err error) {
303303
// checkSheet provides a function to fill each row element and make that is
304304
// continuous in a worksheet of XML.
305305
func (ws *xlsxWorksheet) checkSheet() {
306-
var row int
307-
var r0 xlsxRow
308-
for i, r := range ws.SheetData.Row {
309-
if i == 0 && r.R == 0 {
310-
r0 = r
311-
ws.SheetData.Row = ws.SheetData.Row[1:]
306+
row, r0 := ws.checkSheetRows()
307+
sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
308+
row = 0
309+
for _, r := range ws.SheetData.Row {
310+
if r.R == nil {
311+
row++
312+
r.R = intPtr(row)
313+
sheetData.Row[row-1] = r
312314
continue
313315
}
314-
if r.R != 0 && r.R > row {
315-
row = r.R
316+
if *r.R == row && row > 0 {
317+
sheetData.Row[*r.R-1].C = append(sheetData.Row[*r.R-1].C, r.C...)
316318
continue
317319
}
318-
if r.R != row {
319-
row++
320+
if *r.R != 0 {
321+
sheetData.Row[*r.R-1] = r
322+
row = *r.R
320323
}
321324
}
322-
sheetData := xlsxSheetData{Row: make([]xlsxRow, row)}
323-
row = 0
324-
for _, r := range ws.SheetData.Row {
325-
if r.R == row && row > 0 {
326-
sheetData.Row[r.R-1].C = append(sheetData.Row[r.R-1].C, r.C...)
325+
for i := 1; i <= len(sheetData.Row); i++ {
326+
sheetData.Row[i-1].R = intPtr(i)
327+
}
328+
ws.checkSheetR0(&sheetData, r0)
329+
}
330+
331+
// checkSheetRows returns the last row number of the worksheet and rows element
332+
// with r="0" attribute.
333+
func (ws *xlsxWorksheet) checkSheetRows() (int, []xlsxRow) {
334+
var (
335+
row, max int
336+
r0 []xlsxRow
337+
maxRowNum = func(num int, c []xlsxC) int {
338+
for _, cell := range c {
339+
if _, n, err := CellNameToCoordinates(cell.R); err == nil && n > num {
340+
num = n
341+
}
342+
}
343+
return num
344+
}
345+
)
346+
for i, r := range ws.SheetData.Row {
347+
if r.R == nil {
348+
row++
327349
continue
328350
}
329-
if r.R != 0 {
330-
sheetData.Row[r.R-1] = r
331-
row = r.R
351+
if i == 0 && *r.R == 0 {
352+
if num := maxRowNum(row, r.C); num > max {
353+
max = num
354+
}
355+
r0 = append(r0, r)
332356
continue
333357
}
334-
row++
335-
r.R = row
336-
sheetData.Row[row-1] = r
358+
if *r.R != 0 && *r.R > row {
359+
row = *r.R
360+
}
337361
}
338-
for i := 1; i <= row; i++ {
339-
sheetData.Row[i-1].R = i
362+
if max > row {
363+
row = max
340364
}
341-
ws.checkSheetR0(&sheetData, &r0)
365+
return row, r0
342366
}
343367

344368
// checkSheetR0 handle the row element with r="0" attribute, cells in this row
345369
// could be disorderly, the cell in this row can be used as the value of
346370
// which cell is empty in the normal rows.
347-
func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0 *xlsxRow) {
348-
for _, cell := range r0.C {
349-
if col, row, err := CellNameToCoordinates(cell.R); err == nil {
350-
rows, rowIdx := len(sheetData.Row), row-1
351-
for r := rows; r < row; r++ {
352-
sheetData.Row = append(sheetData.Row, xlsxRow{R: r + 1})
353-
}
354-
columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
355-
for c := columns; c < col; c++ {
356-
sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
357-
}
358-
if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
359-
sheetData.Row[rowIdx].C[colIdx] = cell
371+
func (ws *xlsxWorksheet) checkSheetR0(sheetData *xlsxSheetData, r0s []xlsxRow) {
372+
for _, r0 := range r0s {
373+
for _, cell := range r0.C {
374+
if col, row, err := CellNameToCoordinates(cell.R); err == nil {
375+
rowIdx := row - 1
376+
columns, colIdx := len(sheetData.Row[rowIdx].C), col-1
377+
for c := columns; c < col; c++ {
378+
sheetData.Row[rowIdx].C = append(sheetData.Row[rowIdx].C, xlsxC{})
379+
}
380+
if !sheetData.Row[rowIdx].C[colIdx].hasValue() {
381+
sheetData.Row[rowIdx].C[colIdx] = cell
382+
}
360383
}
361384
}
362385
}

‎rows.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ import (
4545
// fmt.Println()
4646
// }
4747
func (f *File) GetRows(sheet string, opts ...Options) ([][]string, error) {
48-
rows, err := f.Rows(sheet)
49-
if err != nil {
48+
if _, err := f.workSheetReader(sheet); err != nil {
5049
return nil, err
5150
}
51+
rows, _ := f.Rows(sheet)
5252
results, cur, max := make([][]string, 0, 64), 0, 0
5353
for rows.Next() {
5454
cur++
@@ -368,7 +368,7 @@ func (f *File) getRowHeight(sheet string, row int) int {
368368
defer ws.mu.Unlock()
369369
for i := range ws.SheetData.Row {
370370
v := &ws.SheetData.Row[i]
371-
if v.R == row && v.Ht != nil {
371+
if v.R != nil && *v.R == row && v.Ht != nil {
372372
return int(convertRowHeightToPixels(*v.Ht))
373373
}
374374
}
@@ -399,7 +399,7 @@ func (f *File) GetRowHeight(sheet string, row int) (float64, error) {
399399
return ht, nil // it will be better to use 0, but we take care with BC
400400
}
401401
for _, v := range ws.SheetData.Row {
402-
if v.R == row && v.Ht != nil {
402+
if v.R != nil && *v.R == row && v.Ht != nil {
403403
return *v.Ht, nil
404404
}
405405
}
@@ -554,7 +554,7 @@ func (f *File) RemoveRow(sheet string, row int) error {
554554
keep := 0
555555
for rowIdx := 0; rowIdx < len(ws.SheetData.Row); rowIdx++ {
556556
v := &ws.SheetData.Row[rowIdx]
557-
if v.R != row {
557+
if v.R != nil && *v.R != row {
558558
ws.SheetData.Row[keep] = *v
559559
keep++
560560
}
@@ -625,7 +625,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
625625
var rowCopy xlsxRow
626626

627627
for i, r := range ws.SheetData.Row {
628-
if r.R == row {
628+
if *r.R == row {
629629
rowCopy = deepcopy.Copy(ws.SheetData.Row[i]).(xlsxRow)
630630
ok = true
631631
break
@@ -642,7 +642,7 @@ func (f *File) DuplicateRowTo(sheet string, row, row2 int) error {
642642

643643
idx2 := -1
644644
for i, r := range ws.SheetData.Row {
645-
if r.R == row2 {
645+
if *r.R == row2 {
646646
idx2 = i
647647
break
648648
}

‎sheet.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1951,7 +1951,7 @@ func (ws *xlsxWorksheet) prepareSheetXML(col int, row int) {
19511951
if rowCount < row {
19521952
// append missing rows
19531953
for rowIdx := rowCount; rowIdx < row; rowIdx++ {
1954-
ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: rowIdx + 1, CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
1954+
ws.SheetData.Row = append(ws.SheetData.Row, xlsxRow{R: intPtr(rowIdx + 1), CustomHeight: customHeight, Ht: ht, C: make([]xlsxC, 0, sizeHint)})
19551955
}
19561956
}
19571957
rowData := &ws.SheetData.Row[row-1]

‎xmlWorksheet.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ type xlsxSheetData struct {
308308
// particular row in the worksheet.
309309
type xlsxRow struct {
310310
C []xlsxC `xml:"c"`
311-
R int `xml:"r,attr,omitempty"`
311+
R *int `xml:"r,attr"`
312312
Spans string `xml:"spans,attr,omitempty"`
313313
S int `xml:"s,attr,omitempty"`
314314
CustomFormat bool `xml:"customFormat,attr,omitempty"`

0 commit comments

Comments
 (0)