This closes qax-os#1523, preventing format text cell value as a numeric

- Simplify variable declaration and error return statements
- Remove the internal `xlsxTabColor` data type
- Using the `xlsxColor` data type instead of `xlsxTabColor`
- Update unit test, improve code coverage

Author: xuri

calc.go

@@ -791,11 +791,11 @@ func (f *File) CalcCellValue(sheet, cell string, opts ...Options) (result string
 	result = token.Value()
 	if isNum, precision, decimal := isNumeric(result); isNum {
 		if precision > 15 {
-			result, err = f.formattedValue(styleIdx, strings.ToUpper(strconv.FormatFloat(decimal, 'G', 15, 64)), rawCellValue)
+			result, err = f.formattedValue(&xlsxC{S: styleIdx, V: strings.ToUpper(strconv.FormatFloat(decimal, 'G', 15, 64))}, rawCellValue, CellTypeNumber)
 			return
 		}
 		if !strings.HasPrefix(result, "0") {
-			result, err = f.formattedValue(styleIdx, strings.ToUpper(strconv.FormatFloat(decimal, 'f', -1, 64)), rawCellValue)
+			result, err = f.formattedValue(&xlsxC{S: styleIdx, V: strings.ToUpper(strconv.FormatFloat(decimal, 'f', -1, 64))}, rawCellValue, CellTypeNumber)
 		}
 	}
 	return

cell.go

@@ -516,7 +516,7 @@ func (c *xlsxC) getCellBool(f *File, raw bool) (string, error) {
 			return "FALSE", nil
 		}
 	}
-	return f.formattedValue(c.S, c.V, raw)
+	return f.formattedValue(c, raw, CellTypeBool)
 }
 
 // setCellDefault prepares cell type and string type cell value by a given
@@ -551,7 +551,7 @@ func (c *xlsxC) getCellDate(f *File, raw bool) (string, error) {
 			c.V = strconv.FormatFloat(excelTime, 'G', 15, 64)
 		}
 	}
-	return f.formattedValue(c.S, c.V, raw)
+	return f.formattedValue(c, raw, CellTypeBool)
 }
 
 // getValueFrom return a value from a column/row cell, this function is
@@ -567,21 +567,20 @@ func (c *xlsxC) getValueFrom(f *File, d *xlsxSST, raw bool) (string, error) {
 		return c.getCellDate(f, raw)
 	case "s":
 		if c.V != "" {
-			xlsxSI := 0
-			xlsxSI, _ = strconv.Atoi(strings.TrimSpace(c.V))
+			xlsxSI, _ := strconv.Atoi(strings.TrimSpace(c.V))
 			if _, ok := f.tempFiles.Load(defaultXMLPathSharedStrings); ok {
-				return f.formattedValue(c.S, f.getFromStringItem(xlsxSI), raw)
+				return f.formattedValue(&xlsxC{S: c.S, V: f.getFromStringItem(xlsxSI)}, raw, CellTypeSharedString)
 			}
 			if len(d.SI) > xlsxSI {
-				return f.formattedValue(c.S, d.SI[xlsxSI].String(), raw)
+				return f.formattedValue(&xlsxC{S: c.S, V: d.SI[xlsxSI].String()}, raw, CellTypeSharedString)
 			}
 		}
-		return f.formattedValue(c.S, c.V, raw)
+		return f.formattedValue(c, raw, CellTypeSharedString)
 	case "inlineStr":
 		if c.IS != nil {
-			return f.formattedValue(c.S, c.IS.String(), raw)
+			return f.formattedValue(&xlsxC{S: c.S, V: c.IS.String()}, raw, CellTypeInlineString)
 		}
-		return f.formattedValue(c.S, c.V, raw)
+		return f.formattedValue(c, raw, CellTypeInlineString)
 	default:
 		if isNum, precision, decimal := isNumeric(c.V); isNum && !raw {
 			if precision > 15 {
@@ -590,7 +589,7 @@ func (c *xlsxC) getValueFrom(f *File, d *xlsxSST, raw bool) (string, error) {
 				c.V = strconv.FormatFloat(decimal, 'f', -1, 64)
 			}
 		}
-		return f.formattedValue(c.S, c.V, raw)
+		return f.formattedValue(c, raw, CellTypeNumber)
 	}
 }
 
