Compare commits

...

9 Commits

Author SHA1 Message Date
lark
6dad8564a1
Merge e26253477bab8892477222039cf96b21cd9059fd into d9307dbcbbe796a64d9e0ef23452da888dd7f904 2026-06-23 11:59:00 +00:00
Muhammad Ardy Junata
d9307dbcbb
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) <noreply@anthropic.com>

---------

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-22 23:34:41 +08:00
Pierre F.
da1e108614
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) <noreply@anthropic.com>

---------

Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-22 22:43:44 +08:00
MuaazTasawar
074b669a95
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 <appleboy.tw@gmail.com>
2026-06-22 21:18:24 +08:00
DadaVinqi
4a3eb31fb1
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) <noreply@anthropic.com>

---------

Co-authored-by: m0_66095053 <2876430886@qq.com>
Co-authored-by: Bo-Yi Wu <appleboy.tw@gmail.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-22 20:56:28 +08:00
Amirhf
293ad7edeb
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 <appleboy.tw@gmail.com>
2026-06-22 19:09:26 +08:00
Bo-Yi Wu
2e4d4f3896
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
2026-06-22 19:00:06 +08:00
willlv
e26253477b fix: resolve errcheck and gofmt lint errors
- Add '_ =' to ignore return values of c.Error() calls in tests
- Add '_ =' to recursive c.Error() call in Context.Error()
- Run gofmt to fix formatting issues
2026-03-23 16:19:12 +08:00
willlv
010dea5bd5 fix: unwrap errors.Join() created joined errors in Context.Error() (#4237)
Automatically unwrap errors created by errors.Join() when passed to
Context.Error(), so they are added as separate error entries instead
of a single combined error. This improves error log readability and
structure.

Before:
  Error #01: gin error
  Error #02: service error
  store error
  Error #03: other error

After:
  Error #01: gin error
  Error #02: service error
  Error #03: store error
  Error #04: other error

Closes #4237
2026-03-23 16:00:36 +08:00
8 changed files with 268 additions and 17 deletions

View File

@ -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
}
@ -254,6 +264,22 @@ func (c *Context) Error(err error) *Error {
panic("err is nil")
}
// Unwrap errors.Join() created joinErr
type unwrapper interface {
Unwrap() []error
}
if joinErr, ok := err.(unwrapper); ok {
errs := joinErr.Unwrap()
if len(errs) > 0 {
// Recursively add each error from the joined errors
for _, e := range errs {
_ = c.Error(e)
}
// Return the last added error
return c.Errors.Last()
}
}
var parsedError *Error
ok := errors.As(err, &parsedError)
if !ok {
@ -716,6 +742,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 {
@ -728,11 +759,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)

View File

@ -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,52 @@ 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))
}
// 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) {
@ -689,6 +732,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}

View File

@ -7,6 +7,7 @@ package gin
import (
"errors"
"fmt"
"net/http/httptest"
"testing"
"github.com/gin-gonic/gin/codec/json"
@ -138,3 +139,53 @@ func TestErrorUnwrap(t *testing.T) {
var testErrNonPointer TestErr
require.ErrorAs(t, wrappedErr, &testErrNonPointer)
}
// TestErrorJoinUnwrap tests that gin.Error() automatically unwraps errors.Join() created joined errors.
func TestErrorJoinUnwrap(t *testing.T) {
c, _ := CreateTestContext(httptest.NewRecorder())
// Test with errors.Join()
err1 := errors.New("service error")
err2 := errors.New("store error")
joinedErr := errors.Join(err1, err2)
_ = c.Error(joinedErr)
// Should be unwrapped into 2 separate errors
assert.Len(t, c.Errors, 2)
assert.Equal(t, "service error", c.Errors[0].Error())
assert.Equal(t, "store error", c.Errors[1].Error())
// Test mixed usage
c2, _ := CreateTestContext(httptest.NewRecorder())
_ = c2.Error(errors.New("gin error"))
_ = c2.Error(errors.Join(err1, err2))
_ = c2.Error(errors.New("other error"))
assert.Len(t, c2.Errors, 4)
expected := `Error #01: gin error
Error #02: service error
Error #03: store error
Error #04: other error
`
assert.Equal(t, expected, c2.Errors.String())
// Test empty join (edge case)
c3, _ := CreateTestContext(httptest.NewRecorder())
emptyJoin := errors.Join() // Creates nil error
if emptyJoin != nil {
_ = c3.Error(emptyJoin)
// errors.Join() with no arguments returns nil, so this shouldn't panic
}
// Test nested joins
c4, _ := CreateTestContext(httptest.NewRecorder())
err3 := errors.New("nested1")
err4 := errors.New("nested2")
nestedJoin := errors.Join(errors.Join(err3, err4), errors.New("outer"))
_ = c4.Error(nestedJoin)
assert.Len(t, c4.Errors, 3)
assert.Equal(t, "nested1", c4.Errors[0].Error())
assert.Equal(t, "nested2", c4.Errors[1].Error())
assert.Equal(t, "outer", c4.Errors[2].Error())
}

2
go.mod
View File

@ -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

6
go.sum
View File

@ -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=

View File

@ -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)
}

View File

@ -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)

View File

@ -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{}