diff --git a/context.go b/context.go index a2e28e5b..2ea259bd 100644 --- a/context.go +++ b/context.go @@ -716,6 +716,10 @@ func (c *Context) MultipartForm() (*multipart.Form, error) { } // SaveUploadedFile uploads the form file to specific dst. +// An optional perm argument allows specifying the permission bits for any +// newly created directories. If not provided, the default is 0750. +// Note: permissions are only applied to directories created by this call; +// pre-existing directories (e.g. /tmp) are not modified. func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm ...fs.FileMode) error { src, err := file.Open() if err != nil { @@ -728,11 +732,20 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm mode = perm[0] } dir := filepath.Dir(dst) + // Check if the directory already exists before calling MkdirAll so we + // know whether to apply chmod. We must not chmod pre-existing directories + // (e.g. /tmp) because the process may not own them. + _, 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 apply chmod when the directory was freshly created by MkdirAll. + // If the directory already existed, os.Stat returned nil, so we skip + // the chmod to avoid "operation not permitted" on unowned directories. + 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 364a92ae..241a7f1e 100644 --- a/context_test.go +++ b/context_test.go @@ -275,6 +275,40 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) } +// TestSaveUploadedFileToExistingDir is a regression test for issue #4622. +// SaveUploadedFile must not call os.Chmod on a directory that already exists, +// because the process may not own it (e.g. /tmp on Linux/macOS). +func TestSaveUploadedFileToExistingDir(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) + + // Use os.TempDir() which is guaranteed to already exist and may not be + // owned by the current process — this is exactly the scenario from #4622. + dst := filepath.Join(os.TempDir(), "gin_save_uploaded_regression_test.txt") + t.Cleanup(func() { + _ = os.Remove(dst) + }) + + // Must not fail with "chmod ...: operation not permitted" + require.NoError(t, c.SaveUploadedFile(f, dst)) + + // Verify the file was actually written with correct content + content, err := os.ReadFile(dst) + require.NoError(t, err) + assert.Equal(t, "existing_dir_test", string(content)) +} + func TestContextReset(t *testing.T) { router := New() c := router.allocateContext(0)