Skip to content

fix: keep http.Hijacker when wrapping a response writer - #1387

Open
knQzx wants to merge 1 commit into
getsentry:masterfrom
knQzx:fix/wrap-response-writer-hijacker
Open

fix: keep http.Hijacker when wrapping a response writer#1387
knQzx wants to merge 1 commit into
getsentry:masterfrom
knQzx:fix/wrap-response-writer-hijacker

Conversation

@knQzx

@knQzx knQzx commented Aug 11, 2026

Copy link
Copy Markdown

NewWrapResponseWriter only returns a wrapper with a Hijack method when the underlying writer implements http.Flusher, http.Hijacker and io.ReaderFrom at once. a writer that supports flushing and hijacking but not the io.ReaderFrom optimization falls through to flushWriter, which has no Hijack, so protocol upgrades behind sentryhttp and sentrynegroni fail with "ResponseWriter is not an http.Hijacker"

losing functionality because an optimization is missing looks unintended: the upstream chi wrapper this file is derived from has flushHijackWriter and hijackWriter fallbacks for exactly these combinations, and they were dropped in this copy. the CHANGELOG also declares "Add support for the http.Hijacker interface", so hijacking is meant to survive the wrapper

this restores both fallbacks and adds tests for the three hijackable combinations

NewWrapResponseWriter only returns a wrapper with a Hijack method when the
underlying writer implements http.Flusher, http.Hijacker and io.ReaderFrom
at once. A writer that supports flushing and hijacking but not the
io.ReaderFrom optimization falls through to flushWriter, which drops
Hijack, so protocol upgrades behind the middleware fail.

Restore the flushHijackWriter and hijackWriter fallbacks from the upstream
chi wrapper this file is derived from.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant