From fe12f4df2909c62310db9c963bb655ee0adcde77 Mon Sep 17 00:00:00 2001 From: Mike Ma Date: Wed, 6 May 2026 22:01:51 -0500 Subject: [PATCH] Avoid chmod on pre-existing upload destination directories When SaveUploadedFile targets a path under an existing directory, applying the optional permission mode to that existing directory can fail despite the file creation itself being allowed. This change only creates and chmods the destination directory when it does not already exist. The permission tests now cover both newly created upload directories and existing destination directories whose mode should be preserved. Tested: docker run --rm --user "$(id -u):$(id -g)" -e GOMODCACHE=/tmp/gomodcache -e GOCACHE=/tmp/gocache -v "$PWD":/src -w /src golang:1.25.0 go test . -count=1 --- context.go | 17 ++++++++++++----- context_test.go | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/context.go b/context.go index 5174033e..e6e99698 100644 --- a/context.go +++ b/context.go @@ -728,11 +728,18 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm mode = perm[0] } dir := filepath.Dir(dst) - if err = os.MkdirAll(dir, mode); err != nil { - return err - } - if err = os.Chmod(dir, mode); err != nil { - return err + if dir != "." { + if _, err = os.Stat(dir); err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return err + } + if err = os.MkdirAll(dir, mode); err != nil { + return err + } + if err = os.Chmod(dir, mode); err != nil { + return err + } + } } out, err := os.Create(dst) diff --git a/context_test.go b/context_test.go index ef60379d..08800a1d 100644 --- a/context_test.go +++ b/context_test.go @@ -246,16 +246,42 @@ func TestSaveUploadedFileWithPermission(t *testing.T) { f, err := c.FormFile("file") require.NoError(t, err) assert.Equal(t, "permission_test", f.Filename) + dst := filepath.Join(t.TempDir(), "uploads", "permission_test") var mode fs.FileMode = 0o755 - require.NoError(t, c.SaveUploadedFile(f, "permission_test", mode)) - t.Cleanup(func() { - assert.NoError(t, os.Remove("permission_test")) - }) - info, err := os.Stat(filepath.Dir("permission_test")) + require.NoError(t, c.SaveUploadedFile(f, dst, mode)) + info, err := os.Stat(filepath.Dir(dst)) require.NoError(t, err) assert.Equal(t, info.Mode().Perm(), mode) } +func TestSaveUploadedFileWithPermissionExistingDirectory(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "permission_test") + require.NoError(t, err) + _, err = w.Write([]byte("permission_test")) + require.NoError(t, err) + mw.Close() + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodPost, "/", buf) + c.Request.Header.Set("Content-Type", mw.FormDataContentType()) + f, err := c.FormFile("file") + require.NoError(t, err) + assert.Equal(t, "permission_test", f.Filename) + + dstDir := filepath.Join(t.TempDir(), "uploads") + const existingMode fs.FileMode = 0o700 + require.NoError(t, os.Mkdir(dstDir, existingMode)) + + var mode fs.FileMode = 0o755 + dst := filepath.Join(dstDir, "permission_test") + require.NoError(t, c.SaveUploadedFile(f, dst, mode)) + + info, err := os.Stat(dstDir) + require.NoError(t, err) + assert.Equal(t, existingMode, info.Mode().Perm()) +} + func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { buf := new(bytes.Buffer) mw := multipart.NewWriter(buf) @@ -271,7 +297,7 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.NoError(t, err) assert.Equal(t, "permission_test", f.Filename) var mode fs.FileMode = 0o644 - require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) + require.Error(t, c.SaveUploadedFile(f, filepath.Join(t.TempDir(), "test", "permission_test"), mode)) } func TestContextReset(t *testing.T) {