Fix #539 Fixed error opening excel file created in encoding d… (#540)

* Fixed issue #539 Fixed error opening excel file created in encoding different from UTF-8, added logging of possible errors when decoding XML if the function does not provide exit with an error

* Added test for CharsetReader

* Fixed #discussion_r359397878

Discussion: #540 (comment)

* Fixed go fmt

* go mod tidy and removed unused imports

* The code has been refactored

.gitignore

@@ -1,4 +1,5 @@
 ~$*.xlsx
 test/Test*.xlsx
 *.out
-*.test
+*.test
+.idea

calcchain.go

@@ -9,16 +9,26 @@
 
 package excelize
 
-import "encoding/xml"
+import (
+	"bytes"
+	"encoding/xml"
+	"io"
+	"log"
+)
 
 // calcChainReader provides a function to get the pointer to the structure
 // after deserialization of xl/calcChain.xml.
 func (f *File) calcChainReader() *xlsxCalcChain {
+	var err error
+
 	if f.CalcChain == nil {
-		var c xlsxCalcChain
-		_ = xml.Unmarshal(namespaceStrictToTransitional(f.readXML("xl/calcChain.xml")), &c)
-		f.CalcChain = &c
+		f.CalcChain = new(xlsxCalcChain)
+		if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML("xl/calcChain.xml")))).
+			Decode(f.CalcChain); err != nil && err != io.EOF {
+			log.Printf("xml decode error: %s", err)
+		}
 	}
+
 	return f.CalcChain
 }
 

chart.go

@@ -10,9 +10,12 @@
 package excelize
 
 import (
+	"bytes"
 	"encoding/json"
 	"encoding/xml"
 	"errors"
+	"io"
+	"log"
 	"strconv"
 	"strings"
 )
