Skip to content

Commit a2792d8

Browse files
committed
fix: use safe type assertions and godoc for CloseNotify and Hijack
1 parent d3ffc99 commit a2792d8

2 files changed

Lines changed: 68 additions & 10 deletions

File tree

response_writer.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ const (
1717
defaultStatus = http.StatusOK
1818
)
1919

20-
var errHijackAlreadyWritten = errors.New("gin: response body already written")
20+
var (
21+
errHijackAlreadyWritten = errors.New("gin: response body already written")
22+
errHijackNotSupported = errors.New("gin: underlying ResponseWriter does not implement http.Hijacker")
23+
)
2124

2225
// ResponseWriter ...
2326
type ResponseWriter interface {
@@ -117,12 +120,25 @@ func (w *responseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
117120
if w.size < 0 {
118121
w.size = 0
119122
}
120-
return w.ResponseWriter.(http.Hijacker).Hijack()
123+
if hijacker, ok := w.ResponseWriter.(http.Hijacker); ok {
124+
return hijacker.Hijack()
125+
}
126+
return nil, nil, errHijackNotSupported
121127
}
122128

123129
// CloseNotify implements the http.CloseNotifier interface.
130+
//
131+
// If the underlying ResponseWriter doesn't implement http.CloseNotifier
132+
// (e.g. httptest.NewRecorder), the returned channel will never fire.
133+
// Use Request.Context().Done() to observe client disconnects instead.
134+
//
135+
// Deprecated: the CloseNotifier interface predates Go's context package.
136+
// New code should use Request.Context instead.
124137
func (w *responseWriter) CloseNotify() <-chan bool {
125-
return w.ResponseWriter.(http.CloseNotifier).CloseNotify()
138+
if cn, ok := w.ResponseWriter.(http.CloseNotifier); ok {
139+
return cn.CloseNotify()
140+
}
141+
return make(chan bool)
126142
}
127143

128144
// Flush implements the http.Flusher interface.

response_writer_test.go

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -113,15 +113,12 @@ func TestResponseWriterHijack(t *testing.T) {
113113
writer.reset(testWriter)
114114
w := ResponseWriter(writer)
115115

116-
assert.Panics(t, func() {
117-
_, _, err := w.Hijack()
118-
require.NoError(t, err)
119-
})
116+
_, _, err := w.Hijack()
117+
require.ErrorIs(t, err, errHijackNotSupported)
120118
assert.True(t, w.Written())
121119

122-
assert.Panics(t, func() {
123-
w.CloseNotify()
124-
})
120+
ch := w.CloseNotify()
121+
assert.NotNil(t, ch)
125122

126123
w.Flush()
127124
}
@@ -315,3 +312,48 @@ func TestPusherWithoutPusher(t *testing.T) {
315312
pusher := w.Pusher()
316313
assert.Nil(t, pusher, "Expected pusher to be nil")
317314
}
315+
316+
// mockCloseNotifier is an http.ResponseWriter that implements http.CloseNotifier.
317+
type mockCloseNotifier struct {
318+
*httptest.ResponseRecorder
319+
}
320+
321+
func (m *mockCloseNotifier) CloseNotify() <-chan bool {
322+
return make(chan bool)
323+
}
324+
325+
func TestCloseNotifyWithCloseNotifier(t *testing.T) {
326+
rw := &mockCloseNotifier{ResponseRecorder: httptest.NewRecorder()}
327+
w := &responseWriter{}
328+
w.reset(rw)
329+
330+
ch := w.CloseNotify()
331+
assert.NotNil(t, ch, "Expected CloseNotify channel to be non-nil")
332+
}
333+
334+
func TestCloseNotifyWithoutCloseNotifier(t *testing.T) {
335+
// httptest.NewRecorder does not implement http.CloseNotifier
336+
rw := httptest.NewRecorder()
337+
w := &responseWriter{}
338+
w.reset(rw)
339+
340+
ch := w.CloseNotify()
341+
assert.NotNil(t, ch, "Expected non-nil channel when CloseNotifier is not supported")
342+
select {
343+
case <-ch:
344+
t.Fatal("channel should never fire when CloseNotifier is not supported")
345+
default:
346+
}
347+
}
348+
349+
func TestHijackWithoutHijacker(t *testing.T) {
350+
// httptest.NewRecorder does not implement http.Hijacker
351+
rw := httptest.NewRecorder()
352+
w := &responseWriter{}
353+
w.reset(rw)
354+
355+
conn, buf, err := w.Hijack()
356+
assert.Nil(t, conn)
357+
assert.Nil(t, buf)
358+
require.ErrorIs(t, err, errHijackNotSupported)
359+
}

0 commit comments

Comments
 (0)