From 3498e1cdbcd2db17e8234d5f7c43bd12f14f814c Mon Sep 17 00:00:00 2001 From: okxint Date: Wed, 2 Sep 2026 12:29:43 +0530 Subject: [PATCH 1/2] fix(render): encode non-BMP characters as UTF-16 surrogate pairs in AsciiJSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AsciiJSON.Render used fmt.Appendf(buf, "\\u%04x", r) for all non-ASCII runes. The %04x format is a *minimum* width, not a fixed width, so runes above U+FFFF (emoji, musical symbols, supplementary CJK, etc.) produce a 5-digit escape like \u1f600. JSON \u escapes are always exactly 4 hex digits (RFC 8259 ยง7), so a decoder reads the first 4 digits as a different character and treats the remaining digit(s) as literal text. The output is still syntactically valid JSON, which makes the corruption easy to miss โ€” values silently fail to round-trip. Fix: detect runes > U+FFFF and split them into a UTF-16 surrogate pair (\uHHHH\uLLLL) before writing the escape. BMP characters continue to use the single-escape path unchanged. Add TestRenderAsciiJSONNonBMP to verify correct surrogate-pair encoding and full round-trip fidelity for ๐Ÿ˜€ (U+1F600), ๐„ž (U+1D11E), and ๐ €€ (U+20000). Fixes #4688 Co-Authored-By: Claude Sonnet 4.6 --- render/json.go | 14 ++++++++++++-- render/render_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/render/json.go b/render/json.go index 2f98676c..8fb13dee 100644 --- a/render/json.go +++ b/render/json.go @@ -160,11 +160,21 @@ func (r AsciiJSON) Render(w http.ResponseWriter) error { } var buffer bytes.Buffer - escapeBuf := make([]byte, 0, 6) // Preallocate 6 bytes for Unicode escape sequences + escapeBuf := make([]byte, 0, 12) // 12 bytes for a UTF-16 surrogate pair (\uHHHH\uLLLL) for _, r := range bytesconv.BytesToString(ret) { if r > unicode.MaxASCII { - escapeBuf = fmt.Appendf(escapeBuf[:0], "\\u%04x", r) // Reuse escapeBuf + if r > 0xFFFF { + // Non-BMP character: encode as a UTF-16 surrogate pair per RFC 8259 ยง7. + // \u alone cannot represent code points above U+FFFF (it is always 4 hex digits), + // so we split the rune into a high surrogate and a low surrogate. + r -= 0x10000 + high := 0xD800 + (r>>10)&0x3FF + low := 0xDC00 + r&0x3FF + escapeBuf = fmt.Appendf(escapeBuf[:0], "\\u%04x\\u%04x", high, low) + } else { + escapeBuf = fmt.Appendf(escapeBuf[:0], "\\u%04x", r) + } buffer.Write(escapeBuf) } else { buffer.WriteByte(byte(r)) diff --git a/render/render_test.go b/render/render_test.go index f63878b9..68a06c9f 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -5,6 +5,7 @@ package render import ( + "encoding/json" "encoding/xml" "errors" "html/template" @@ -261,6 +262,48 @@ func TestRenderAsciiJSON(t *testing.T) { assert.Equal(t, "3.1415926", w2.Body.String()) } +func TestRenderAsciiJSONNonBMP(t *testing.T) { + // Non-BMP code points (> U+FFFF) must be encoded as UTF-16 surrogate pairs. + // Previously, fmt.Appendf(buf, "\\u%04x", r) emitted 5+ hex digits for such + // runes, which is invalid JSON โ€” decoders misread the first 4 digits as a + // different character and left the remaining digit(s) as literal text. + // RFC 8259 ยง7: \u escapes are exactly 4 hex digits; non-BMP values use pairs. + + cases := []struct { + name string + input string + want string // exact \uHHHH\uLLLL literal + decoded string // value after json.Unmarshal round-trip + }{ + // ๐Ÿ˜€ U+1F600: high=\uD83D low=\uDE00 + {"grinning face", "๐Ÿ˜€", `\ud83d\ude00`, "๐Ÿ˜€"}, + // ๐„ž U+1D11E: high=\uD834 low=\uDD1E + {"musical symbol G clef", "๐„ž", `\ud834\udd1e`, "๐„ž"}, + // ๐ €€ U+20000 (first CJK Extension B): high=\uD840 low=\uDC00 + {"CJK Extension B first", "๐ €€", `\ud840\udc00`, "๐ €€"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := httptest.NewRecorder() + err := (AsciiJSON{map[string]string{"v": tc.input}}).Render(w) + require.NoError(t, err) + + body := w.Body.String() + + // The surrogate pair must appear verbatim in the raw output. + assert.Contains(t, body, tc.want, + "expected surrogate pair %q in raw output %q", tc.want, body) + + // The round-trip via json.Unmarshal must recover the original rune. + var decoded map[string]string + require.NoError(t, json.Unmarshal([]byte(body), &decoded)) + assert.Equal(t, tc.decoded, decoded["v"], + "round-trip mismatch: got %q want %q", decoded["v"], tc.decoded) + }) + } +} + func TestRenderAsciiJSONFail(t *testing.T) { w := httptest.NewRecorder() data := make(chan int) From b05d45dbb144a322ebb6384950a20570858946c6 Mon Sep 17 00:00:00 2001 From: okxint Date: Wed, 2 Sep 2026 12:31:11 +0530 Subject: [PATCH 2/2] fix: reset skippedNodes before each getValue call in HandleMethodNotAllowed loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With HandleMethodNotAllowed enabled, handleHTTPRequest searches every method tree to build the Allow response header. It reuses the pooled Context's skippedNodes slice across all getValue calls in that loop. getValue extends the slice with a raw reslice: *skippedNodes = (*skippedNodes)[:index+1] // tree.go:435 The slice has a fixed capacity equal to engine.maxSections (allocated once when the Context is created). Context.reset() zeroes the length at the start of each request, but nothing zeroes it between getValue calls inside the HandleMethodNotAllowed loop. With enough wildcard routes, residual entries from earlier trees fill the slice until the reslice exceeds capacity and panics: panic: runtime error: slice bounds out of range [:7] with capacity 6 Because the panic fires during route matching โ€” before any handler runs โ€” gin.Recovery() cannot intercept it. net/http's per-connection recover logs "http: panic serving โ€ฆ" and closes the connection with no response; callers see an empty reply or a connection reset. Fix: zero the slice length before each getValue call in the loop. This matches the same reset Context.reset() performs at request start and costs only a single pointer write per method tree. Add TestMethodNotAllowedSkippedNodesPanic reproducing the exact route set and request from issue #4818 to guard against regression. Fixes #4818 Co-Authored-By: Claude Sonnet 4.6 --- gin.go | 5 +++++ gin_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/gin.go b/gin.go index 2e033bf3..2b141a64 100644 --- a/gin.go +++ b/gin.go @@ -743,6 +743,11 @@ func (engine *Engine) handleHTTPRequest(c *Context) { if tree.method == httpMethod { continue } + // Reset skippedNodes before each getValue call so residue from the + // previous tree does not accumulate. The slice is allocated once per + // pooled Context with a fixed capacity (engine.maxSections); getValue + // extends it with a raw reslice that panics if len exceeds cap. + *c.skippedNodes = (*c.skippedNodes)[:0] if value := tree.root.getValue(rPath, nil, c.skippedNodes, unescape); value.handlers != nil { allowed = append(allowed, tree.method) } diff --git a/gin_test.go b/gin_test.go index a9cf1755..4ec73f7a 100644 --- a/gin_test.go +++ b/gin_test.go @@ -1058,6 +1058,38 @@ func TestMethodNotAllowedNoRoute(t *testing.T) { assert.Equal(t, http.StatusNotFound, resp.Code) } +// TestMethodNotAllowedSkippedNodesPanic is a regression test for #4818. +// With HandleMethodNotAllowed enabled, handleHTTPRequest calls getValue once +// per method tree to build the Allow header. Each call may push onto +// c.skippedNodes, which has a fixed capacity (engine.maxSections). Without a +// reset between calls the slice fills up and a subsequent reslice panics with +// "slice bounds out of range". Because the panic occurs before any handler +// runs, gin.Recovery() cannot catch it and the client receives an empty reply. +func TestMethodNotAllowedSkippedNodesPanic(t *testing.T) { + SetMode(ReleaseMode) + router := New() + router.HandleMethodNotAllowed = true + + h := func(c *Context) {} + router.OPTIONS("/:p0/:p1/a/:p2", h) + router.GET("/:p0/:p1/a/:p2", h) + router.PATCH("/b/:p0/:p1/c", h) + router.DELETE("/b/:p0/:p1/d/:p3", h) + router.GET("/b/:p0/:p1/e/f", h) + router.POST("/b/:p0/:p1/g/:p4/h", h) + router.OPTIONS("/b/:p0/:p1/g/:p4/h", h) + router.DELETE("/b/cache", h) + router.GET("/b/clients/:p1/g", h) + router.POST("/b/clients/:p1/g", h) + router.PATCH("/b/clients/:p1/g/:p4", h) + router.OPTIONS("/b/clients/:p1/g/:p4", h) + + req := httptest.NewRequest(http.MethodPost, "/b/clients/42", nil) + w := httptest.NewRecorder() + assert.NotPanics(t, func() { router.ServeHTTP(w, req) }) + assert.Equal(t, http.StatusMethodNotAllowed, w.Code) +} + // Test the fix for https://github.com/gin-gonic/gin/pull/4415 func TestLiteralColonWithRun(t *testing.T) { SetMode(TestMode)