@@ -1735,14 +1738,21 @@ func (f *File) drawPlotAreaTxPr() *cTxPr {
 // deserialization, two different structures: decodeWsDr and encodeWsDr are
 // defined.
 func (f *File) drawingParser(path string) (*xlsxWsDr, int) {
+	var (
+		err error
+		ok  bool
+	)
+
 	if f.Drawings[path] == nil {
 		content := xlsxWsDr{}
 		content.A = NameSpaceDrawingML
 		content.Xdr = NameSpaceDrawingMLSpreadSheet
-		_, ok := f.XLSX[path]
-		if ok { // Append Model
+		if _, ok = f.XLSX[path]; ok { // Append Model
 			decodeWsDr := decodeWsDr{}
-			_ = xml.Unmarshal(namespaceStrictToTransitional(f.readXML(path)), &decodeWsDr)
+			if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(path)))).
+				Decode(&decodeWsDr); err != nil && err != io.EOF {
+				log.Printf("xml decode error: %s", err)
+			}
 			content.R = decodeWsDr.R
 			for _, v := range decodeWsDr.OneCellAnchor {
 				content.OneCellAnchor = append(content.OneCellAnchor, &xdrCellAnchor{

comment.go

@@ -10,9 +10,12 @@
 package excelize
 
 import (
+	"bytes"
 	"encoding/json"
 	"encoding/xml"
 	"fmt"
+	"io"
+	"log"
 	"strconv"
 	"strings"
 )
@@ -303,12 +306,16 @@ func (f *File) countComments() int {
 // decodeVMLDrawingReader provides a function to get the pointer to the
 // structure after deserialization of xl/drawings/vmlDrawing%d.xml.
 func (f *File) decodeVMLDrawingReader(path string) *decodeVmlDrawing {
+	var err error
+
 	if f.DecodeVMLDrawing[path] == nil {
 		c, ok := f.XLSX[path]
 		if ok {
-			d := decodeVmlDrawing{}
-			_ = xml.Unmarshal(namespaceStrictToTransitional(c), &d)
-			f.DecodeVMLDrawing[path] = &d
+			f.DecodeVMLDrawing[path] = new(decodeVmlDrawing)
+			if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(c))).
+				Decode(f.DecodeVMLDrawing[path]); err != nil && err != io.EOF {
+				log.Printf("xml decode error: %s", err)
+			}
 		}
 	}
 	return f.DecodeVMLDrawing[path]
@@ -328,12 +335,16 @@ func (f *File) vmlDrawingWriter() {
 // commentsReader provides a function to get the pointer to the structure
 // after deserialization of xl/comments%d.xml.
 func (f *File) commentsReader(path string) *xlsxComments {
+	var err error
+
 	if f.Comments[path] == nil {
 		content, ok := f.XLSX[path]
 		if ok {
-			c := xlsxComments{}
-			_ = xml.Unmarshal(namespaceStrictToTransitional(content), &c)
-			f.Comments[path] = &c
+			f.Comments[path] = new(xlsxComments)
+			if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(content))).
+				Decode(f.Comments[path]); err != nil && err != io.EOF {
+				log.Printf("xml decode error: %s", err)
+			}
 		}
 	}
 	return f.Comments[path]

docProps.go

@@ -10,7 +10,10 @@
 package excelize
 
 import (
+	"bytes"
 	"encoding/xml"
+	"fmt"
+	"io"
 	"reflect"
 )
 
@@ -65,13 +68,23 @@ import (
 //        Version:        "1.0.0",
 //    })
 //
-func (f *File) SetDocProps(docProperties *DocProperties) error {
-	core := decodeCoreProperties{}
-	err := xml.Unmarshal(namespaceStrictToTransitional(f.readXML("docProps/core.xml")), &core)
-	if err != nil {
-		return err
+func (f *File) SetDocProps(docProperties *DocProperties) (err error) {
+	var (
+		core               *decodeCoreProperties
+		newProps           *xlsxCoreProperties
+		fields             []string
+		output             []byte
+		immutable, mutable reflect.Value
+		field, val         string
+	)
+
+	core = new(decodeCoreProperties)
+	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML("docProps/core.xml")))).
+		Decode(core); err != nil && err != io.EOF {
+		err = fmt.Errorf("xml decode error: %s", err)
+		return
 	}
-	newProps := xlsxCoreProperties{
+	newProps, err = &xlsxCoreProperties{
 		Dc:             NameSpaceDublinCore,
 		Dcterms:        NameSpaceDublinCoreTerms,
 		Dcmitype:       NameSpaceDublinCoreMetadataIntiative,
@@ -88,18 +101,16 @@ func (f *File) SetDocProps(docProperties *DocProperties) error {
 		ContentStatus:  core.ContentStatus,
 		Category:       core.Category,
 		Version:        core.Version,
+	}, nil
+	newProps.Created.Text, newProps.Created.Type, newProps.Modified.Text, newProps.Modified.Type =
+		core.Created.Text, core.Created.Type, core.Modified.Text, core.Modified.Type
+	fields = []string{
+		"Category", "ContentStatus", "Creator", "Description", "Identifier", "Keywords",
+		"LastModifiedBy", "Revision", "Subject", "Title", "Language", "Version",
 	}
-	newProps.Created.Text = core.Created.Text
-	newProps.Created.Type = core.Created.Type
-	newProps.Modified.Text = core.Modified.Text
-	newProps.Modified.Type = core.Modified.Type
-
-	fields := []string{"Category", "ContentStatus", "Creator", "Description", "Identifier", "Keywords", "LastModifiedBy", "Revision", "Subject", "Title", "Language", "Version"}
-	immutable := reflect.ValueOf(*docProperties)
-	mutable := reflect.ValueOf(&newProps).Elem()
-	for _, field := range fields {
-		val := immutable.FieldByName(field).String()
-		if val != "" {
+	immutable, mutable = reflect.ValueOf(*docProperties), reflect.ValueOf(newProps).Elem()
+	for _, field = range fields {
+		if val = immutable.FieldByName(field).String(); val != "" {
 			mutable.FieldByName(field).SetString(val)
 		}
 	}
@@ -109,19 +120,22 @@ func (f *File) SetDocProps(docProperties *DocProperties) error {
 	if docProperties.Modified != "" {
 		newProps.Modified.Text = docProperties.Modified
 	}
-	output, err := xml.Marshal(&newProps)
+	output, err = xml.Marshal(newProps)
 	f.saveFileList("docProps/core.xml", output)
-	return err
+
+	return
 }
 
 // GetDocProps provides a function to get document core properties.
-func (f *File) GetDocProps() (*DocProperties, error) {
-	core := decodeCoreProperties{}
-	err := xml.Unmarshal(namespaceStrictToTransitional(f.readXML("docProps/core.xml")), &core)
-	if err != nil {
-		return nil, err
+func (f *File) GetDocProps() (ret *DocProperties, err error) {
+	var core = new(decodeCoreProperties)
+
+	if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML("docProps/core.xml")))).
+		Decode(core); err != nil && err != io.EOF {
+		err = fmt.Errorf("xml decode error: %s", err)
+		return
 	}
-	return &DocProperties{
+	ret, err = &DocProperties{
 		Category:       core.Category,
 		ContentStatus:  core.ContentStatus,
 		Created:        core.Created.Text,
@@ -137,4 +151,6 @@ func (f *File) GetDocProps() (*DocProperties, error) {
 		Language:       core.Language,
 		Version:        core.Version,
 	}, nil
+
+	return
 }

docProps_test.go

@@ -39,7 +39,7 @@ func TestSetDocProps(t *testing.T) {
 	}))
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestSetDocProps.xlsx")))
 	f.XLSX["docProps/core.xml"] = nil
-	assert.EqualError(t, f.SetDocProps(&DocProperties{}), "EOF")
+	assert.NoError(t, f.SetDocProps(&DocProperties{}))
 }
 
 func TestGetDocProps(t *testing.T) {
@@ -52,5 +52,5 @@ func TestGetDocProps(t *testing.T) {
 	assert.Equal(t, props.Creator, "Microsoft Office User")
 	f.XLSX["docProps/core.xml"] = nil
 	_, err = f.GetDocProps()
-	assert.EqualError(t, err, "EOF")
+	assert.NoError(t, err)
 }

excelize.go

@@ -22,6 +22,8 @@ import (
 	"path"
 	"strconv"
 	"strings"
+
+	"golang.org/x/net/html/charset"
 )
 
 // File define a populated XLSX file struct.
@@ -43,8 +45,11 @@ type File struct {
 	WorkBook         *xlsxWorkbook
 	Relationships    map[string]*xlsxRelationships
 	XLSX             map[string][]byte
+	CharsetReader    charsetTranscoderFn
 }
 
+type charsetTranscoderFn func(charset string, input io.Reader) (rdr io.Reader, err error)
+
 // OpenFile take the name of an XLSX file and returns a populated XLSX file
 // struct for it.
 func OpenFile(filename string) (*File, error) {
@@ -61,6 +66,21 @@ func OpenFile(filename string) (*File, error) {
 	return f, nil
 }
 
+// object builder
+func newFile() *File {
+	return &File{
+		checked:          make(map[string]bool),
+		sheetMap:         make(map[string]string),
+		Comments:         make(map[string]*xlsxComments),
+		Drawings:         make(map[string]*xlsxWsDr),
+		Sheet:            make(map[string]*xlsxWorksheet),
+		DecodeVMLDrawing: make(map[string]*decodeVmlDrawing),
+		VMLDrawing:       make(map[string]*vmlDrawing),
+		Relationships:    make(map[string]*xlsxRelationships),
+		CharsetReader:    charset.NewReaderLabel,
+	}
+}
+
 // OpenReader take an io.Reader and return a populated XLSX file.
 func OpenReader(r io.Reader) (*File, error) {
 	b, err := ioutil.ReadAll(r)
@@ -88,24 +108,25 @@ func OpenReader(r io.Reader) (*File, error) {
 	if err != nil {
 		return nil, err
 	}
-	f := &File{
-		checked:          make(map[string]bool),
-		Comments:         make(map[string]*xlsxComments),
-		Drawings:         make(map[string]*xlsxWsDr),
-		Sheet:            make(map[string]*xlsxWorksheet),
-		SheetCount:       sheetCount,
-		DecodeVMLDrawing: make(map[string]*decodeVmlDrawing),
-		VMLDrawing:       make(map[string]*vmlDrawing),
-		Relationships:    make(map[string]*xlsxRelationships),
-		XLSX:             file,
-	}
+	f := newFile()
+	f.SheetCount, f.XLSX = sheetCount, file
 	f.CalcChain = f.calcChainReader()
 	f.sheetMap = f.getSheetMap()
 	f.Styles = f.stylesReader()
 	f.Theme = f.themeReader()
 	return f, nil
 }
 
+// CharsetTranscoder Set user defined codepage transcoder function for open XLSX from non UTF-8 encoding
+func (f *File) CharsetTranscoder(fn charsetTranscoderFn) *File { f.CharsetReader = fn; return f }
+
+// Creates new XML decoder with charset reader
+func (f *File) xmlNewDecoder(rdr io.Reader) (ret *xml.Decoder) {
+	ret = xml.NewDecoder(rdr)
+	ret.CharsetReader = f.CharsetReader
+	return
+}
+
 // setDefaultTimeStyle provides a function to set default numbers format for
 // time.Time type cell value by given worksheet name, cell coordinates and
 // number format code.
@@ -123,26 +144,38 @@ func (f *File) setDefaultTimeStyle(sheet, axis string, format int) error {
 
 // workSheetReader provides a function to get the pointer to the structure
 // after deserialization by given worksheet name.
-func (f *File) workSheetReader(sheet string) (*xlsxWorksheet, error) {
-	name, ok := f.sheetMap[trimSheetName(sheet)]
-	if !ok {
-		return nil, fmt.Errorf("sheet %s is not exist", sheet)
+func (f *File) workSheetReader(sheet string) (xlsx *xlsxWorksheet, err error) {
+	var (
+		name string
+		ok   bool
+	)
+
+	if name, ok = f.sheetMap[trimSheetName(sheet)]; !ok {
+		err = fmt.Errorf("sheet %s is not exist", sheet)
+		return
 	}
-	if f.Sheet[name] == nil {
-		var xlsx xlsxWorksheet
-		_ = xml.Unmarshal(namespaceStrictToTransitional(f.readXML(name)), &xlsx)
+	if xlsx = f.Sheet[name]; f.Sheet[name] == nil {
+		xlsx = new(xlsxWorksheet)
+		if err = f.xmlNewDecoder(bytes.NewReader(namespaceStrictToTransitional(f.readXML(name)))).
+			Decode(xlsx); err != nil && err != io.EOF {
+			err = fmt.Errorf("xml decode error: %s", err)
+			return
+		}
+		err = nil
 		if f.checked == nil {
 			f.checked = make(map[string]bool)
 		}
-		ok := f.checked[name]
-		if !ok {
-			checkSheet(&xlsx)
-			checkRow(&xlsx)
+		if ok = f.checked[name]; !ok {
+			checkSheet(xlsx)
+			if err = checkRow(xlsx); err != nil {
+				return
+			}
 			f.checked[name] = true
 		}
-		f.Sheet[name] = &xlsx
+		f.Sheet[name] = xlsx
 	}
-	return f.Sheet[name], nil
+
+	return
 }
 
 // checkSheet provides a function to fill each row element and make that is

file.go

@@ -33,12 +33,8 @@ func NewFile() *File {
 	file["xl/styles.xml"] = []byte(XMLHeader + templateStyles)
 	file["xl/workbook.xml"] = []byte(XMLHeader + templateWorkbook)
 	file["[Content_Types].xml"] = []byte(XMLHeader + templateContentTypes)
-	f := &File{
-		sheetMap:   make(map[string]string),
-		Sheet:      make(map[string]*xlsxWorksheet),
-		SheetCount: 1,
-		XLSX:       file,
-	}
+	f := newFile()
+	f.SheetCount, f.XLSX = 1, file
 	f.CalcChain = f.calcChainReader()
 	f.Comments = make(map[string]*xlsxComments)
 	f.ContentTypes = f.contentTypesReader()