perf(recovery): optimize the log output of CustomRecoveryWithWriter (#4258)

Co-authored-by: 1911860538 <alxps1911@gmail.com>

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: <token>" 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

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

render/toml.go

@@ -15,7 +15,7 @@ type TOML struct {
 	Data any
 }
 
-var TOMLContentType = []string{"application/toml; charset=utf-8"}
+var tomlContentType = []string{"application/toml; charset=utf-8"}
 
 // Render (TOML) marshals the given interface object and writes data with custom ContentType.
 func (r TOML) Render(w http.ResponseWriter) error {
@@ -32,5 +32,5 @@ func (r TOML) Render(w http.ResponseWriter) error {
 
 // WriteContentType (TOML) writes TOML ContentType for response.
 func (r TOML) WriteContentType(w http.ResponseWriter) {
-	writeContentType(w, TOMLContentType)
+	writeContentType(w, tomlContentType)
 }

response_writer.go

@@ -6,6 +6,7 @@ package gin
 
 import (
 	"bufio"
+	"errors"
 	"io"
 	"net"
 	"net/http"
@@ -16,6 +17,8 @@ const (
 	defaultStatus = http.StatusOK
 )
 
+var errHijackAlreadyWritten = errors.New("gin: response already written")
+
 // ResponseWriter ...
 type ResponseWriter interface {
 	http.ResponseWriter
@@ -106,6 +109,9 @@ func (w *responseWriter) Written() bool {
 
 // Hijack implements the http.Hijacker interface.
 func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
+	if w.Written() {
+		return nil, nil, errHijackAlreadyWritten
+	}
 	if w.size < 0 {
 		w.size = 0
 	}

response_writer_test.go

@@ -5,6 +5,8 @@
 package gin
 
 import (
+	"bufio"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -124,6 +126,74 @@ func TestResponseWriterHijack(t *testing.T) {
 	w.Flush()
 }
 
+type mockHijacker struct {
+	*httptest.ResponseRecorder
+	hijacked bool
+}
+
+// Hijack implements the http.Hijacker interface. It just records that it was called.
+func (m *mockHijacker) Hijack() (net.Conn, *bufio.ReadWriter, error) {
+	m.hijacked = true
+	return nil, nil, nil
+}
+
+func TestResponseWriterHijackAfterWrite(t *testing.T) {
+	tests := []struct {
+		name                      string
+		action                    func(w ResponseWriter) error // Action to perform before hijacking
+		expectWrittenBeforeHijack bool
+		expectHijackSuccess       bool
+		expectWrittenAfterHijack  bool
+		expectError               error
+	}{
+		{
+			name:                      "hijack before write should succeed",
+			action:                    func(w ResponseWriter) error { return nil },
+			expectWrittenBeforeHijack: false,
+			expectHijackSuccess:       true,
+			expectWrittenAfterHijack:  true, // Hijack itself marks the writer as written
+			expectError:               nil,
+		},
+		{
+			name: "hijack after write should fail",
+			action: func(w ResponseWriter) error {
+				_, err := w.Write([]byte("test"))
+				return err
+			},
+			expectWrittenBeforeHijack: true,
+			expectHijackSuccess:       false,
+			expectWrittenAfterHijack:  true,
+			expectError:               errHijackAlreadyWritten,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			hijacker := &mockHijacker{ResponseRecorder: httptest.NewRecorder()}
+			writer := &responseWriter{}
+			writer.reset(hijacker)
+			w := ResponseWriter(writer)
+
+			// Check initial state
+			assert.False(t, w.Written(), "should not be written initially")
+
+			// Perform pre-hijack action
+			require.NoError(t, tc.action(w), "unexpected error during pre-hijack action")
+
+			// Check state before hijacking
+			assert.Equal(t, tc.expectWrittenBeforeHijack, w.Written(), "unexpected w.Written() state before hijack")
+
+			// Attempt to hijack
+			_, _, hijackErr := w.Hijack()
+
+			// Check results
+			require.ErrorIs(t, hijackErr, tc.expectError, "unexpected error from Hijack()")
+			assert.Equal(t, tc.expectHijackSuccess, hijacker.hijacked, "unexpected hijacker.hijacked state")
+			assert.Equal(t, tc.expectWrittenAfterHijack, w.Written(), "unexpected w.Written() state after hijack")
+		})
+	}
+}
+
 func TestResponseWriterFlush(t *testing.T) {
 	testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		writer := &responseWriter{}

test_helpers.go

@@ -6,7 +6,10 @@ package gin
 
 import "net/http"
 
-// CreateTestContext returns a fresh engine and context for testing purposes
+// CreateTestContext returns a fresh Engine and a Context associated with it.
+// This is useful for tests that need to set up a new Gin engine instance
+// along with a context, for example, to test middleware that doesn't depend on
+// specific routes. The ResponseWriter `w` is used to initialize the context's writer.
 func CreateTestContext(w http.ResponseWriter) (c *Context, r *Engine) {
 	r = New()
 	c = r.allocateContext(0)
@@ -15,7 +18,11 @@ func CreateTestContext(w http.ResponseWriter) (c *Context, r *Engine) {
 	return
 }
 
-// CreateTestContextOnly returns a fresh context base on the engine for testing purposes
+// CreateTestContextOnly returns a fresh Context associated with the provided Engine `r`.
+// This is useful for tests that operate on an existing, possibly pre-configured,
+// Gin engine instance and need a new context for it.
+// The ResponseWriter `w` is used to initialize the context's writer.
+// The context is allocated with the `maxParams` setting from the provided engine.
 func CreateTestContextOnly(w http.ResponseWriter, r *Engine) (c *Context) {
 	c = r.allocateContext(r.maxParams)
 	c.reset()