Compare commits

...

4 Commits

Author SHA1 Message Date
Name
4f1dd60fd4
Merge a14cf7c08dc37b7f9308248928aa3b16307f3c2f into 3ab698dc5110af1977d57226e4995c57dd34c233 2026-01-17 19:17:50 +08:00
OHZEKI Naoki
3ab698dc51
refactor(recovery): smart error comparison (#4142)
* refactor(recovery): rename var in CustomRecoveryWithWriter

* refactor(recovery): smart error comparison

* test(recovery): Directly reference the syscall error string
2026-01-17 16:40:43 +08:00
1911860538
a14cf7c08d refactor(render): use WriteJSON when JsonpJson.Callback is empty 2025-11-08 22:06:23 +08:00
1911860538
b7afe5a6af fix(render): improve JsonpJSON content type handling and simplify Context.JSONP 2025-11-08 13:35:02 +08:00
5 changed files with 47 additions and 47 deletions

View File

@ -1166,13 +1166,10 @@ func (c *Context) SecureJSON(code int, obj any) {
// JSONP serializes the given struct as JSON into the response body.
// It adds padding to response body to request data from a server residing in a different domain than the client.
// It also sets the Content-Type as "application/javascript".
//
// When the callback parameter is empty, it behaves equivalently to Context.JSON.
func (c *Context) JSONP(code int, obj any) {
callback := c.DefaultQuery("callback", "")
if callback == "" {
c.Render(code, render.JSON{Data: obj})
return
}
c.Render(code, render.JsonpJSON{Callback: callback, Data: obj})
c.Render(code, render.JsonpJSON{Callback: c.Query("callback"), Data: obj})
}
// JSON serializes the given struct as JSON into the response body.

View File

@ -12,12 +12,12 @@ import (
"fmt"
"io"
"log"
"net"
"net/http"
"net/http/httputil"
"os"
"runtime"
"strings"
"syscall"
"time"
"github.com/gin-gonic/gin/internal/bytesconv"
@ -57,40 +57,33 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc {
}
return func(c *Context) {
defer func() {
if err := recover(); err != nil {
if rec := recover(); rec != nil {
// Check for a broken connection, as it is not really a
// condition that warrants a panic stack trace.
var brokenPipe bool
if ne, ok := err.(*net.OpError); ok {
var se *os.SyscallError
if errors.As(ne, &se) {
seStr := strings.ToLower(se.Error())
if strings.Contains(seStr, "broken pipe") ||
strings.Contains(seStr, "connection reset by peer") {
brokenPipe = true
}
}
}
if e, ok := err.(error); ok && errors.Is(e, http.ErrAbortHandler) {
brokenPipe = true
var isBrokenPipe bool
err, ok := rec.(error)
if ok {
isBrokenPipe = errors.Is(err, syscall.EPIPE) ||
errors.Is(err, syscall.ECONNRESET) ||
errors.Is(err, http.ErrAbortHandler)
}
if logger != nil {
if brokenPipe {
logger.Printf("%s\n%s%s", err, secureRequestDump(c.Request), reset)
if isBrokenPipe {
logger.Printf("%s\n%s%s", rec, secureRequestDump(c.Request), reset)
} else if IsDebugging() {
logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s",
timeFormat(time.Now()), secureRequestDump(c.Request), err, stack(stackSkip), reset)
timeFormat(time.Now()), secureRequestDump(c.Request), rec, stack(stackSkip), reset)
} else {
logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s",
timeFormat(time.Now()), err, stack(stackSkip), reset)
timeFormat(time.Now()), rec, stack(stackSkip), reset)
}
}
if brokenPipe {
if isBrokenPipe {
// If the connection is dead, we can't write a status to it.
c.Error(err.(error)) //nolint: errcheck
c.Error(err) //nolint: errcheck
c.Abort()
} else {
handle(c, err)
handle(c, rec)
}
}
}()

View File

@ -98,13 +98,13 @@ func TestFunction(t *testing.T) {
func TestPanicWithBrokenPipe(t *testing.T) {
const expectCode = 204
expectMsgs := map[syscall.Errno]string{
syscall.EPIPE: "broken pipe",
syscall.ECONNRESET: "connection reset by peer",
expectErrnos := []syscall.Errno{
syscall.EPIPE,
syscall.ECONNRESET,
}
for errno, expectMsg := range expectMsgs {
t.Run(expectMsg, func(t *testing.T) {
for _, errno := range expectErrnos {
t.Run("Recovery from "+errno.Error(), func(t *testing.T) {
var buf strings.Builder
router := New()
@ -122,7 +122,8 @@ func TestPanicWithBrokenPipe(t *testing.T) {
w := PerformRequest(router, http.MethodGet, "/recovery")
// TEST
assert.Equal(t, expectCode, w.Code)
assert.Contains(t, strings.ToLower(buf.String()), expectMsg)
assert.Contains(t, strings.ToLower(buf.String()), errno.Error())
assert.NotContains(t, strings.ToLower(buf.String()), "[Recovery]")
})
}
}

View File

@ -115,14 +115,14 @@ func (r SecureJSON) WriteContentType(w http.ResponseWriter) {
// Render (JsonpJSON) marshals the given interface object and writes it and its callback with custom ContentType.
func (r JsonpJSON) Render(w http.ResponseWriter) (err error) {
r.WriteContentType(w)
ret, err := json.API.Marshal(r.Data)
if err != nil {
return err
if r.Callback == "" {
return WriteJSON(w, r.Data)
}
if r.Callback == "" {
_, err = w.Write(ret)
r.WriteContentType(w)
ret, err := json.API.Marshal(r.Data)
if err != nil {
return err
}

View File

@ -183,19 +183,28 @@ func TestRenderJsonpJSONError(t *testing.T) {
assert.Equal(t, `write "`+`);`+`" error`, err.Error())
}
func TestRenderJsonpJSONError2(t *testing.T) {
func TestRenderJsonpJSONWithEmptyCallback(t *testing.T) {
w := httptest.NewRecorder()
data := map[string]any{
"foo": "bar",
"num": 42,
"nested": map[string]any{
"key": "value",
},
}
(JsonpJSON{"", data}).WriteContentType(w)
assert.Equal(t, "application/javascript; charset=utf-8", w.Header().Get("Content-Type"))
e := (JsonpJSON{"", data}).Render(w)
require.NoError(t, e)
err := (JsonpJSON{Callback: "", Data: data}).Render(w)
assert.JSONEq(t, "{\"foo\":\"bar\"}", w.Body.String())
assert.Equal(t, "application/javascript; charset=utf-8", w.Header().Get("Content-Type"))
require.NoError(t, err)
// Verify Content-Type is set to jsonContentType when callback is empty
assert.Equal(t, "application/json; charset=utf-8", w.Header().Get("Content-Type"))
renderData, err := json.API.Marshal(data)
require.NoError(t, err)
// Verify body contains correct JSON data
assert.JSONEq(t, string(renderData), w.Body.String())
}
func TestRenderJsonpJSONFail(t *testing.T) {