@@ -1325,47 +1324,44 @@ func (f *File) getCellStringFunc(sheet, cell string, fn func(x *xlsxWorksheet, c
 // formattedValue provides a function to returns a value after formatted. If
 // it is possible to apply a format to the cell value, it will do so, if not
 // then an error will be returned, along with the raw value of the cell.
-func (f *File) formattedValue(s int, v string, raw bool) (string, error) {
-	if raw {
-		return v, nil
-	}
-	if s == 0 {
-		return v, nil
+func (f *File) formattedValue(c *xlsxC, raw bool, cellType CellType) (string, error) {
+	if raw || c.S == 0 {
+		return c.V, nil
 	}
 	styleSheet, err := f.stylesReader()
 	if err != nil {
-		return v, err
+		return c.V, err
 	}
 	if styleSheet.CellXfs == nil {
-		return v, err
+		return c.V, err
 	}
-	if s >= len(styleSheet.CellXfs.Xf) || s < 0 {
-		return v, err
+	if c.S >= len(styleSheet.CellXfs.Xf) || c.S < 0 {
+		return c.V, err
 	}
 	var numFmtID int
-	if styleSheet.CellXfs.Xf[s].NumFmtID != nil {
-		numFmtID = *styleSheet.CellXfs.Xf[s].NumFmtID
+	if styleSheet.CellXfs.Xf[c.S].NumFmtID != nil {
+		numFmtID = *styleSheet.CellXfs.Xf[c.S].NumFmtID
 	}
 	date1904 := false
 	wb, err := f.workbookReader()
 	if err != nil {
-		return v, err
+		return c.V, err
 	}
 	if wb != nil && wb.WorkbookPr != nil {
 		date1904 = wb.WorkbookPr.Date1904
 	}
 	if ok := builtInNumFmtFunc[numFmtID]; ok != nil {
-		return ok(v, builtInNumFmt[numFmtID], date1904), err
+		return ok(c.V, builtInNumFmt[numFmtID], date1904, cellType), err
 	}
 	if styleSheet.NumFmts == nil {
-		return v, err
+		return c.V, err
 	}
 	for _, xlsxFmt := range styleSheet.NumFmts.NumFmt {
 		if xlsxFmt.NumFmtID == numFmtID {
-			return format(v, xlsxFmt.FormatCode, date1904), err
+			return format(c.V, xlsxFmt.FormatCode, date1904, cellType), err
 		}
 	}
-	return v, err
+	return c.V, err
 }
 
 // prepareCellStyle provides a function to prepare style index of cell in

cell_test.go

@@ -803,29 +803,29 @@ func TestSetCellRichText(t *testing.T) {
 
 func TestFormattedValue(t *testing.T) {
 	f := NewFile()
-	result, err := f.formattedValue(0, "43528", false)
+	result, err := f.formattedValue(&xlsxC{S: 0, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
 	// S is too large
-	result, err = f.formattedValue(15, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 15, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
 	// S is too small
-	result, err = f.formattedValue(-15, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: -15, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
-	result, err = f.formattedValue(1, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 1, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 	customNumFmt := "[$-409]MM/DD/YYYY"
 	_, err = f.NewStyle(&Style{
 		CustomNumFmt: &customNumFmt,
 	})
 	assert.NoError(t, err)
-	result, err = f.formattedValue(1, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 1, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "03/04/2019", result)
 
@@ -834,15 +834,15 @@ func TestFormattedValue(t *testing.T) {
 	f.Styles.CellXfs.Xf = append(f.Styles.CellXfs.Xf, xlsxXf{
 		NumFmtID: &numFmtID,
 	})
-	result, err = f.formattedValue(2, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 2, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
 	// Test format value with invalid number format ID
 	f.Styles.CellXfs.Xf = append(f.Styles.CellXfs.Xf, xlsxXf{
 		NumFmtID: nil,
 	})
-	result, err = f.formattedValue(3, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 3, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
@@ -851,7 +851,16 @@ func TestFormattedValue(t *testing.T) {
 	f.Styles.CellXfs.Xf = append(f.Styles.CellXfs.Xf, xlsxXf{
 		NumFmtID: &numFmtID,
 	})
-	result, err = f.formattedValue(1, "43528", false)
+	result, err = f.formattedValue(&xlsxC{S: 1, V: "43528"}, false, CellTypeNumber)
+	assert.NoError(t, err)
+	assert.Equal(t, "43528", result)
+
+	// Test format numeric value with shared string data type
+	f.Styles.NumFmts, numFmtID = nil, 11
+	f.Styles.CellXfs.Xf = append(f.Styles.CellXfs.Xf, xlsxXf{
+		NumFmtID: &numFmtID,
+	})
+	result, err = f.formattedValue(&xlsxC{S: 5, V: "43528"}, false, CellTypeSharedString)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 
@@ -860,32 +869,36 @@ func TestFormattedValue(t *testing.T) {
 		NumFmt: 1,
 	})
 	assert.NoError(t, err)
-	result, err = f.formattedValue(styleID, "310.56", false)
+	result, err = f.formattedValue(&xlsxC{S: styleID, V: "310.56"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "311", result)
 
 	for _, fn := range builtInNumFmtFunc {
-		assert.Equal(t, "0_0", fn("0_0", "", false))
+		assert.Equal(t, "0_0", fn("0_0", "", false, CellTypeNumber))
 	}
 
 	// Test format value with unsupported charset workbook
 	f.WorkBook = nil
 	f.Pkg.Store(defaultXMLPathWorkbook, MacintoshCyrillicCharset)
-	_, err = f.formattedValue(1, "43528", false)
+	_, err = f.formattedValue(&xlsxC{S: 1, V: "43528"}, false, CellTypeNumber)
 	assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
 
 	// Test format value with unsupported charset style sheet
 	f.Styles = nil
 	f.Pkg.Store(defaultXMLPathStyles, MacintoshCyrillicCharset)
-	_, err = f.formattedValue(1, "43528", false)
+	_, err = f.formattedValue(&xlsxC{S: 1, V: "43528"}, false, CellTypeNumber)
 	assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8")
+
+	for _, fn := range builtInNumFmtFunc {
+		assert.Equal(t, fn("text", "0", false, CellTypeNumber), "text")
+	}
 }
 
 func TestFormattedValueNilXfs(t *testing.T) {
 	// Set the CellXfs to nil and verify that the formattedValue function does not crash
 	f := NewFile()
 	f.Styles.CellXfs = nil
-	result, err := f.formattedValue(3, "43528", false)
+	result, err := f.formattedValue(&xlsxC{S: 3, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 }
@@ -894,7 +907,7 @@ func TestFormattedValueNilNumFmts(t *testing.T) {
 	// Set the NumFmts value to nil and verify that the formattedValue function does not crash
 	f := NewFile()
 	f.Styles.NumFmts = nil
-	result, err := f.formattedValue(3, "43528", false)
+	result, err := f.formattedValue(&xlsxC{S: 3, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 }
@@ -903,7 +916,7 @@ func TestFormattedValueNilWorkbook(t *testing.T) {
 	// Set the Workbook value to nil and verify that the formattedValue function does not crash
 	f := NewFile()
 	f.WorkBook = nil
-	result, err := f.formattedValue(3, "43528", false)
+	result, err := f.formattedValue(&xlsxC{S: 3, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 }
@@ -913,7 +926,7 @@ func TestFormattedValueNilWorkbookPr(t *testing.T) {
 	// crash.
 	f := NewFile()
 	f.WorkBook.WorkbookPr = nil
-	result, err := f.formattedValue(3, "43528", false)
+	result, err := f.formattedValue(&xlsxC{S: 3, V: "43528"}, false, CellTypeNumber)
 	assert.NoError(t, err)
 	assert.Equal(t, "43528", result)
 }

chart_test.go

@@ -121,6 +121,12 @@ func TestDeleteDrawing(t *testing.T) {
 	path := "xl/drawings/drawing1.xml"
 	f.Pkg.Store(path, MacintoshCyrillicCharset)
 	assert.EqualError(t, f.deleteDrawing(0, 0, path, "Chart"), "XML syntax error on line 1: invalid UTF-8")
+	f, err := OpenFile(filepath.Join("test", "Book1.xlsx"))
+	assert.NoError(t, err)
+	f.Drawings.Store(path, &xlsxWsDr{TwoCellAnchor: []*xdrCellAnchor{{
+		GraphicFrame: string(MacintoshCyrillicCharset),
+	}}})
+	assert.EqualError(t, f.deleteDrawing(0, 0, path, "Chart"), "XML syntax error on line 1: invalid UTF-8")
 }
 
 func TestAddChart(t *testing.T) {

crypt_test.go

@@ -12,11 +12,14 @@
 package excelize
 
 import (
+	"bytes"
+	"encoding/binary"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/richardlehane/mscfb"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -51,6 +54,25 @@ func TestEncrypt(t *testing.T) {
 	// Test remove password by save workbook with options
 	assert.NoError(t, f.Save(Options{Password: ""}))
 	assert.NoError(t, f.Close())
+
+	doc, err := mscfb.New(bytes.NewReader(raw))
+	assert.NoError(t, err)
+	encryptionInfoBuf, encryptedPackageBuf := extractPart(doc)
+	binary.LittleEndian.PutUint64(encryptionInfoBuf[20:32], uint64(0))
+	_, err = standardDecrypt(encryptionInfoBuf, encryptedPackageBuf, &Options{Password: "password"})
+	assert.NoError(t, err)
+	_, err = decrypt(nil, nil, nil)
+	assert.EqualError(t, err, "crypto/aes: invalid key size 0")
+	_, err = agileDecrypt(encryptionInfoBuf, MacintoshCyrillicCharset, &Options{Password: "password"})
+	assert.EqualError(t, err, "XML syntax error on line 1: invalid character entity &0 (no semicolon)")
+	_, err = convertPasswdToKey("password", nil, Encryption{
+		KeyEncryptors: KeyEncryptors{KeyEncryptor: []KeyEncryptor{
+			{EncryptedKey: EncryptedKey{KeyData: KeyData{SaltValue: "=="}}},
+		}},
+	})
+	assert.EqualError(t, err, "illegal base64 data at input byte 0")
+	_, err = createIV([]byte{0}, Encryption{KeyData: KeyData{SaltValue: "=="}})
+	assert.EqualError(t, err, "illegal base64 data at input byte 0")
 }
 
 func TestEncryptionMechanism(t *testing.T) {

excelize.go

@@ -101,11 +101,10 @@ func OpenFile(filename string, opts ...Options) (*File, error) {
 	}
 	f, err := OpenReader(file, opts...)
 	if err != nil {
-		closeErr := file.Close()
-		if closeErr == nil {
-			return f, err
+		if closeErr := file.Close(); closeErr != nil {
+			return f, closeErr
 		}
-		return f, closeErr
+		return f, err
 	}
 	f.Path = filename
 	return f, file.Close()

numfmt.go

@@ -31,6 +31,7 @@ type languageInfo struct {
 // numberFormat directly maps the number format parser runtime required
 // fields.
 type numberFormat struct {
+	cellType                                       CellType
 	section                                        []nfp.Section
 	t                                              time.Time
 	sectionIdx                                     int
@@ -336,6 +337,9 @@ var (
 // prepareNumberic split the number into two before and after parts by a
 // decimal point.
 func (nf *numberFormat) prepareNumberic(value string) {
+	if nf.cellType != CellTypeNumber && nf.cellType != CellTypeDate {
+		return
+	}
 	if nf.isNumeric, _, _ = isNumeric(value); !nf.isNumeric {
 		return
 	}
@@ -344,9 +348,9 @@ func (nf *numberFormat) prepareNumberic(value string) {
 // format provides a function to return a string parse by number format
 // expression. If the given number format is not supported, this will return
 // the original cell value.
-func format(value, numFmt string, date1904 bool) string {
+func format(value, numFmt string, date1904 bool, cellType CellType) string {
 	p := nfp.NumberFormatParser()
-	nf := numberFormat{section: p.Parse(numFmt), value: value, date1904: date1904}
+	nf := numberFormat{section: p.Parse(numFmt), value: value, date1904: date1904, cellType: cellType}
 	nf.number, nf.valueSectionType = nf.getValueSectionType(value)
 	nf.prepareNumberic(value)
 	for i, section := range nf.section {
@@ -947,7 +951,7 @@ func (nf *numberFormat) textHandler() (result string) {
 		if token.TType == nfp.TokenTypeLiteral {
 			result += token.TValue
 		}
-		if token.TType == nfp.TokenTypeTextPlaceHolder {
+		if token.TType == nfp.TokenTypeTextPlaceHolder || token.TType == nfp.TokenTypeZeroPlaceHolder {
 			result += nf.value
 		}
 	}

numfmt_test.go

@@ -1005,7 +1005,7 @@ func TestNumFmt(t *testing.T) {
 		{"-8.0450685976001E-21", "0_);[Red]\\(0\\)", "(0)"},
 		{"-8.04506", "0_);[Red]\\(0\\)", "(8)"},
 	} {
-		result := format(item[0], item[1], false)
+		result := format(item[0], item[1], false, CellTypeNumber)
 		assert.Equal(t, item[2], result, item)
 	}
 }