From 6732132cf9cb3e2c461bc6f8c3beb0eb13155908 Mon Sep 17 00:00:00 2001 From: barry3406 Date: Sat, 11 Apr 2026 08:26:06 -0700 Subject: [PATCH] fix(context): skip chmod on existing dir in SaveUploadedFile Since v1.12.0, SaveUploadedFile calls os.Chmod on the destination's parent directory unconditionally after os.MkdirAll. For pre-existing directories the caller does not own (e.g. /tmp), that fails with "operation not permitted" and breaks a very common usage pattern. Stat the directory before MkdirAll and only chmod when MkdirAll actually created it, preserving the configurable mode behavior from #4088 for newly created directories. Fixes #4622 --- context.go | 9 +++++-- context_test.go | 62 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5174033e..8494ff32 100644 --- a/context.go +++ b/context.go @@ -728,11 +728,16 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm mode = perm[0] } dir := filepath.Dir(dst) + _, statErr := os.Stat(dir) if err = os.MkdirAll(dir, mode); err != nil { return err } - if err = os.Chmod(dir, mode); err != nil { - return err + // Only chmod the directory when MkdirAll actually created it; otherwise + // saving into a pre-existing dir we do not own (e.g. /tmp) fails. See #4622. + if errors.Is(statErr, os.ErrNotExist) { + 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..1a845f3e 100644 --- a/context_test.go +++ b/context_test.go @@ -274,6 +274,68 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) } +// TestSaveUploadedFilePreservesExistingDirPermissions is a regression test for +// https://github.com/gin-gonic/gin/issues/4622. SaveUploadedFile must not chmod +// a directory that already exists (e.g. /tmp), otherwise it fails with +// "operation not permitted" when the process does not own that directory. +func TestSaveUploadedFilePreservesExistingDirPermissions(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "existing_dir_test") + require.NoError(t, err) + _, err = w.Write([]byte("existing_dir_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) + + // Create a pre-existing directory with a mode that differs from the one + // we pass to SaveUploadedFile, mimicking a non-owned target like /tmp. + const existingMode fs.FileMode = 0o700 + dir := t.TempDir() + require.NoError(t, os.Chmod(dir, existingMode)) + + var mode fs.FileMode = 0o755 + require.NoError(t, c.SaveUploadedFile(f, filepath.Join(dir, "existing_dir_test"), mode)) + + info, err := os.Stat(dir) + require.NoError(t, err) + assert.Equal(t, existingMode, info.Mode().Perm(), "pre-existing directory mode must not be modified") + + content, err := os.ReadFile(filepath.Join(dir, "existing_dir_test")) + require.NoError(t, err) + assert.Equal(t, "existing_dir_test", string(content)) +} + +// TestSaveUploadedFileAppliesModeToCreatedDir ensures we still apply the +// requested mode when SaveUploadedFile actually creates the destination +// directory, preserving the behavior introduced in #4088. +func TestSaveUploadedFileAppliesModeToCreatedDir(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "created_dir_test") + require.NoError(t, err) + _, err = w.Write([]byte("created_dir_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) + + nested := filepath.Join(t.TempDir(), "nested") + var mode fs.FileMode = 0o755 + require.NoError(t, c.SaveUploadedFile(f, filepath.Join(nested, "created_dir_test"), mode)) + + info, err := os.Stat(nested) + require.NoError(t, err) + assert.Equal(t, mode, info.Mode().Perm()) +} + func TestContextReset(t *testing.T) { router := New() c := router.allocateContext(0)