Merge 2ac84ddd382d4459632fe59f0919551e78e6170e into 5f4f9643258dc2a65e684b63f12c8d543c936c67

SaveUploadedFile called os.Chmod on the target directory even if it
already existed. This breaks when saving to system directories like
/tmp where the process lacks permission to chmod.

Now only newly created directories get chmod'd. Existing directories
are left as-is, which is the correct behavior since the caller should
not modify permissions of directories they don't own.

Fixes #4622

context.go

@@ -715,6 +715,24 @@ func (c *Context) MultipartForm() (*multipart.Form, error) {
 	return c.Request.MultipartForm, err
 }
 
+func dirExists(path string) bool {
+	info, err := os.Stat(path)
+	if err != nil {
+		return false
+	}
+	return info.IsDir()
+}
+
+func ensureDirWithMode(dir string, mode fs.FileMode) error {
+	if dirExists(dir) {
+		return nil
+	}
+	if err := os.MkdirAll(dir, mode); err != nil {
+		return err
+	}
+	return os.Chmod(dir, mode)
+}
+
 // SaveUploadedFile uploads the form file to specific dst.
 func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm ...fs.FileMode) error {
 	src, err := file.Open()
@@ -728,10 +746,9 @@ 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 {
+	// Only chmod newly created directories. Attempting to chmod
-		return err
+	// pre-existing directories (e.g. /tmp) may fail with EPERM.
-	}
+	if err = ensureDirWithMode(dir, mode); err != nil {
-	if err = os.Chmod(dir, mode); err != nil {
 		return err
 	}
 

context_test.go

@@ -20,6 +20,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -247,13 +248,60 @@ func TestSaveUploadedFileWithPermission(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, "permission_test", f.Filename)
 	var mode fs.FileMode = 0o755
-	require.NoError(t, c.SaveUploadedFile(f, "permission_test", mode))
+	tmpDir := t.TempDir()
-	t.Cleanup(func() {
+	newSubDir := filepath.Join(tmpDir, "newdir")
-		assert.NoError(t, os.Remove("permission_test"))
+	dst := filepath.Join(newSubDir, "permission_test")
-	})
+	require.NoError(t, c.SaveUploadedFile(f, dst, mode))
-	info, err := os.Stat(filepath.Dir("permission_test"))
+	if runtime.GOOS != "windows" {
+		info, err := os.Stat(newSubDir)
+		require.NoError(t, err)
+		assert.Equal(t, mode, info.Mode().Perm())
+	}
+}
+
+func TestDirExists(t *testing.T) {
+	tmpDir := t.TempDir()
+	assert.True(t, dirExists(tmpDir))
+	assert.False(t, dirExists(filepath.Join(tmpDir, "missing")))
+
+	filePath := filepath.Join(tmpDir, "file")
+	require.NoError(t, os.WriteFile(filePath, []byte("test"), 0o600))
+	assert.False(t, dirExists(filePath))
+}
+
+func TestSaveUploadedFileKeepsExistingDirPermission(t *testing.T) {
+	buf := new(bytes.Buffer)
+	mw := multipart.NewWriter(buf)
+	w, err := mw.CreateFormFile("file", "permission_test")
 	require.NoError(t, err)
-	assert.Equal(t, info.Mode().Perm(), mode)
+	_, 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)
+
+	tmpDir := t.TempDir()
+	existingDir := filepath.Join(tmpDir, "existing")
+	require.NoError(t, os.Mkdir(existingDir, 0o700))
+	if runtime.GOOS != "windows" {
+		require.NoError(t, os.Chmod(existingDir, 0o700))
+	}
+
+	var mode fs.FileMode = 0o755
+	dst := filepath.Join(existingDir, "permission_test")
+	require.NoError(t, c.SaveUploadedFile(f, dst, mode))
+
+	saved, err := os.ReadFile(dst)
+	require.NoError(t, err)
+	assert.Equal(t, "permission_test", string(saved))
+	if runtime.GOOS != "windows" {
+		info, err := os.Stat(existingDir)
+		require.NoError(t, err)
+		assert.Equal(t, fs.FileMode(0o700), info.Mode().Perm())
+	}
 }
 
 func TestSaveUploadedFileWithPermissionFailed(t *testing.T) {