From 363fa940ac038f5c89aee0dbfa74fd9d1ce9e37e Mon Sep 17 00:00:00 2001 From: xuri Date: Mon, 13 Feb 2023 13:28:02 +0800 Subject: [PATCH] This closes #1468, checks the table name, and added a new error constant `ErrTableNameLength` - XML Structure field typo fixed - Update documentation for the `AddChart` function - Update unit test --- chart.go | 5 ++--- errors.go | 9 +++++++++ picture.go | 4 ++-- stream.go | 5 ++++- stream_test.go | 2 ++ table.go | 44 +++++++++++++++++++++++++++++++++++++++----- table_test.go | 19 +++++++++++++++++++ xmlWorksheet.go | 4 ++-- 8 files changed, 79 insertions(+), 13 deletions(-) diff --git a/chart.go b/chart.go index b983388..839674c 100644 --- a/chart.go +++ b/chart.go @@ -773,7 +773,7 @@ func parseChartOptions(opts *Chart) (*Chart, error) { // The 'ShowVal' property is optional. The default value is false. // // Set the primary horizontal and vertical axis options by 'XAxis' and 'YAxis'. -// The properties of XAxis that can be set are: +// The properties of 'XAxis' that can be set are: // // None // MajorGridLines @@ -790,13 +790,12 @@ func parseChartOptions(opts *Chart) (*Chart, error) { // MajorGridLines // MinorGridLines // MajorUnit -// TickLabelSkip // ReverseOrder // Maximum // Minimum // Font // -// none: Disable axes. +// None: Disable axes. // // MajorGridLines: Specifies major grid lines. // diff --git a/errors.go b/errors.go index 2a627d3..1357d0f 100644 --- a/errors.go +++ b/errors.go @@ -40,6 +40,12 @@ func newInvalidExcelDateError(dateValue float64) error { return fmt.Errorf("invalid date value %f, negative values are not supported", dateValue) } +// newInvalidTableNameError defined the error message on receiving the invalid +// table name. +func newInvalidTableNameError(name string) error { + return fmt.Errorf("invalid table name %q", name) +} + // newUnsupportedChartType defined the error message on receiving the chart // type are unsupported. func newUnsupportedChartType(chartType string) error { @@ -230,6 +236,9 @@ var ( // ErrSheetNameLength defined the error message on receiving the sheet // name length exceeds the limit. ErrSheetNameLength = fmt.Errorf("the sheet name length exceeds the %d characters limit", MaxSheetNameLength) + // ErrTableNameLength defined the error message on receiving the table name + // length exceeds the limit. + ErrTableNameLength = fmt.Errorf("the table name length exceeds the %d characters limit", MaxFieldLength) // ErrCellStyles defined the error message on cell styles exceeds the limit. ErrCellStyles = fmt.Errorf("the cell styles exceeds the %d limit", MaxCellStyles) // ErrUnprotectWorkbook defined the error message on workbook has set no diff --git a/picture.go b/picture.go index f003852..54b03f2 100644 --- a/picture.go +++ b/picture.go @@ -110,14 +110,14 @@ func parseGraphicOptions(opts *GraphicOptions) *GraphicOptions { // } // } // -// The optional parameter "Autofit" specifies if you make image size auto-fits the +// The optional parameter "AutoFit" specifies if you make image size auto-fits the // cell, the default value of that is 'false'. // // The optional parameter "Hyperlink" specifies the hyperlink of the image. // // The optional parameter "HyperlinkType" defines two types of // hyperlink "External" for website or "Location" for moving to one of the -// cells in this workbook. When the "hyperlink_type" is "Location", +// cells in this workbook. When the "HyperlinkType" is "Location", // coordinates need to start with "#". // // The optional parameter "Positioning" defines two types of the position of an diff --git a/stream.go b/stream.go index 4be4def..bafd759 100644 --- a/stream.go +++ b/stream.go @@ -161,7 +161,10 @@ func (f *File) NewStreamWriter(sheet string) (*StreamWriter, error) { // // See File.AddTable for details on the table format. func (sw *StreamWriter) AddTable(rangeRef string, opts *TableOptions) error { - options := parseTableOptions(opts) + options, err := parseTableOptions(opts) + if err != nil { + return err + } coordinates, err := rangeRefToCoordinates(rangeRef) if err != nil { return err diff --git a/stream_test.go b/stream_test.go index 195bdf0..0e69055 100644 --- a/stream_test.go +++ b/stream_test.go @@ -222,6 +222,8 @@ func TestStreamTable(t *testing.T) { // Test add table with illegal cell reference assert.EqualError(t, streamWriter.AddTable("A:B1", nil), newCellNameToCoordinatesError("A", newInvalidCellNameError("A")).Error()) assert.EqualError(t, streamWriter.AddTable("A1:B", nil), newCellNameToCoordinatesError("B", newInvalidCellNameError("B")).Error()) + // Test add table with invalid table name + assert.EqualError(t, streamWriter.AddTable("A:B1", &TableOptions{Name: "1Table"}), newInvalidTableNameError("1Table").Error()) // Test add table with unsupported charset content types file.ContentTypes = nil file.Pkg.Store(defaultXMLPathContentTypes, MacintoshCyrillicCharset) diff --git a/table.go b/table.go index d98fd7e..a00b0a2 100644 --- a/table.go +++ b/table.go @@ -17,18 +17,24 @@ import ( "regexp" "strconv" "strings" + "unicode" + "unicode/utf8" ) // parseTableOptions provides a function to parse the format settings of the // table with default value. -func parseTableOptions(opts *TableOptions) *TableOptions { +func parseTableOptions(opts *TableOptions) (*TableOptions, error) { + var err error if opts == nil { - return &TableOptions{ShowRowStripes: boolPtr(true)} + return &TableOptions{ShowRowStripes: boolPtr(true)}, err } if opts.ShowRowStripes == nil { opts.ShowRowStripes = boolPtr(true) } - return opts + if err = checkTableName(opts.Name); err != nil { + return opts, err + } + return opts, err } // AddTable provides the method to add table in a worksheet by given worksheet @@ -54,7 +60,9 @@ func parseTableOptions(opts *TableOptions) *TableOptions { // header row data of the table before calling the AddTable function. Multiple // tables range reference that can't have an intersection. // -// Name: The name of the table, in the same worksheet name of the table should be unique +// Name: The name of the table, in the same worksheet name of the table should +// be unique, starts with a letter or underscore (_), doesn't include a +// space or character, and should be no more than 255 characters // // StyleName: The built-in table style names // @@ -62,7 +70,10 @@ func parseTableOptions(opts *TableOptions) *TableOptions { // TableStyleMedium1 - TableStyleMedium28 // TableStyleDark1 - TableStyleDark11 func (f *File) AddTable(sheet, rangeRef string, opts *TableOptions) error { - options := parseTableOptions(opts) + options, err := parseTableOptions(opts) + if err != nil { + return err + } // Coordinate conversion, convert C1:B3 to 2,0,1,2. coordinates, err := rangeRefToCoordinates(rangeRef) if err != nil { @@ -147,6 +158,29 @@ func (f *File) setTableHeader(sheet string, x1, y1, x2 int) ([]*xlsxTableColumn, return tableColumns, nil } +// checkSheetName check whether there are illegal characters in the table name. +// Verify that the name: +// 1. Starts with a letter or underscore (_) +// 2. Doesn't include a space or character that isn't allowed +func checkTableName(name string) error { + if utf8.RuneCountInString(name) > MaxFieldLength { + return ErrTableNameLength + } + for i, c := range name { + if string(c) == "_" { + continue + } + if unicode.IsLetter(c) { + continue + } + if i > 0 && unicode.IsDigit(c) { + continue + } + return newInvalidTableNameError(name) + } + return nil +} + // addTable provides a function to add table by given worksheet name, // range reference and format set. func (f *File) addTable(sheet, tableXML string, x1, y1, x2, y2, i int, opts *TableOptions) error { diff --git a/table_test.go b/table_test.go index 1e1afae..33ce2e9 100644 --- a/table_test.go +++ b/table_test.go @@ -3,6 +3,7 @@ package excelize import ( "fmt" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -37,6 +38,24 @@ func TestAddTable(t *testing.T) { f = NewFile() assert.EqualError(t, f.addTable("sheet1", "", 0, 0, 0, 0, 0, nil), "invalid cell reference [0, 0]") assert.EqualError(t, f.addTable("sheet1", "", 1, 1, 0, 0, 0, nil), "invalid cell reference [0, 0]") + // Test add table with invalid table name + for _, cases := range []struct { + name string + err error + }{ + {name: "1Table", err: newInvalidTableNameError("1Table")}, + {name: "-Table", err: newInvalidTableNameError("-Table")}, + {name: "'Table", err: newInvalidTableNameError("'Table")}, + {name: "Table 1", err: newInvalidTableNameError("Table 1")}, + {name: "A&B", err: newInvalidTableNameError("A&B")}, + {name: "_1Table'", err: newInvalidTableNameError("_1Table'")}, + {name: "\u0f5f\u0fb3\u0f0b\u0f21", err: newInvalidTableNameError("\u0f5f\u0fb3\u0f0b\u0f21")}, + {name: strings.Repeat("c", MaxFieldLength+1), err: ErrTableNameLength}, + } { + assert.EqualError(t, f.AddTable("Sheet1", "A1:B2", &TableOptions{ + Name: cases.name, + }), cases.err.Error()) + } } func TestSetTableHeader(t *testing.T) { diff --git a/xmlWorksheet.go b/xmlWorksheet.go index 0974733..98727de 100644 --- a/xmlWorksheet.go +++ b/xmlWorksheet.go @@ -766,7 +766,7 @@ type decodeX14ConditionalFormatting struct { // decodeX14CfRule directly maps the cfRule element. type decodeX14CfRule struct { - XNLName xml.Name `xml:"cfRule"` + XMLName xml.Name `xml:"cfRule"` Type string `xml:"type,attr,omitempty"` ID string `xml:"id,attr,omitempty"` DataBar *decodeX14DataBar `xml:"dataBar"` @@ -774,7 +774,7 @@ type decodeX14CfRule struct { // decodeX14DataBar directly maps the dataBar element. type decodeX14DataBar struct { - XNLName xml.Name `xml:"dataBar"` + XMLName xml.Name `xml:"dataBar"` MaxLength int `xml:"maxLength,attr"` MinLength int `xml:"minLength,attr"` Border bool `xml:"border,attr,omitempty"`