From 175d5e8e12c6c715f3dbf8474c2d8fbdf5fe631e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 7 Apr 2026 20:51:50 +0000 Subject: [PATCH] fix: use safe type assertions in Hijack and CloseNotify to prevent panics Agent-Logs-Url: https://github.com/odlev/gin/sessions/a5300762-e9da-4790-8669-fd5200269ead Co-authored-by: odlev <65655276+odlev@users.noreply.github.com> --- response_writer.go | 18 +++++++++++--- response_writer_test.go | 52 +++++++++++++++++++++++++++++++++++------ 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/response_writer.go b/response_writer.go index 9035e6f1..305322b6 100644 --- a/response_writer.go +++ b/response_writer.go @@ -17,7 +17,10 @@ const ( defaultStatus = http.StatusOK ) -var errHijackAlreadyWritten = errors.New("gin: response body already written") +var ( + errHijackAlreadyWritten = errors.New("gin: response body already written") + errHijackNotSupported = errors.New("gin: underlying ResponseWriter does not implement http.Hijacker") +) // ResponseWriter ... type ResponseWriter interface { @@ -117,12 +120,21 @@ func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) { if w.size < 0 { w.size = 0 } - return w.ResponseWriter.(http.Hijacker).Hijack() + if hijacker, ok := w.ResponseWriter.(http.Hijacker); ok { + return hijacker.Hijack() + } + return nil, nil, errHijackNotSupported } // CloseNotify implements the http.CloseNotifier interface. +// +// Deprecated: the CloseNotifier interface predates Go's context package. +// New code should use Request.Context instead. 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..85c1bbbb 100644 --- a/response_writer_test.go +++ b/response_writer_test.go @@ -113,15 +113,12 @@ func TestResponseWriterHijack(t *testing.T) { writer.reset(testWriter) w := ResponseWriter(writer) - assert.Panics(t, func() { - _, _, err := w.Hijack() - require.NoError(t, err) - }) + _, _, err := w.Hijack() + require.ErrorIs(t, err, errHijackNotSupported) assert.True(t, w.Written()) - assert.Panics(t, func() { - w.CloseNotify() - }) + ch := w.CloseNotify() + assert.Nil(t, ch) w.Flush() } @@ -315,3 +312,44 @@ func TestPusherWithoutPusher(t *testing.T) { pusher := w.Pusher() assert.Nil(t, pusher, "Expected pusher to be nil") } + +// mockCloseNotifier is an http.ResponseWriter that implements http.CloseNotifier. +type mockCloseNotifier struct { + *httptest.ResponseRecorder +} + +func (m *mockCloseNotifier) CloseNotify() <-chan bool { + ch := make(chan bool, 1) + return ch +} + +func TestCloseNotifyWithCloseNotifier(t *testing.T) { + rw := &mockCloseNotifier{ResponseRecorder: httptest.NewRecorder()} + w := &responseWriter{} + w.reset(rw) + + ch := w.CloseNotify() + assert.NotNil(t, ch, "Expected CloseNotify channel to be non-nil") +} + +func TestCloseNotifyWithoutCloseNotifier(t *testing.T) { + // httptest.NewRecorder does not implement http.CloseNotifier + rw := httptest.NewRecorder() + w := &responseWriter{} + w.reset(rw) + + ch := w.CloseNotify() + assert.Nil(t, ch, "Expected CloseNotify channel to be nil when underlying writer does not support it") +} + +func TestHijackWithoutHijacker(t *testing.T) { + // httptest.NewRecorder does not implement http.Hijacker + rw := httptest.NewRecorder() + w := &responseWriter{} + w.reset(rw) + + conn, buf, err := w.Hijack() + assert.Nil(t, conn) + assert.Nil(t, buf) + require.ErrorIs(t, err, errHijackNotSupported) +}