From c92f4bf41083dff65caa7d9f82b870fea52024a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ngh=C4=A9a=20Nguy=E1=BB=85n=20Ng=E1=BB=8Dc?= Date: Sat, 9 May 2026 10:02:46 +0700 Subject: [PATCH 1/2] fix(context): only chmod newly created dirs in SaveUploadedFile Fixes #4622 - SaveUploadedFile no longer attempts to modify permissions on existing directories, which breaks saving to /tmp and other system directories. - Check if directory exists before calling os.MkdirAll - Only call os.Chmod if directory was newly created - Preserves backward compatibility for newly created dirs - Allows saving files to existing system directories This regression was introduced in v1.12.0 when chmod support was added, but was incorrectly applied to all directories. BREAKING CHANGE REVERTED: Restores v1.10.1 behavior for existing dirs Co-authored-by: Nghia Nguyen --- context.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5174033e..368dc719 100644 --- a/context.go +++ b/context.go @@ -728,11 +728,17 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm mode = perm[0] } dir := filepath.Dir(dst) + // Check if directory exists to only chmod newly created directories + _, statErr := os.Stat(dir) + dirExists := !os.IsNotExist(statErr) if err = os.MkdirAll(dir, mode); err != nil { return err } - if err = os.Chmod(dir, mode); err != nil { - return err + // Only chmod if directory was newly created (fixes #4622) + if !dirExists { + if err = os.Chmod(dir, mode); err != nil { + return err + } } out, err := os.Create(dst) From ec746b406056edd05042f58199ed834d8a085fa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ngh=C4=A9a=20Nguy=E1=BB=85n=20Ng=E1=BB=8Dc?= Date: Sat, 9 May 2026 10:12:56 +0700 Subject: [PATCH 2/2] test(context): cover SaveUploadedFile chmod branches Add a regression test for #4622 that asserts SaveUploadedFile leaves permissions on a pre-existing target directory untouched. Make TestSaveUploadedFileWithPermission hermetic by saving into a not-yet-existing subdirectory under t.TempDir(), so the assertion on the requested mode is meaningful regardless of the test runner's working-directory permissions (the previous form relied on the soon-to-be-removed chmod-on-existing-dir behaviour to pass). --- context_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/context_test.go b/context_test.go index ef60379d..2a562c73 100644 --- a/context_test.go +++ b/context_test.go @@ -246,14 +246,19 @@ func TestSaveUploadedFileWithPermission(t *testing.T) { f, err := c.FormFile("file") require.NoError(t, err) assert.Equal(t, "permission_test", f.Filename) + + // Save into a not-yet-existing subdirectory so the requested mode is + // applied by MkdirAll/Chmod and the assertion is independent of the + // test runner's working-directory permissions. + parent := t.TempDir() + newDir := filepath.Join(parent, "perm_subdir") + dst := filepath.Join(newDir, "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(newDir) require.NoError(t, err) - assert.Equal(t, info.Mode().Perm(), mode) + assert.Equal(t, mode, info.Mode().Perm()) } func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { @@ -274,6 +279,39 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) } +// TestSaveUploadedFileExistingDir verifies that SaveUploadedFile does not +// alter permissions on a pre-existing target directory (regression test for +// #4622, where chmod on a system dir like /tmp would fail). +func TestSaveUploadedFileExistingDir(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 target directory with a non-default permission and + // confirm SaveUploadedFile leaves the existing perm untouched. + dir := t.TempDir() + var existingPerm fs.FileMode = 0o700 + require.NoError(t, os.Chmod(dir, existingPerm)) + + dst := filepath.Join(dir, "existing_dir_test") + var mode fs.FileMode = 0o755 + require.NoError(t, c.SaveUploadedFile(f, dst, mode)) + + info, err := os.Stat(dir) + require.NoError(t, err) + assert.Equal(t, existingPerm, info.Mode().Perm(), + "existing directory permissions must not be modified") +} + func TestContextReset(t *testing.T) { router := New() c := router.allocateContext(0)