From 87a00e4f7e0aca8c5c52bbca4c706bf6b7a4a1b7 Mon Sep 17 00:00:00 2001 From: xuri Date: Mon, 9 Oct 2023 00:14:56 +0800 Subject: [PATCH] This closed #1680, fixing a potential issue that stream reader temporary files can not be clear - Delete image files from the workbook internally when deleting pictures to reduce generated workbook size and resolve potential security issues --- chart.go | 3 ++- chart_test.go | 8 ++++--- drawing.go | 59 +++++++++++++++++++++++++++++++++++++++++++------ drawing_test.go | 9 ++++++++ lib.go | 10 +++++++-- picture.go | 34 +++++++++++++++++++++++++--- picture_test.go | 45 +++++++++++++++++++++++++++++++++++-- 7 files changed, 150 insertions(+), 18 deletions(-) diff --git a/chart.go b/chart.go index 4cfac96000..f2473f162b 100644 --- a/chart.go +++ b/chart.go @@ -1109,7 +1109,8 @@ func (f *File) DeleteChart(sheet, cell string) error { return err } drawingXML := strings.ReplaceAll(f.getSheetRelationshipsTargetByID(sheet, ws.Drawing.RID), "..", "xl") - return f.deleteDrawing(col, row, drawingXML, "Chart") + _, err = f.deleteDrawing(col, row, drawingXML, "Chart") + return err } // countCharts provides a function to get chart files count storage in the diff --git a/chart_test.go b/chart_test.go index a860fa55ef..4516f5c56e 100644 --- a/chart_test.go +++ b/chart_test.go @@ -120,13 +120,15 @@ func TestDeleteDrawing(t *testing.T) { f := NewFile() 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")) + _, err := f.deleteDrawing(0, 0, path, "Chart") + assert.EqualError(t, err, "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") + _, err = f.deleteDrawing(0, 0, path, "Chart") + assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8") } func TestAddChart(t *testing.T) { diff --git a/drawing.go b/drawing.go index 045506c860..c7bf88a0c9 100644 --- a/drawing.go +++ b/drawing.go @@ -1391,11 +1391,14 @@ func (f *File) addSheetDrawingChart(drawingXML string, rID int, opts *GraphicOpt return err } -// deleteDrawing provides a function to delete chart graphic frame by given by +// deleteDrawing provides a function to delete the chart graphic frame and +// returns deleted embed relationships ID (for unique picture cell anchor) by // given coordinates and graphic type. -func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) error { +func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) (string, error) { var ( err error + rID string + rIDs []string wsDr *xlsxWsDr deTwoCellAnchor *decodeCellAnchor ) @@ -1407,32 +1410,74 @@ func (f *File) deleteDrawing(col, row int, drawingXML, drawingType string) error "Chart": func(anchor *decodeCellAnchor) bool { return anchor.Pic == nil }, "Pic": func(anchor *decodeCellAnchor) bool { return anchor.Pic != nil }, } + onAnchorCell := func(c, r int) bool { return c == col && r == row } if wsDr, _, err = f.drawingParser(drawingXML); err != nil { - return err + return rID, err } for idx := 0; idx < len(wsDr.TwoCellAnchor); idx++ { if err = nil; wsDr.TwoCellAnchor[idx].From != nil && xdrCellAnchorFuncs[drawingType](wsDr.TwoCellAnchor[idx]) { - if wsDr.TwoCellAnchor[idx].From.Col == col && wsDr.TwoCellAnchor[idx].From.Row == row { + if onAnchorCell(wsDr.TwoCellAnchor[idx].From.Col, wsDr.TwoCellAnchor[idx].From.Row) { + rID, _ = extractEmbedRID(wsDr.TwoCellAnchor[idx].Pic, nil, rIDs) wsDr.TwoCellAnchor = append(wsDr.TwoCellAnchor[:idx], wsDr.TwoCellAnchor[idx+1:]...) idx-- + continue } + _, rIDs = extractEmbedRID(wsDr.TwoCellAnchor[idx].Pic, nil, rIDs) } } for idx := 0; idx < len(wsDr.TwoCellAnchor); idx++ { deTwoCellAnchor = new(decodeCellAnchor) if err = f.xmlNewDecoder(strings.NewReader("" + wsDr.TwoCellAnchor[idx].GraphicFrame + "")). Decode(deTwoCellAnchor); err != nil && err != io.EOF { - return err + return rID, err } if err = nil; deTwoCellAnchor.From != nil && decodeCellAnchorFuncs[drawingType](deTwoCellAnchor) { - if deTwoCellAnchor.From.Col == col && deTwoCellAnchor.From.Row == row { + if onAnchorCell(deTwoCellAnchor.From.Col, deTwoCellAnchor.From.Row) { + rID, _ = extractEmbedRID(nil, deTwoCellAnchor.Pic, rIDs) wsDr.TwoCellAnchor = append(wsDr.TwoCellAnchor[:idx], wsDr.TwoCellAnchor[idx+1:]...) idx-- + continue } + _, rIDs = extractEmbedRID(nil, deTwoCellAnchor.Pic, rIDs) } } + if inStrSlice(rIDs, rID, true) != -1 { + rID = "" + } f.Drawings.Store(drawingXML, wsDr) - return err + return rID, err +} + +// extractEmbedRID returns embed relationship ID and all relationship ID lists +// for giving cell anchor. +func extractEmbedRID(pic *xlsxPic, decodePic *decodePic, rIDs []string) (string, []string) { + if pic != nil { + rIDs = append(rIDs, pic.BlipFill.Blip.Embed) + return pic.BlipFill.Blip.Embed, rIDs + } + if decodePic != nil { + rIDs = append(rIDs, decodePic.BlipFill.Blip.Embed) + return decodePic.BlipFill.Blip.Embed, rIDs + } + return "", rIDs +} + +// deleteDrawingRels provides a function to delete relationships in +// xl/drawings/_rels/drawings%d.xml.rels by giving drawings relationships path +// and relationship ID. +func (f *File) deleteDrawingRels(rels, rID string) { + drawingRels, _ := f.relsReader(rels) + if drawingRels == nil { + drawingRels = &xlsxRelationships{} + } + drawingRels.mu.Lock() + defer drawingRels.mu.Unlock() + for k, v := range drawingRels.Relationships { + if v.ID == rID { + drawingRels.Relationships = append(drawingRels.Relationships[:k], drawingRels.Relationships[k+1:]...) + } + } + f.Relationships.Store(rels, drawingRels) } // genAxID provides a function to generate ID for primary and secondary diff --git a/drawing_test.go b/drawing_test.go index 7fcee82970..90846702cd 100644 --- a/drawing_test.go +++ b/drawing_test.go @@ -38,3 +38,12 @@ func TestDrawingParser(t *testing.T) { _, _, err = f.drawingParser("wsDr") assert.NoError(t, err) } + +func TestDeleteDrawingRels(t *testing.T) { + f := NewFile() + // Test delete drawing relationships with unsupported charset + rels := "xl/drawings/_rels/drawing1.xml.rels" + f.Relationships.Delete(rels) + f.Pkg.Store(rels, MacintoshCyrillicCharset) + f.deleteDrawingRels(rels, "") +} diff --git a/lib.go b/lib.go index 98f48c80d6..83c28e7c0b 100644 --- a/lib.go +++ b/lib.go @@ -48,16 +48,22 @@ func (f *File) ReadZipReader(r *zip.Reader) (map[string][]byte, int, error) { fileName = partName } if strings.EqualFold(fileName, defaultXMLPathSharedStrings) && fileSize > f.options.UnzipXMLSizeLimit { - if tempFile, err := f.unzipToTemp(v); err == nil { + tempFile, err := f.unzipToTemp(v) + if tempFile != "" { f.tempFiles.Store(fileName, tempFile) + } + if err == nil { continue } } if strings.HasPrefix(strings.ToLower(fileName), "xl/worksheets/sheet") { worksheets++ if fileSize > f.options.UnzipXMLSizeLimit && !v.FileInfo().IsDir() { - if tempFile, err := f.unzipToTemp(v); err == nil { + tempFile, err := f.unzipToTemp(v) + if tempFile != "" { f.tempFiles.Store(fileName, tempFile) + } + if err == nil { continue } } diff --git a/picture.go b/picture.go index c289850727..b263aa1328 100644 --- a/picture.go +++ b/picture.go @@ -468,8 +468,7 @@ func (f *File) GetPictures(sheet, cell string) ([]Picture, error) { } // DeletePicture provides a function to delete all pictures in a cell by given -// worksheet name and cell reference. Note that the image file won't be deleted -// from the document currently. +// worksheet name and cell reference. func (f *File) DeletePicture(sheet, cell string) error { col, row, err := CellNameToCoordinates(cell) if err != nil { @@ -485,7 +484,36 @@ func (f *File) DeletePicture(sheet, cell string) error { return err } drawingXML := strings.ReplaceAll(f.getSheetRelationshipsTargetByID(sheet, ws.Drawing.RID), "..", "xl") - return f.deleteDrawing(col, row, drawingXML, "Pic") + drawingRels := "xl/drawings/_rels/" + filepath.Base(drawingXML) + ".rels" + rID, err := f.deleteDrawing(col, row, drawingXML, "Pic") + if err != nil { + return err + } + rels := f.getDrawingRelationships(drawingRels, rID) + if rels == nil { + return err + } + var used bool + f.Pkg.Range(func(k, v interface{}) bool { + if strings.Contains(k.(string), "xl/drawings/_rels/") { + r, err := f.relsReader(k.(string)) + if err != nil { + return true + } + for _, rel := range r.Relationships { + if rel.ID != rels.ID && rel.Type == SourceRelationshipImage && + filepath.Base(rel.Target) == filepath.Base(rels.Target) { + used = true + } + } + } + return true + }) + if !used { + f.Pkg.Delete(strings.Replace(rels.Target, "../", "xl/", -1)) + } + f.deleteDrawingRels(drawingRels, rID) + return err } // getPicture provides a function to get picture base name and raw content diff --git a/picture_test.go b/picture_test.go index cd070d5ccf..8e7e3a9787 100644 --- a/picture_test.go +++ b/picture_test.go @@ -240,10 +240,25 @@ func TestAddPictureFromBytes(t *testing.T) { func TestDeletePicture(t *testing.T) { f, err := OpenFile(filepath.Join("test", "Book1.xlsx")) assert.NoError(t, err) + // Test delete picture on a worksheet which does not contains any pictures assert.NoError(t, f.DeletePicture("Sheet1", "A1")) - assert.NoError(t, f.AddPicture("Sheet1", "P1", filepath.Join("test", "images", "excel.jpg"), nil)) - assert.NoError(t, f.DeletePicture("Sheet1", "P1")) + // Add same pictures on different worksheets + assert.NoError(t, f.AddPicture("Sheet1", "F20", filepath.Join("test", "images", "excel.jpg"), nil)) + assert.NoError(t, f.AddPicture("Sheet1", "I20", filepath.Join("test", "images", "excel.jpg"), nil)) + assert.NoError(t, f.AddPicture("Sheet2", "F1", filepath.Join("test", "images", "excel.jpg"), nil)) + // Test delete picture on a worksheet, the images should be preserved + assert.NoError(t, f.DeletePicture("Sheet1", "F20")) assert.NoError(t, f.SaveAs(filepath.Join("test", "TestDeletePicture.xlsx"))) + assert.NoError(t, f.Close()) + + f, err = OpenFile(filepath.Join("test", "TestDeletePicture.xlsx")) + assert.NoError(t, err) + // Test delete same picture on different worksheet, the images should be removed + assert.NoError(t, f.DeletePicture("Sheet1", "F10")) + assert.NoError(t, f.DeletePicture("Sheet2", "F1")) + assert.NoError(t, f.DeletePicture("Sheet1", "I20")) + assert.NoError(t, f.SaveAs(filepath.Join("test", "TestDeletePicture2.xlsx"))) + // Test delete picture on not exists worksheet assert.EqualError(t, f.DeletePicture("SheetN", "A1"), "sheet SheetN does not exist") // Test delete picture with invalid sheet name @@ -253,6 +268,32 @@ func TestDeletePicture(t *testing.T) { assert.NoError(t, f.Close()) // Test delete picture on no chart worksheet assert.NoError(t, NewFile().DeletePicture("Sheet1", "A1")) + + f, err = OpenFile(filepath.Join("test", "TestDeletePicture.xlsx")) + assert.NoError(t, err) + // Test delete picture with unsupported charset drawing + f.Pkg.Store("xl/drawings/drawing1.xml", MacintoshCyrillicCharset) + assert.EqualError(t, f.DeletePicture("Sheet1", "F10"), "XML syntax error on line 1: invalid UTF-8") + assert.NoError(t, f.Close()) + + f, err = OpenFile(filepath.Join("test", "TestDeletePicture.xlsx")) + assert.NoError(t, err) + // Test delete picture with unsupported charset drawing relationships + f.Relationships.Delete("xl/drawings/_rels/drawing1.xml.rels") + f.Pkg.Store("xl/drawings/_rels/drawing1.xml.rels", MacintoshCyrillicCharset) + assert.NoError(t, f.DeletePicture("Sheet2", "F1")) + assert.NoError(t, f.Close()) + + f = NewFile() + assert.NoError(t, err) + assert.NoError(t, f.AddPicture("Sheet1", "A1", filepath.Join("test", "images", "excel.jpg"), nil)) + assert.NoError(t, f.AddPicture("Sheet1", "G1", filepath.Join("test", "images", "excel.jpg"), nil)) + drawing, ok := f.Drawings.Load("xl/drawings/drawing1.xml") + assert.True(t, ok) + // Made two picture reference the same drawing relationship ID + drawing.(*xlsxWsDr).TwoCellAnchor[1].Pic.BlipFill.Blip.Embed = "rId1" + assert.NoError(t, f.DeletePicture("Sheet1", "A1")) + assert.NoError(t, f.Close()) } func TestDrawingResize(t *testing.T) {