diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index f287c265..0ede2a05 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -33,7 +33,7 @@ jobs: steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 # Initializes the CodeQL tools for scanning. - name: Initialize CodeQL diff --git a/.github/workflows/gin.yml b/.github/workflows/gin.yml index 8857bebc..072312dd 100644 --- a/.github/workflows/gin.yml +++ b/.github/workflows/gin.yml @@ -16,11 +16,11 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v6 + uses: actions/setup-go@v7 with: go-version: "^1" - name: Setup golangci-lint @@ -55,17 +55,17 @@ jobs: GOPROXY: https://proxy.golang.org steps: - name: Set up Go ${{ matrix.go }} - uses: actions/setup-go@v6 + uses: actions/setup-go@v7 with: go-version: ${{ matrix.go }} cache: false - name: Checkout Code - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: ref: ${{ github.ref }} - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | ${{ matrix.go-build }} @@ -78,6 +78,6 @@ jobs: run: make test - name: Upload coverage to Codecov - uses: codecov/codecov-action@v6 + uses: codecov/codecov-action@v7 with: flags: ${{ matrix.os }},go-${{ matrix.go }},${{ matrix.test-tags }} diff --git a/.github/workflows/goreleaser.yml b/.github/workflows/goreleaser.yml index ea933e7e..6dd36ff1 100644 --- a/.github/workflows/goreleaser.yml +++ b/.github/workflows/goreleaser.yml @@ -13,11 +13,11 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 0 - name: Set up Go - uses: actions/setup-go@v6 + uses: actions/setup-go@v7 with: go-version: "^1" - name: Run GoReleaser diff --git a/.github/workflows/trivy-scan.yml b/.github/workflows/trivy-scan.yml index 9060e45c..751d0106 100644 --- a/.github/workflows/trivy-scan.yml +++ b/.github/workflows/trivy-scan.yml @@ -22,7 +22,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: fetch-depth: 0 diff --git a/binding/default_validator.go b/binding/default_validator.go index 8203bcaa..f27d437b 100644 --- a/binding/default_validator.go +++ b/binding/default_validator.go @@ -32,7 +32,10 @@ func (err SliceValidationError) Error() string { if b.Len() > 0 { b.WriteString("\n") } - b.WriteString("[" + strconv.Itoa(i) + "]: " + err[i].Error()) + b.WriteString("[") + b.WriteString(strconv.Itoa(i)) + b.WriteString("]: ") + b.WriteString(err[i].Error()) } } return b.String() diff --git a/context.go b/context.go index 5174033e..1dc730e3 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 } @@ -619,8 +629,8 @@ func (c *Context) DefaultPostForm(key, defaultValue string) string { // For example, during a PATCH request to update the user's email: // // email=mail@example.com --> ("mail@example.com", true) := GetPostForm("email") // set email to "mail@example.com" -// email= --> ("", true) := GetPostForm("email") // set email to "" -// --> ("", false) := GetPostForm("email") // do nothing with email +// email= --> ("", true) := GetPostForm("email") // set email to "" +// --> ("", false) := GetPostForm("email") // do nothing with email func (c *Context) GetPostForm(key string) (string, bool) { if values, ok := c.GetPostFormArray(key); ok { return values[0], ok @@ -716,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 { @@ -728,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) @@ -764,7 +787,7 @@ func (c *Context) BindJSON(obj any) error { return c.MustBindWith(obj, binding.JSON) } -// BindXML is a shortcut for c.MustBindWith(obj, binding.BindXML). +// BindXML is a shortcut for c.MustBindWith(obj, binding.XML). func (c *Context) BindXML(obj any) error { return c.MustBindWith(obj, binding.XML) } @@ -1047,6 +1070,33 @@ func (c *Context) IsWebsocket() bool { return false } +// Scheme returns the HTTP scheme of the request ("http" or "https"). +// When running behind reverse proxies or load balancers `Request.URL.Scheme` is usually empty. +// the original scheme is commonly forwarded via headers such as X-Forwarded-Proto. +// Reference: +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-Proto +func (c *Context) Scheme() string { + if c.Request.TLS != nil { + return "https" + } + if scheme := c.requestHeader("X-Forwarded-Proto"); scheme != "" { + return scheme + } + if scheme := c.requestHeader("X-Forwarded-Protocol"); scheme != "" { + return scheme + } + if ssl := c.requestHeader("X-Forwarded-Ssl"); ssl == "on" { + return "https" + } + if scheme := c.requestHeader("X-Url-Scheme"); scheme != "" { + return scheme + } + if scheme := c.Request.URL.Scheme; scheme != "" { + return scheme + } + return "http" +} + func (c *Context) requestHeader(key string) string { return c.Request.Header.Get(key) } diff --git a/context_test.go b/context_test.go index ef60379d..e8d305e4 100644 --- a/context_test.go +++ b/context_test.go @@ -7,6 +7,7 @@ package gin import ( "bytes" "context" + "crypto/tls" "errors" "fmt" "html/template" @@ -247,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) { @@ -271,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) { @@ -688,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} @@ -2955,6 +3043,65 @@ func TestWebsocketsRequired(t *testing.T) { assert.False(t, c.IsWebsocket()) } +func TestContextScheme(t *testing.T) { + // TLS connection takes highest priority. + c, _ := CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.TLS = &tls.ConnectionState{} + assert.Equal(t, "https", c.Scheme()) + + // X-Forwarded-Proto header. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Forwarded-Proto", "https") + assert.Equal(t, "https", c.Scheme()) + + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Forwarded-Proto", "http") + assert.Equal(t, "http", c.Scheme()) + + // X-Forwarded-Protocol header. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Forwarded-Protocol", "https") + assert.Equal(t, "https", c.Scheme()) + + // X-Forwarded-Ssl: on header. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Forwarded-Ssl", "on") + assert.Equal(t, "https", c.Scheme()) + + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Forwarded-Ssl", "off") + assert.Equal(t, "http", c.Scheme()) + + // X-Url-Scheme header. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.Header.Set("X-Url-Scheme", "https") + assert.Equal(t, "https", c.Scheme()) + + // Request.URL.Scheme fallback. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "https://example.com/", nil) + assert.Equal(t, "https", c.Scheme()) + + // Default fallback: plain http. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + assert.Equal(t, "http", c.Scheme()) + + // TLS takes priority over X-Forwarded-Proto. + c, _ = CreateTestContext(httptest.NewRecorder()) + c.Request, _ = http.NewRequest(http.MethodGet, "/", nil) + c.Request.TLS = &tls.ConnectionState{} + c.Request.Header.Set("X-Forwarded-Proto", "http") + assert.Equal(t, "https", c.Scheme()) +} + func TestGetRequestHeaderValue(t *testing.T) { c, _ := CreateTestContext(httptest.NewRecorder()) c.Request, _ = http.NewRequest(http.MethodGet, "/chat", nil) diff --git a/go.mod b/go.mod index c5481db5..3f80e1c0 100644 --- a/go.mod +++ b/go.mod @@ -5,18 +5,18 @@ go 1.25.0 require ( github.com/bytedance/sonic v1.15.0 github.com/gin-contrib/sse v1.1.0 - github.com/go-playground/validator/v10 v10.30.1 + github.com/go-playground/validator/v10 v10.30.3 github.com/goccy/go-json v0.10.6 github.com/goccy/go-yaml v1.19.2 github.com/json-iterator/go v1.1.12 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 - golang.org/x/net v0.52.0 + golang.org/x/net v0.56.0 google.golang.org/protobuf v1.36.11 ) @@ -39,7 +39,7 @@ require ( github.com/twitchyliquid64/golang-asm v0.15.1 // indirect go.uber.org/mock v0.6.0 // indirect golang.org/x/arch v0.25.0 // indirect - golang.org/x/crypto v0.49.0 // indirect - golang.org/x/sys v0.42.0 // indirect - golang.org/x/text v0.35.0 // indirect + golang.org/x/crypto v0.53.0 // indirect + golang.org/x/sys v0.46.0 // indirect + golang.org/x/text v0.39.0 // indirect ) diff --git a/go.sum b/go.sum index a3b5b8a3..f04ab19c 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,8 @@ github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/o github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.30.1 h1:f3zDSN/zOma+w6+1Wswgd9fLkdwy06ntQJp0BBvFG0w= -github.com/go-playground/validator/v10 v10.30.1/go.mod h1:oSuBIQzuJxL//3MelwSLD5hc2Tu889bF0Idm9Dg26cM= +github.com/go-playground/validator/v10 v10.30.3 h1:4MU6YkEwx7GbcPJOZxrtbu+QfF3pJLJuaYTeAH0DYy8= +github.com/go-playground/validator/v10 v10.30.3/go.mod h1:4Axh7oCNGcoGkqLoE4YWt6n20mcEIsPRlB7vPk3lpyc= github.com/goccy/go-json v0.10.6 h1:p8HrPJzOakx/mn/bQtjgNjdTcN+/S6FcG2CTtQOrHVU= github.com/goccy/go-json v0.10.6/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM= @@ -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= @@ -77,15 +79,15 @@ go.uber.org/mock v0.6.0 h1:hyF9dfmbgIX5EfOdasqLsWD6xqpNZlXblLB/Dbnwv3Y= go.uber.org/mock v0.6.0/go.mod h1:KiVJ4BqZJaMj4svdfmHM0AUx4NJYO8ZNpPnZn1Z+BBU= golang.org/x/arch v0.25.0 h1:qnk6Ksugpi5Bz32947rkUgDt9/s5qvqDPl/gBKdMJLE= golang.org/x/arch v0.25.0/go.mod h1:0X+GdSIP+kL5wPmpK7sdkEVTt2XoYP0cSjQSbZBwOi8= -golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= -golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= -golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= -golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/crypto v0.53.0 h1:QZ4Muo8THX6CizN2vPPd5fBGHyogrdK9fG4wLPFUsto= +golang.org/x/crypto v0.53.0/go.mod h1:DNLU434OwVakk9PzuwV8w62mAJpRJL3vsgcfp4Qnsio= +golang.org/x/net v0.56.0 h1:Rw8j/hFzGvJUZwNBXnAtf5sVDVt+65SK2C7IxCxZt5o= +golang.org/x/net v0.56.0/go.mod h1:D3Ku6r+V6JROoZK144D2XfMHFcMq/0zSfLelVTCFKec= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= -golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw= +golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/path.go b/path.go index 3b67caa9..672213d1 100644 --- a/path.go +++ b/path.go @@ -55,7 +55,7 @@ func cleanPath(p string) string { // A bit more clunky without a 'lazybuf' like the path package, but the loop // gets completely inlined (bufApp calls). - // loop has no expensive function calls (except 1x make) // So in contrast to the path package this loop has no expensive function + // So in contrast to the path package this loop has no expensive function // calls (except make, if needed). for r < n { 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) diff --git a/response_writer.go b/response_writer.go index 9035e6f1..8e2d8b30 100644 --- a/response_writer.go +++ b/response_writer.go @@ -114,15 +114,22 @@ func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { if w.size > 0 { return nil, nil, errHijackAlreadyWritten } + hijacker, ok := w.ResponseWriter.(http.Hijacker) + if !ok { + return nil, nil, http.ErrNotSupported + } if w.size < 0 { w.size = 0 } - return w.ResponseWriter.(http.Hijacker).Hijack() + return hijacker.Hijack() } // CloseNotify implements the http.CloseNotifier interface. 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..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{} @@ -113,15 +136,18 @@ func TestResponseWriterHijack(t *testing.T) { writer.reset(testWriter) w := ResponseWriter(writer) - assert.Panics(t, func() { - _, _, err := w.Hijack() - require.NoError(t, err) - }) - assert.True(t, w.Written()) + // httptest.ResponseRecorder doesn't implement http.Hijacker; return + // http.ErrNotSupported instead of panicking (#4638). On unsupported the + // writer state stays untouched so the handler can still emit a normal + // HTTP response as a fallback. + conn, buf, err := w.Hijack() + assert.Nil(t, conn) + assert.Nil(t, buf) + require.ErrorIs(t, err, http.ErrNotSupported) + assert.False(t, w.Written()) - assert.Panics(t, func() { - w.CloseNotify() - }) + // CloseNotify on a non-CloseNotifier returns nil instead of panicking. + assert.Nil(t, w.CloseNotify()) w.Flush() }