-
Notifications
You must be signed in to change notification settings - Fork 5k
http/http2: node v26.3.0 compat — HTTP/1 fallback + upgrade handoff, http2 session errors, perf_hooks and frame framing (+11 upstream tests) #34432
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
9383d83
http: honor --insecure-http-parser, freeze writeHead framing, drop ov…
cirospaciari 69fc04f
Merge remote-tracking branch 'origin/main' into ciro/node-http-v26-co…
cirospaciari c5a99f8
http: fix framing/leniency edge cases found in review
cirospaciari df4965e
http: run a connectionListener so emit('connection', socket) is served
cirospaciari 7cf0e01
http2: serve the HTTP/1 half of protocol autoselection
cirospaciari 7540792
http: bound request headers the way llhttp does
cirospaciari e800d5d
http: render the chunked Transfer-Encoding after Connection, like Node
cirospaciari 1d5e39f
test: the 200 after a 304 is chunked, like Node
cirospaciari 013e52f
http: honor the chunked Transfer-Encoding bit in the JS fallback
cirospaciari 11588aa
Merge branch 'main' into ciro/node-http-v26-compat
cirospaciari a733f96
http: charge a header value's trailing whitespace, like llhttp
cirospaciari 65377b9
Merge remote-tracking branch 'origin/ciro/node-http-v26-compat' into …
cirospaciari d0c0354
http: split parser leniency into llhttp's two bits
cirospaciari ac379e9
http: honor the auto-header bits exactly in the JS fallback
cirospaciari 4371163
http: bound pipelined dispatch like Node's flood prevention
cirospaciari a91f521
[autofix.ci] apply automated fixes
autofix-ci[bot] b8b6be5
http: hand off Upgrade and CONNECT in the JS fallback
cirospaciari d9a4921
Merge remote-tracking branch 'origin/ciro/node-http-v26-compat' into …
cirospaciari 09c0380
test: cover the fallback's Upgrade/CONNECT handoff
cirospaciari 6e654b4
Merge branch 'main' into ciro/node-http-v26-compat
cirospaciari ce59cfe
address review: CONNECT double-delivery, spill-replay backpressure, f…
robobun a76d5a8
Merge branch 'main' into ciro/node-http-v26-compat; address ce59cfe r…
robobun 6641668
[autofix.ci] apply automated fixes
autofix-ci[bot] 363c67c
http2: fail the session when an outbound header block cannot be encoded
cirospaciari 73065c5
http2: perf_hooks entries, END_STREAM on the final DATA frame, and cl…
cirospaciari b38891c
http2: pack END_STREAM onto end(chunk) and match node's frame accounting
cirospaciari 958112f
http2: fix silent client stall when the socket flows before connect c…
cirospaciari f4a9f71
[autofix.ci] apply automated fixes
autofix-ci[bot] 05ed257
h2: release auto-flush registration after deferred COMPRESSION_ERROR;…
robobun 754d353
ci: retrigger
robobun 9dd71e1
h2: keep pending_header_compression_error latched across the onError …
robobun 3a54f48
[autofix.ci] apply automated fixes
autofix-ci[bot] 4ce300b
fallback: add writeInformational so writeEarlyHints/writeProcessing r…
robobun be93d72
h2/fallback review: send_trailers encode failure -> session COMPRESSI…
robobun cb56096
test: wire must-not-run request handlers to reject/flag instead of th…
robobun 6d0f17b
revert send_trailers to per-stream FRAME_SIZE_ERROR; fix onHttp1Socke…
robobun a227e00
Merge branch 'main' into ciro/node-http-v26-compat
cirospaciari 8715a0e
Merge branch 'main' into ciro/node-http-v26-compat
robobun d361c02
docs: move rewrite_read doc comment back above its fn (sync_engine_fr…
robobun f641f77
fallback: map HPE_CHUNK_EXTENSIONS_OVERFLOW to 413 like Node's socket…
robobun 9ac1cf6
Merge branch 'main' into ciro/node-http-v26-compat
cirospaciari 77cbe93
http2: defer streamEnd(7) destroy when end(chunk)'s own _write drove …
robobun 3c6783e
[autofix.ci] apply automated fixes
autofix-ci[bot] 4485e5c
http2: stamp timeToFirstHeader in server streamHeaders too; fallback …
robobun 1af5840
Merge branch 'main' into ciro/node-http-v26-compat
cirospaciari fe85cfb
Merge remote-tracking branch 'origin/main' into ciro/node-http-v26-co…
robobun 4227bbf
NodeHTTP.cpp: hoist lenientHttpFlags.toInt32 and exception-check befo…
robobun b87e396
trim comments to <=3 lines, cite spec/node source
robobun b85baf4
test(spawn-maxbuf): widen killWindow on ASAN builds too
robobun 29b0954
Merge remote-tracking branch 'origin/main' into ciro/node-http-v26-co…
robobun 9a12c62
Merge remote-tracking branch 'origin/main' into ciro/node-http-v26-co…
robobun 64bcf70
node:http flood prevention: rename nodeHttpReadsPausedSignal -> nodeH…
robobun 59c93f1
http2.ts: drop unused throwNotImplemented import and unused bunTLSCon…
robobun 86d0bda
node:http flood prevention: hold the parked-request spill in a WTF::V…
robobun 70833a7
http2: mark the push stream handed out on the unencodable-block path …
robobun e758733
test: restore node-http-proxy.js to its original localhost form
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@robobun this branch is for onReadsPaused. is it correct to be calling onReadsResumable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. This is inside
onWritable, which fires when outgoing backpressure has drained. The check is "reads are currently paused" and the call is "reads may now be resumable, decide and act".Bun__NodeHTTP__onReadsResumable(onNodeHttpReadsResumable<SSL>in JSNodeHTTPServerSocket.cpp) is the single decision point for whether to actually resume. It applies the same conditions the removed inline code did (getBufferedAmount() == 0, queued-pipeline state) plus the case the inline code could not handle: parked pipelined request bytes innodeHttpPausedSpill. Those were received while reads were paused mid-parse and must be replayed through the parser before raw reads resume, or the request stream reorders. The hook:getBufferedAmount() > 0(still backpressured);nodeHttpQueuedPipelinedCount > 0(queue-drain re-enters it later);HTTP_NODE_READS_PAUSEDand callsresume()(same as the old inline code);feedNodeHttpDataand only resumes raw reads once the spill has drained without re-pausing.Three call sites reach it (
onWritablehere,startPipelinedResponseImpl, and Rust'sresume_socket) so the resume conditions live in one place instead of being duplicated inline at each.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes —
HTTP_NODE_READS_PAUSEDis a state bit ("reads are currently paused"), not the event being fired. This is theonWritablepath right afterflush(), i.e. the outgoing backpressure that caused the pause just drained, so we notify the reads-resumable hook, which replays the parked spill and only then clears the bit andresume()s (seeonNodeHttpReadsResumablein JSNodeHTTPServerSocket.cpp:444-499 — sameif (state & HTTP_NODE_READS_PAUSED) onNodeHttpReadsResumable()pattern at :554). There is noonReadsPausedhook; pausing happens inline at HttpContext.h:407/451 viapause()+Bun__NodeHTTP__setReadsPausedSignal.