From b02b79fe6de1b725d1055ed8589a9bd9a17a9ccc Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Thu, 2 Jan 2025 18:05:47 +0100 Subject: [PATCH] zip creation: avoid stat if not strictly required Signed-off-by: Nicola Murino --- internal/common/eventmanager.go | 17 ++++++++++------- internal/common/eventmanager_test.go | 4 ++-- internal/httpd/api_utils.go | 17 ++++++++++------- internal/httpd/internal_test.go | 14 +++++++------- 4 files changed, 29 insertions(+), 23 deletions(-) diff --git a/internal/common/eventmanager.go b/internal/common/eventmanager.go index d67f171e..aaa9cf5d 100644 --- a/internal/common/eventmanager.go +++ b/internal/common/eventmanager.go @@ -999,7 +999,7 @@ func getFileWriter(conn *BaseConnection, virtualPath string, expectedSize int64) return w, numFiles, truncatedSize, cancelFn, nil } -func addZipEntry(wr *zipWriterWrapper, conn *BaseConnection, entryPath, baseDir string, recursion int) error { +func addZipEntry(wr *zipWriterWrapper, conn *BaseConnection, entryPath, baseDir string, info os.FileInfo, recursion int) error { //nolint:gocyclo if entryPath == wr.Name { // skip the archive itself return nil @@ -1009,10 +1009,13 @@ func addZipEntry(wr *zipWriterWrapper, conn *BaseConnection, entryPath, baseDir return util.ErrRecursionTooDeep } recursion++ - info, err := conn.DoStat(entryPath, 1, false) - if err != nil { - eventManagerLog(logger.LevelError, "unable to add zip entry %q, stat error: %v", entryPath, err) - return err + var err error + if info == nil { + info, err = conn.DoStat(entryPath, 1, false) + if err != nil { + eventManagerLog(logger.LevelError, "unable to add zip entry %q, stat error: %v", entryPath, err) + return err + } } entryName, err := getZipEntryName(entryPath, baseDir) if err != nil { @@ -1050,7 +1053,7 @@ func addZipEntry(wr *zipWriterWrapper, conn *BaseConnection, entryPath, baseDir } for _, info := range contents { fullPath := util.CleanPath(path.Join(entryPath, info.Name())) - if err := addZipEntry(wr, conn, fullPath, baseDir, recursion); err != nil { + if err := addZipEntry(wr, conn, fullPath, baseDir, info, recursion); err != nil { eventManagerLog(logger.LevelError, "unable to add zip entry: %v", err) return err } @@ -2042,7 +2045,7 @@ func executeCompressFsActionForUser(c dataprovider.EventActionFsCompress, replac } startTime := time.Now() for _, item := range paths { - if err := addZipEntry(zipWriter, conn, item, baseDir, 0); err != nil { + if err := addZipEntry(zipWriter, conn, item, baseDir, nil, 0); err != nil { closeWriterAndUpdateQuota(writer, conn, name, "", numFiles, truncatedSize, err, operationUpload, startTime) //nolint:errcheck return err } diff --git a/internal/common/eventmanager_test.go b/internal/common/eventmanager_test.go index 44f401d4..72261bfe 100644 --- a/internal/common/eventmanager_test.go +++ b/internal/common/eventmanager_test.go @@ -1823,7 +1823,7 @@ func TestFilesystemActionErrors(t *testing.T) { Writer: zip.NewWriter(bytes.NewBuffer(nil)), Entries: map[string]bool{}, } - err = addZipEntry(wr, conn, "/adir/sub/f.dat", "/adir/sub/sub", 0) + err = addZipEntry(wr, conn, "/adir/sub/f.dat", "/adir/sub/sub", nil, 0) assert.Error(t, err) assert.Contains(t, getErrorString(err), "is outside base dir") } @@ -1833,7 +1833,7 @@ func TestFilesystemActionErrors(t *testing.T) { Writer: zip.NewWriter(bytes.NewBuffer(nil)), Entries: map[string]bool{}, } - err = addZipEntry(wr, conn, "/p1", "/", 2000) + err = addZipEntry(wr, conn, "/p1", "/", nil, 2000) assert.ErrorIs(t, err, util.ErrRecursionTooDeep) err = dataprovider.DeleteUser(username, "", "", "") diff --git a/internal/httpd/api_utils.go b/internal/httpd/api_utils.go index fa4bac08..aabea0da 100644 --- a/internal/httpd/api_utils.go +++ b/internal/httpd/api_utils.go @@ -384,7 +384,7 @@ func renderCompressedFiles(w http.ResponseWriter, conn *Connection, baseDir stri for _, file := range files { fullPath := util.CleanPath(path.Join(baseDir, file)) - if err := addZipEntry(wr, conn, fullPath, baseDir, 0); err != nil { + if err := addZipEntry(wr, conn, fullPath, baseDir, nil, 0); err != nil { if share != nil { dataprovider.UpdateShareLastUse(share, -1) //nolint:errcheck } @@ -400,16 +400,19 @@ func renderCompressedFiles(w http.ResponseWriter, conn *Connection, baseDir stri } } -func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string, recursion int) error { +func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string, info os.FileInfo, recursion int) error { if recursion >= util.MaxRecursion { conn.Log(logger.LevelDebug, "unable to add zip entry %q, recursion too depth: %d", entryPath, recursion) return util.ErrRecursionTooDeep } recursion++ - info, err := conn.Stat(entryPath, 1) - if err != nil { - conn.Log(logger.LevelDebug, "unable to add zip entry %q, stat error: %v", entryPath, err) - return err + var err error + if info == nil { + info, err = conn.Stat(entryPath, 1) + if err != nil { + conn.Log(logger.LevelDebug, "unable to add zip entry %q, stat error: %v", entryPath, err) + return err + } } entryName, err := getZipEntryName(entryPath, baseDir) if err != nil { @@ -441,7 +444,7 @@ func addZipEntry(wr *zip.Writer, conn *Connection, entryPath, baseDir string, re } for _, info := range contents { fullPath := util.CleanPath(path.Join(entryPath, info.Name())) - if err := addZipEntry(wr, conn, fullPath, baseDir, recursion); err != nil { + if err := addZipEntry(wr, conn, fullPath, baseDir, info, recursion); err != nil { return err } } diff --git a/internal/httpd/internal_test.go b/internal/httpd/internal_test.go index c17f60fd..4b087126 100644 --- a/internal/httpd/internal_test.go +++ b/internal/httpd/internal_test.go @@ -2291,14 +2291,14 @@ func TestZipErrors(t *testing.T) { assert.Contains(t, err.Error(), "write error") } - err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", 0) + err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", nil, 0) if assert.Error(t, err) { assert.Contains(t, err.Error(), "write error") } - err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", 2000) + err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", nil, 2000) assert.ErrorIs(t, err, util.ErrRecursionTooDeep) - err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), path.Join("/", filepath.Base(testDir), "dir"), 0) + err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), path.Join("/", filepath.Base(testDir), "dir"), nil, 0) if assert.Error(t, err) { assert.Contains(t, err.Error(), "is outside base dir") } @@ -2307,14 +2307,14 @@ func TestZipErrors(t *testing.T) { err = os.WriteFile(testFilePath, util.GenerateRandomBytes(65535), os.ModePerm) assert.NoError(t, err) err = addZipEntry(wr, connection, path.Join("/", filepath.Base(testDir), filepath.Base(testFilePath)), - "/"+filepath.Base(testDir), 0) + "/"+filepath.Base(testDir), nil, 0) if assert.Error(t, err) { assert.Contains(t, err.Error(), "write error") } connection.User.Permissions["/"] = []string{dataprovider.PermListItems} err = addZipEntry(wr, connection, path.Join("/", filepath.Base(testDir), filepath.Base(testFilePath)), - "/"+filepath.Base(testDir), 0) + "/"+filepath.Base(testDir), nil, 0) assert.ErrorIs(t, err, os.ErrPermission) // creating a virtual folder to a missing path stat is ok but readdir fails @@ -2326,14 +2326,14 @@ func TestZipErrors(t *testing.T) { }) connection.User = user wr = zip.NewWriter(bytes.NewBuffer(make([]byte, 0))) - err = addZipEntry(wr, connection, user.VirtualFolders[0].VirtualPath, "/", 0) + err = addZipEntry(wr, connection, user.VirtualFolders[0].VirtualPath, "/", nil, 0) assert.Error(t, err) user.Filters.FilePatterns = append(user.Filters.FilePatterns, sdk.PatternsFilter{ Path: "/", DeniedPatterns: []string{"*.zip"}, }) - err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", 0) + err = addZipEntry(wr, connection, "/"+filepath.Base(testDir), "/", nil, 0) assert.ErrorIs(t, err, os.ErrPermission) err = os.RemoveAll(testDir)