From 2e4d4f38962a6f15ae496d59b294f307eef95429 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 22 Jun 2026 19:00:06 +0800 Subject: [PATCH 1/6] chore(deps): bump github.com/quic-go/quic-go to v0.60.0 (#4713) - Upgrade quic-go from v0.59.0 to v0.60.0 --- go.mod | 2 +- go.sum | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index df181253..705034ee 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/mattn/go-isatty v0.0.20 github.com/modern-go/reflect2 v1.0.2 github.com/pelletier/go-toml/v2 v2.2.4 - github.com/quic-go/quic-go v0.59.0 + github.com/quic-go/quic-go v0.60.0 github.com/stretchr/testify v1.11.1 github.com/ugorji/go/codec v1.3.1 go.mongodb.org/mongo-driver/v2 v2.5.0 diff --git a/go.sum b/go.sum index f7f9e27b..c38e6eae 100644 --- a/go.sum +++ b/go.sum @@ -50,10 +50,12 @@ github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0 github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/quic-go/go-ossfuzz-seeds v0.1.0 h1:APacT+iIaNF6fd8AGEiN3bT/Jtkd2jz4v4TzM7MFjy0= +github.com/quic-go/go-ossfuzz-seeds v0.1.0/go.mod h1:3IOHRbJIc+L6YKMwfDtJAM9Vj9k0YY4muhuyUYk5tbk= github.com/quic-go/qpack v0.6.0 h1:g7W+BMYynC1LbYLSqRt8PBg5Tgwxn214ZZR34VIOjz8= github.com/quic-go/qpack v0.6.0/go.mod h1:lUpLKChi8njB4ty2bFLX2x4gzDqXwUpaO1DP9qMDZII= -github.com/quic-go/quic-go v0.59.0 h1:OLJkp1Mlm/aS7dpKgTc6cnpynnD2Xg7C1pwL6vy/SAw= -github.com/quic-go/quic-go v0.59.0/go.mod h1:upnsH4Ju1YkqpLXC305eW3yDZ4NfnNbmQRCMWS58IKU= +github.com/quic-go/quic-go v0.60.0 h1:xcQioE8OM66UQLeUMHltK1CCcOu3JbVB4JAQdDQSB+0= +github.com/quic-go/quic-go v0.60.0/go.mod h1:wpKpjmPpftl30sL6pFh7REVpjbcCVy4zt2vDyK1TuJk= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/rogpeppe/go-internal v1.10.0/go.mod h1:UQnix2H7Ngw/k4C5ijL5+65zddjncjaFoBhdsK/akog= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= From 293ad7edebb3ae30369288bd6416ca0d78474727 Mon Sep 17 00:00:00 2001 From: Amirhf Date: Mon, 22 Jun 2026 14:39:26 +0330 Subject: [PATCH 2/6] fix(context): Copy() copies Errors and Accepted fields (#4695) * Previously, Copy() only copied Keys and Params, leaving Errors and Accepted as nil even when set on the original context. Goroutines receiving a copied context could not observe errors attached before the copy, and content-negotiation state set by middleware was silently lost. * Replace assert.Equal(t, len(c.Errors), len(cp.Errors)) with assert.Len(t, cp.Errors, 2) to satisfy the testifylint linter rule --------- Co-authored-by: Bo-Yi Wu --- context.go | 10 ++++++++++ context_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/context.go b/context.go index a2e28e5b..640e20b2 100644 --- a/context.go +++ b/context.go @@ -141,6 +141,16 @@ func (c *Context) Copy() *Context { cp.Params = make([]Param, len(cParams)) copy(cp.Params, cParams) + if c.Errors != nil { + cp.Errors = make(errorMsgs, len(c.Errors)) + copy(cp.Errors, c.Errors) + } + + if c.Accepted != nil { + cp.Accepted = make([]string, len(c.Accepted)) + copy(cp.Accepted, c.Accepted) + } + return &cp } diff --git a/context_test.go b/context_test.go index 364a92ae..2dfdd392 100644 --- a/context_test.go +++ b/context_test.go @@ -689,6 +689,50 @@ func TestContextCopy(t *testing.T) { assert.Equal(t, cp.fullPath, c.fullPath) } +func TestContextCopyCopiesErrors(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + _ = c.Error(errors.New("first error")) + _ = c.Error(errors.New("second error")) + + cp := c.Copy() + + // copied context has the same errors + assert.Len(t, cp.Errors, 2) + assert.Equal(t, c.Errors[0].Error(), cp.Errors[0].Error()) + assert.Equal(t, c.Errors[1].Error(), cp.Errors[1].Error()) + + // mutations on the copy do not affect the original + _ = cp.Error(errors.New("third error")) + assert.Len(t, c.Errors, 2) + assert.Len(t, cp.Errors, 3) +} + +func TestContextCopyCopiesAccepted(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.SetAccepted("application/json", "text/html") + + cp := c.Copy() + + assert.Equal(t, c.Accepted, cp.Accepted) + + // mutations on the copy do not affect the original + cp.SetAccepted("text/plain") + assert.Equal(t, []string{"application/json", "text/html"}, c.Accepted) + assert.Equal(t, []string{"text/plain"}, cp.Accepted) +} + +func TestContextCopyNilErrorsAndAccepted(t *testing.T) { + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + + cp := c.Copy() + + assert.Nil(t, cp.Errors) + assert.Nil(t, cp.Accepted) +} + func TestContextHandlerName(t *testing.T) { c, _ := CreateTestContext(httptest.NewRecorder()) c.handlers = HandlersChain{func(c *Context) {}, handlerNameTest} From 4a3eb31fb15b2a2d78b4bdbe0c31a2c564b1977a Mon Sep 17 00:00:00 2001 From: DadaVinqi Date: Mon, 22 Jun 2026 20:56:28 +0800 Subject: [PATCH 3/6] fix(recovery): record recovered panics in c.Errors (#4698) * fix: record recovered panic errors * chore(deps): bump quic-go to v0.59.1 * refactor(recovery): simplify panic error recording - Collapse the if/else into a single c.Error call in defaultHandleRecovery - Assert the recorded panic error is ErrorTypePrivate in the test Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: m0_66095053 <2876430886@qq.com> Co-authored-by: Bo-Yi Wu Co-authored-by: Claude Opus 4.8 (1M context) --- recovery.go | 7 ++++++- recovery_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/recovery.go b/recovery.go index bbf1d565..2124fe5f 100644 --- a/recovery.go +++ b/recovery.go @@ -106,7 +106,12 @@ func secureRequestDump(r *http.Request) string { return strings.Join(lines, "\r\n") } -func defaultHandleRecovery(c *Context, _ any) { +func defaultHandleRecovery(c *Context, err any) { + e, ok := err.(error) + if !ok { + e = fmt.Errorf("%v", err) + } + c.Error(e) //nolint: errcheck c.AbortWithStatus(http.StatusInternalServerError) } diff --git a/recovery_test.go b/recovery_test.go index 028c4ad6..e5211790 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -5,6 +5,7 @@ package gin import ( + "errors" "net" "net/http" "os" @@ -152,6 +153,49 @@ func TestPanicWithAbortHandler(t *testing.T) { assert.NotContains(t, out, "panic recovered") } +func TestPanicInHandlerRecordsError(t *testing.T) { + tests := []struct { + name string + recoveredErr any + expectedErr string + }{ + { + name: "string panic", + recoveredErr: "Oops, Houston, we have a problem", + expectedErr: "Oops, Houston, we have a problem", + }, + { + name: "error panic", + recoveredErr: errors.New("recovered error"), + expectedErr: "recovered error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router := New() + + var recoveredErrors errorMsgs + router.Use(func(c *Context) { + c.Next() + recoveredErrors = c.Errors + }) + router.Use(RecoveryWithWriter(nil)) + router.GET("/recovery", func(_ *Context) { + panic(tt.recoveredErr) + }) + + w := PerformRequest(router, http.MethodGet, "/recovery") + + assert.Equal(t, http.StatusInternalServerError, w.Code) + if assert.Len(t, recoveredErrors, 1) { + assert.EqualError(t, recoveredErrors[0], tt.expectedErr) + assert.Equal(t, ErrorTypePrivate, recoveredErrors[0].Type) + } + }) + } +} + func TestCustomRecoveryWithWriter(t *testing.T) { errBuffer := new(strings.Builder) buffer := new(strings.Builder) From 074b669a95fd834701359acc55371e6b377618f6 Mon Sep 17 00:00:00 2001 From: MuaazTasawar Date: Mon, 22 Jun 2026 18:18:24 +0500 Subject: [PATCH 4/6] test(response_writer): add tests for Flush() with and without http.Flusher (#4699) * test(response_writer): add tests for Flush() with and without http.Flusher * test(response_writer): drop stray comment and clarify Flush regression note - Remove orphaned doc comment left at the end of the file - Reword the issue #4460 reference to reflect the no-panic guard --------- Co-authored-by: Bo-Yi Wu --- response_writer_test.go | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/response_writer_test.go b/response_writer_test.go index 83d3fc8b..03417375 100644 --- a/response_writer_test.go +++ b/response_writer_test.go @@ -15,10 +15,33 @@ import ( "github.com/stretchr/testify/require" ) -// TODO -// func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { -// func (w *responseWriter) CloseNotify() <-chan bool { -// func (w *responseWriter) Flush() { +// TestResponseWriterFlushWithFlusher verifies Flush() calls the underlying Flusher. +func TestResponseWriterFlushWithFlusher(t *testing.T) { + testWriter := httptest.NewRecorder() + writer := &responseWriter{ResponseWriter: testWriter} + writer.Flush() + assert.True(t, testWriter.Flushed) +} + +// TestResponseWriterFlushWithNonFlusher verifies Flush() is a no-op +// when the underlying ResponseWriter does not implement http.Flusher. +// Guards against the panic reported in https://github.com/gin-gonic/gin/issues/4460 +func TestResponseWriterFlushWithNonFlusher(t *testing.T) { + nonFlusher := &nonFlusherWriter{header: http.Header{}} + writer := &responseWriter{ResponseWriter: nonFlusher} + require.NotPanics(t, func() { + writer.Flush() + }) +} + +// nonFlusherWriter is a minimal http.ResponseWriter that does NOT implement http.Flusher. +type nonFlusherWriter struct { + header http.Header +} + +func (w *nonFlusherWriter) Header() http.Header { return w.header } +func (w *nonFlusherWriter) Write(b []byte) (int, error) { return len(b), nil } +func (w *nonFlusherWriter) WriteHeader(code int) {} var ( _ ResponseWriter = &responseWriter{} From da1e108614ecbbadfa5736b1b297b16121d23b9b Mon Sep 17 00:00:00 2001 From: "Pierre F." Date: Mon, 22 Jun 2026 16:43:44 +0200 Subject: [PATCH 5/6] test(context): use t.TempDir() for SaveUploadedFile permission test on WSL (#4709) * fix: change file creation to use c.Temp instead to work on wsl * test(context): use t.TempDir() in SaveUploadedFile failure test for WSL Mirror the WSL-portability fix applied to TestSaveUploadedFileWithPermission: write under t.TempDir() instead of a relative path so the test does not depend on the working directory's filesystem (drvfs reports 0o777 on WSL) and is cleaned up automatically. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Bo-Yi Wu Co-authored-by: Claude Opus 4.8 (1M context) --- context_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/context_test.go b/context_test.go index 2dfdd392..d93e470f 100644 --- a/context_test.go +++ b/context_test.go @@ -248,13 +248,11 @@ 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)) - t.Cleanup(func() { - assert.NoError(t, os.Remove("permission_test")) - }) - info, err := os.Stat(filepath.Dir("permission_test")) + dst := filepath.Join(t.TempDir(), "subdir", "permission_test") + require.NoError(t, c.SaveUploadedFile(f, dst, mode)) + info, err := os.Stat(filepath.Dir(dst)) require.NoError(t, err) - assert.Equal(t, info.Mode().Perm(), mode) + assert.Equal(t, mode, info.Mode().Perm()) } func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { @@ -272,7 +270,8 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.NoError(t, err) assert.Equal(t, "permission_test", f.Filename) var mode fs.FileMode = 0o644 - require.Error(t, c.SaveUploadedFile(f, "test/permission_test", mode)) + dst := filepath.Join(t.TempDir(), "test", "permission_test") + require.Error(t, c.SaveUploadedFile(f, dst, mode)) } func TestContextReset(t *testing.T) { From d9307dbcbbe796a64d9e0ef23452da888dd7f904 Mon Sep 17 00:00:00 2001 From: Muhammad Ardy Junata Date: Mon, 22 Jun 2026 22:34:41 +0700 Subject: [PATCH 6/6] fix(context): skip chmod on pre-existing dirs in SaveUploadedFile (#4702) * 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. * test: make SaveUploadedFile #4622 regression test platform-independent The regression test relied on os.Chmod failing on a pre-existing directory, which only happens for a non-owner, non-root process. When tests run as root (common in containers/CI) or with a user-owned $TMPDIR, the buggy chmod succeeds, so the test passed even against the unfixed code. Assert the actual contract instead: a pre-existing directory's permissions are left unchanged. This catches the regression deterministically on every platform. Also tighten the SaveUploadedFile doc/comments: the requested perm is enforced only on the newly created destination directory, not on every directory in the path. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Bo-Yi Wu Co-authored-by: Claude Opus 4.8 (1M context) --- context.go | 17 +++++++++++++++-- context_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 640e20b2..893edb28 100644 --- a/context.go +++ b/context.go @@ -726,6 +726,11 @@ func (c *Context) MultipartForm() (*multipart.Form, error) { } // SaveUploadedFile uploads the form file to specific dst. +// An optional perm argument specifies the permission bits used when creating +// the destination directory. If not provided, the default is 0750. The exact +// permission is enforced only on the destination directory and only when it is +// newly 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 { @@ -738,11 +743,19 @@ func (c *Context) SaveUploadedFile(file *multipart.FileHeader, dst string, perm mode = perm[0] } dir := filepath.Dir(dst) + // Record whether the destination directory exists before MkdirAll, so we + // only chmod a directory we just created. Chmod'ing a pre-existing directory + // the process does not own (e.g. /tmp) fails with "operation not permitted" + // (#4622). A non-ErrNotExist stat error also skips chmod and lets MkdirAll + // surface the underlying failure. + _, statErr := os.Stat(dir) if err = os.MkdirAll(dir, mode); err != nil { return err } - if err = os.Chmod(dir, mode); err != nil { - return err + 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 d93e470f..e8d305e4 100644 --- a/context_test.go +++ b/context_test.go @@ -274,6 +274,50 @@ func TestSaveUploadedFileWithPermissionFailed(t *testing.T) { require.Error(t, c.SaveUploadedFile(f, dst, 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), where chmod +// fails with "operation not permitted". This asserts the behavioral contract +// directly — a pre-existing directory's permissions are left unchanged — so it +// catches the regression on every platform, including environments (root/CI, +// user-owned $TMPDIR) where chmod on the temp dir would otherwise succeed. +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) + + // A pre-existing directory owned by this process, set to a known mode. + dir := t.TempDir() + require.NoError(t, os.Chmod(dir, 0o700)) + + // Pass a perm that differs from the directory's current mode. The fix must + // not apply it to the pre-existing directory; the old code chmod'd it + // unconditionally, which also failed outright on unowned dirs like /tmp. + dst := filepath.Join(dir, "existing_dir_test.txt") + require.NoError(t, c.SaveUploadedFile(f, dst, 0o755)) + + // The pre-existing directory's permissions must be unchanged. + info, err := os.Stat(dir) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o700), info.Mode().Perm(), + "permissions of a pre-existing directory must not be modified") + + // The file must still be written with the 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)