Repository navigation
connection: enforce status-written check before hijacking in QUIC (#1748) - #1766
Open
dedsec-terminal wants to merge 1 commit into
Open
dedsec-terminal wants to merge 1 commit into
dedsec-terminal wants to merge 1 commit into
Conversation
…oudflare#1748) Align httpResponseAdapter.Hijack precondition with http2RespWriter by requiring status/response headers to be written prior to hijacking. Reject subsequent Hijack and Write calls after hijacking with http.ErrHijacked. Closes cloudflare#1748 Signed-off-by: dedsec-terminal <dedsec.terminal@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
http.Hijackerpreconditions differed between the HTTP/2 and QUIC transports:http2RespWriter.Hijackinconnection/http2.go),Hijack()verifies that the response status was already written (rp.statusWritten) before returning the hijacked connection, returning an error (status not yet written before attempting to hijack connection) otherwise.httpResponseAdapter.Hijackinconnection/quic_connection.go), no precondition check existed, returning a connection even whenconnectResponseSentwas still false. Furthermore, repeatedHijack()or post-hijackWrite()calls did not return standardhttp.ErrHijacked.This change aligns
httpResponseAdapterwithhttp2RespWriter:connectResponseSentto be true before allowingHijack(). If not yet written, it returns the same error:status not yet written before attempting to hijack connection.hijackedstate under mutex protection.Hijack()calls returnhttp.ErrHijacked.Write()calls return0, http.ErrHijacked.WriteHeader()calls are safely ignored without attempting to write headers.Testing & Reproduction
Added unit test
TestHTTPResponseAdapter_Hijackinconnection/quic_connection_test.gocovering:Hijack()fails with"status not yet written before attempting to hijack connection"before response status is sent.Hijack()succeeds once response status is sent.Hijack()calls returnhttp.ErrHijacked.Write()calls return0, http.ErrHijacked.WriteHeader()after hijack is a safe no-op.All connection tests pass cleanly:
go test -v -run TestHTTPResponseAdapter_Hijack ./connectionChecked diff formatting with
git diff --check.Links
Closes #1748