mirror of
https://github.com/gin-gonic/gin.git
synced 2026-06-27 03:58:18 +08:00
fix: skip chmod on pre-existing dirs in SaveUploadedFile
Fixes #4622 Prior to this change, SaveUploadedFile unconditionally called os.Chmod(dir, mode) after os.MkdirAll, even when the target directory already existed. This caused 'operation not permitted' errors when saving files into system-owned directories like /tmp that the current process does not own. The fix uses os.Stat before MkdirAll to detect whether the directory already exists, and only calls os.Chmod when the directory was freshly created by MkdirAll. This restores the behaviour from v1.10.1 where only os.MkdirAll was used (which correctly skips permission changes on existing dirs) while preserving the custom permission feature added in v1.12.0. Regression test added: TestSaveUploadedFileToExistingDir saves a file into os.TempDir() (a pre-existing system directory) and asserts no error is returned.
This commit is contained in:
parent
d75fcd4c9a
commit
2df018666f
17
context.go
17
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)
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user