From 592a2d08c2f2b6f93bec92bd6498633852473302 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:37:04 +0000 Subject: [PATCH 1/4] fix: skip chmod on pre-existing directories in SaveUploadedFile Only apply os.Chmod to directories newly created by os.MkdirAll. Previously, chmod was called unconditionally, which broke saving files into pre-existing directories not owned by the process (e.g., /tmp) with "operation not permitted" error. Refs #4622 Agent-Logs-Url: https://github.com/odlev/gin/sessions/2d0f57ad-a46e-45f6-a2f2-b6d4c352f22e Co-authored-by: odlev <65655276+odlev@users.noreply.github.com> --- context.go | 9 +++++++-- context_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5174033e..d541e50b 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 newly created directories to avoid "operation not permitted" + // errors on pre-existing directories we may not own (e.g., /tmp). + if statErr != nil { + 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..79d72275 100644 --- a/context_test.go +++ b/context_test.go @@ -274,6 +274,31 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) } +func TestSaveUploadedFileToExistingDir(t *testing.T) { + buf := new(bytes.Buffer) + mw := multipart.NewWriter(buf) + w, err := mw.CreateFormFile("file", "test") + require.NoError(t, err) + _, err = w.Write([]byte("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) + + // Save to a pre-existing directory. Previously this would fail with + // "chmod : operation not permitted" when the directory was not + // owned by the current process (e.g., /tmp). + dir := t.TempDir() + dst := filepath.Join(dir, "uploaded.txt") + require.NoError(t, c.SaveUploadedFile(f, dst)) + t.Cleanup(func() { + os.Remove(dst) + }) +} + func TestContextReset(t *testing.T) { router := New() c := router.allocateContext(0) From f86381e7ad2372fcbc564055e40cac5160d29f22 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:38:20 +0000 Subject: [PATCH 2/4] fix: use os.IsNotExist for precise check, remove redundant cleanup Agent-Logs-Url: https://github.com/odlev/gin/sessions/2d0f57ad-a46e-45f6-a2f2-b6d4c352f22e Co-authored-by: odlev <65655276+odlev@users.noreply.github.com> --- context.go | 2 +- context_test.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/context.go b/context.go index d541e50b..8014d4bc 100644 --- a/context.go +++ b/context.go @@ -734,7 +734,7 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm } // Only chmod newly created directories to avoid "operation not permitted" // errors on pre-existing directories we may not own (e.g., /tmp). - if statErr != nil { + if os.IsNotExist(statErr) { if err = os.Chmod(dir, mode); err != nil { return err } diff --git a/context_test.go b/context_test.go index 79d72275..2a3ce70f 100644 --- a/context_test.go +++ b/context_test.go @@ -294,9 +294,6 @@ func TestSaveUploadedFileToExistingDir(t *testing.T) { dir := t.TempDir() dst := filepath.Join(dir, "uploaded.txt") require.NoError(t, c.SaveUploadedFile(f, dst)) - t.Cleanup(func() { - os.Remove(dst) - }) } func TestContextReset(t *testing.T) { From 175d5e8e12c6c715f3dbf8474c2d8fbdf5fe631e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:51:50 +0000 Subject: [PATCH 3/4] fix: use safe type assertions in Hijack and CloseNotify to prevent panics Agent-Logs-Url: https://github.com/odlev/gin/sessions/a5300762-e9da-4790-8669-fd5200269ead Co-authored-by: odlev <65655276+odlev@users.noreply.github.com> --- response_writer.go | 18 +++++++++++--- response_writer_test.go | 52 +++++++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/response_writer.go b/response_writer.go index 9035e6f1..305322b6 100644 --- a/response_writer.go +++ b/response_writer.go @@ -17,7 +17,10 @@ const ( defaultStatus = http.StatusOK ) -var errHijackAlreadyWritten = errors.New("gin: response body already written") +var ( + errHijackAlreadyWritten = errors.New("gin: response body already written") + errHijackNotSupported = errors.New("gin: underlying ResponseWriter does not implement http.Hijacker") +) // ResponseWriter ... type ResponseWriter interface { @@ -117,12 +120,21 @@ func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { if w.size < 0 { w.size = 0 } - return w.ResponseWriter.(http.Hijacker).Hijack() + if hijacker, ok := w.ResponseWriter.(http.Hijacker); ok { + return hijacker.Hijack() + } + return nil, nil, errHijackNotSupported } // CloseNotify implements the http.CloseNotifier interface. +// +// Deprecated: the CloseNotifier interface predates Go's context package. +// New code should use Request.Context instead. func (w *responseWriter) CloseNotify() <-chan bool { - return w.ResponseWriter.(http.CloseNotifier).CloseNotify() + if cn, ok := w.ResponseWriter.(http.CloseNotifier); ok { + return cn.CloseNotify() + } + return nil } // Flush implements the http.Flusher interface. diff --git a/response_writer_test.go b/response_writer_test.go index dfc1d2c6..85c1bbbb 100644 --- a/response_writer_test.go +++ b/response_writer_test.go @@ -113,15 +113,12 @@ func TestResponseWriterHijack(t *testing.T) { writer.reset(testWriter) w := ResponseWriter(writer) - assert.Panics(t, func() { - _, _, err := w.Hijack() - require.NoError(t, err) - }) + _, _, err := w.Hijack() + require.ErrorIs(t, err, errHijackNotSupported) assert.True(t, w.Written()) - assert.Panics(t, func() { - w.CloseNotify() - }) + ch := w.CloseNotify() + assert.Nil(t, ch) w.Flush() } @@ -315,3 +312,44 @@ func TestPusherWithoutPusher(t *testing.T) { pusher := w.Pusher() assert.Nil(t, pusher, "Expected pusher to be nil") } + +// mockCloseNotifier is an http.ResponseWriter that implements http.CloseNotifier. +type mockCloseNotifier struct { + *httptest.ResponseRecorder +} + +func (m *mockCloseNotifier) CloseNotify() <-chan bool { + ch := make(chan bool, 1) + return ch +} + +func TestCloseNotifyWithCloseNotifier(t *testing.T) { + rw := &mockCloseNotifier{ResponseRecorder: httptest.NewRecorder()} + w := &responseWriter{} + w.reset(rw) + + ch := w.CloseNotify() + assert.NotNil(t, ch, "Expected CloseNotify channel to be non-nil") +} + +func TestCloseNotifyWithoutCloseNotifier(t *testing.T) { + // httptest.NewRecorder does not implement http.CloseNotifier + rw := httptest.NewRecorder() + w := &responseWriter{} + w.reset(rw) + + ch := w.CloseNotify() + assert.Nil(t, ch, "Expected CloseNotify channel to be nil when underlying writer does not support it") +} + +func TestHijackWithoutHijacker(t *testing.T) { + // httptest.NewRecorder does not implement http.Hijacker + rw := httptest.NewRecorder() + w := &responseWriter{} + w.reset(rw) + + conn, buf, err := w.Hijack() + assert.Nil(t, conn) + assert.Nil(t, buf) + require.ErrorIs(t, err, errHijackNotSupported) +} From 62f18efb5c80f27a15e83684d317f64c13b9a97e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:53:35 +0000 Subject: [PATCH 4/4] test: use unbuffered channel in mockCloseNotifier Agent-Logs-Url: https://github.com/odlev/gin/sessions/a5300762-e9da-4790-8669-fd5200269ead Co-authored-by: odlev <65655276+odlev@users.noreply.github.com> --- response_writer_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/response_writer_test.go b/response_writer_test.go index 85c1bbbb..02dbe4dd 100644 --- a/response_writer_test.go +++ b/response_writer_test.go @@ -319,8 +319,7 @@ type mockCloseNotifier struct { } func (m *mockCloseNotifier) CloseNotify() <-chan bool { - ch := make(chan bool, 1) - return ch + return make(chan bool) } func TestCloseNotifyWithCloseNotifier(t *testing.T) {