From 45b805f6d59ba0b4f315adffe81ed4a82a51a591 Mon Sep 17 00:00:00 2001 From: Name <1911860538@qq.com> Date: Sat, 2 Aug 2025 12:30:14 +0800 Subject: [PATCH] perf(recovery): optimize the log output of CustomRecoveryWithWriter (#4258) Co-authored-by: 1911860538 --- recovery.go | 39 ++++++++++++----------- recovery_test.go | 80 +++++++++++++++++++++++++++++++++++++----------- 2 files changed, 83 insertions(+), 36 deletions(-) diff --git a/recovery.go b/recovery.go index 8a077dbb..fdd463f3 100644 --- a/recovery.go +++ b/recovery.go @@ -17,6 +17,8 @@ import ( "runtime" "strings" "time" + + "github.com/gin-gonic/gin/internal/bytesconv" ) const dunno = "???" @@ -67,19 +69,15 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { } } if logger != nil { - stack := stack(3) - httpRequest, _ := httputil.DumpRequest(c.Request, false) - headers := strings.Split(string(httpRequest), "\r\n") - maskAuthorization(headers) - headersToStr := strings.Join(headers, "\r\n") + const stackSkip = 3 if brokenPipe { - logger.Printf("%s\n%s%s", err, headersToStr, reset) + logger.Printf("%s\n%s%s", err, secureRequestDump(c.Request), reset) } else if IsDebugging() { logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s", - timeFormat(time.Now()), headersToStr, err, stack, reset) + timeFormat(time.Now()), secureRequestDump(c.Request), err, stack(stackSkip), reset) } else { logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s", - timeFormat(time.Now()), err, stack, reset) + timeFormat(time.Now()), err, stack(stackSkip), reset) } } if brokenPipe { @@ -95,6 +93,21 @@ func CustomRecoveryWithWriter(out io.Writer, handle RecoveryFunc) HandlerFunc { } } +// secureRequestDump returns a sanitized HTTP request dump where the Authorization header, +// if present, is replaced with a masked value ("Authorization: *") to avoid leaking sensitive credentials. +// +// Currently, only the Authorization header is sanitized. All other headers and request data remain unchanged. +func secureRequestDump(r *http.Request) string { + httpRequest, _ := httputil.DumpRequest(r, false) + lines := strings.Split(bytesconv.BytesToString(httpRequest), "\r\n") + for i, line := range lines { + if strings.HasPrefix(line, "Authorization:") { + lines[i] = "Authorization: *" + } + } + return strings.Join(lines, "\r\n") +} + func defaultHandleRecovery(c *Context, _ any) { c.AbortWithStatus(http.StatusInternalServerError) } @@ -126,16 +139,6 @@ func stack(skip int) []byte { return buf.Bytes() } -// maskAuthorization replaces any "Authorization: " header with "Authorization: *", hiding sensitive credentials. -func maskAuthorization(headers []string) { - for idx, header := range headers { - key, _, _ := strings.Cut(header, ":") - if strings.EqualFold(key, "Authorization") { - headers[idx] = key + ": *" - } - } -} - // source returns a space-trimmed slice of the n'th line. func source(lines [][]byte, n int) []byte { n-- // in stack trace, lines are 1-indexed but our array is 0-indexed diff --git a/recovery_test.go b/recovery_test.go index 3a36fad9..8a9e3475 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -88,24 +88,6 @@ func TestPanicWithAbort(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) } -func TestMaskAuthorization(t *testing.T) { - secret := "Bearer aaaabbbbccccddddeeeeffff" - headers := []string{ - "Host: www.example.com", - "Authorization: " + secret, - "User-Agent: curl/7.51.0", - "Accept: */*", - "Content-Type: application/json", - "Content-Length: 1", - } - maskAuthorization(headers) - - for _, h := range headers { - assert.NotContains(t, h, secret) - } - assert.Contains(t, headers, "Authorization: *") -} - func TestSource(t *testing.T) { bs := source(nil, 0) assert.Equal(t, dunnoBytes, bs) @@ -263,3 +245,65 @@ func TestRecoveryWithWriterWithCustomRecovery(t *testing.T) { SetMode(TestMode) } + +func TestSecureRequestDump(t *testing.T) { + tests := []struct { + name string + req *http.Request + wantContains string + wantNotContain string + }{ + { + name: "Authorization header standard case", + req: func() *http.Request { + r, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + r.Header.Set("Authorization", "Bearer secret-token") + return r + }(), + wantContains: "Authorization: *", + wantNotContain: "Bearer secret-token", + }, + { + name: "authorization header lowercase", + req: func() *http.Request { + r, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + r.Header.Set("authorization", "some-secret") + return r + }(), + wantContains: "Authorization: *", + wantNotContain: "some-secret", + }, + { + name: "Authorization header mixed case", + req: func() *http.Request { + r, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + r.Header.Set("AuThOrIzAtIoN", "token123") + return r + }(), + wantContains: "Authorization: *", + wantNotContain: "token123", + }, + { + name: "No Authorization header", + req: func() *http.Request { + r, _ := http.NewRequest(http.MethodGet, "http://example.com", nil) + r.Header.Set("Content-Type", "application/json") + return r + }(), + wantContains: "", + wantNotContain: "Authorization: *", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := secureRequestDump(tt.req) + if tt.wantContains != "" && !strings.Contains(result, tt.wantContains) { + t.Errorf("maskHeaders() = %q, want contains %q", result, tt.wantContains) + } + if tt.wantNotContain != "" && strings.Contains(result, tt.wantNotContain) { + t.Errorf("maskHeaders() = %q, want NOT contain %q", result, tt.wantNotContain) + } + }) + } +}