deceiver+handle-thing: fix method lookup and -4095 EOF crash on Node 16+#10
Merged
Conversation
Use anandsuresh/spdy-transport instead of the public npm package until required changes have been merged and released on the public branch.
Currently, the node-spdy agent seems to emit an "aborted" event on every incoming response because the `ending` flag isn't set for agent streams. This commit sets the `ending` flag to true for agent streams. Fixes spdy-http2#281
Under certain conditions, it is possible for the code to send frames AFTER a remote server has sent a GOAWAY frame. This commit fixes the problem.
The old code was missing a handler for the connection-level "error" events. This commit adds an error-handler that re-emits the "error" on the agent that user-code can handle as needed.
… and update handle.js to use the new module. Remove http-deceiver from package.json dependencies.
fix: vendor http-deceiver, drop abandoned dep, fix Node 16+ compat
`process.binding('http_parser')` was removed in Node 16. The previous
fallback set `mode = 'unsupported'` and used string callback names
(`'onHeadersComplete'` etc.), but the parser instances on modern Node
expose those callbacks on numeric slots only, so any incoming
request crashed the process with:
TypeError: this[kOnHeadersComplete] is not a function
at HTTPParser.execute (.../spdy/lib/spdy/deceiver.js:177:31)
Hit on gcp1-prod-0064 (NR processes nr128-nr135).
Load `HTTPParser` from `require('node:_http_common')` (with
`require('_http_common')` as a tier-2 fallback) and the methods list
from `require('http').METHODS` when `HTTPParser.methods` is missing.
This keeps the `mode === 'modern'` numeric-slot path which matches
how the modern parser is wired.
Smoke-tested on Node v24.11.1: spdy.createServer({plain:true}) now
parses an HTTP/1.1 request and returns 200 OK; before this change the
same request triggered the uncaughtException above.
Co-authored-by: Cursor <cursoragent@cursor.com>
…on Node 16+
Two follow-ups to the earlier deceiver fix; both surfaced when bringing
up gcp2-dev03-00 on Node 24.
1. handle-thing@^1.2.5 calls socket._handle.onread with the old
(nread, buffer) signature. On Node 11.1+, the handle's onread is
actually node:internal/stream_base_commons.onStreamRead(arrayBuffer),
which reads nread from streamBaseState[kReadBytesOrError]. Passing
UV_EOF (-4095) as the arrayBuffer arg made onStreamRead try
`new FastBuffer(-4095, ...)` and crash:
RangeError: Invalid typed array length: -4095
at new FastBuffer (node:internal/buffer:956:1)
at Handle.onStreamRead [as onread]
(node:internal/stream_base_commons:188:19)
at Stream.<anonymous> (.../handle-thing/lib/handle.js:120:12)
handle-thing@2.0.1 (spdy-http2/handle-thing#13) translates the
call: it sets streamBaseState[kReadBytesOrError] first and then
invokes onread with just the buffer. It also adds .writev which
modern net.Socket needs. Bump the dependency.
2. The previous deceiver fallback used `require('http').METHODS` for
the methods list. http.METHODS is alphabetically sorted, but the
parser's internal index is the llhttp/http_parser order
(DELETE, GET, HEAD, POST, ...). Sending the alphabetical index made
Node interpret POST as ACL etc. Use `_http_common.methods`
(or `node:_http_common.methods`), which is the real parser-indexed
list.
Smoke-tested end-to-end on Node v24.11.1: spdy.createServer with TLS
+ h2 alpn, hit with the built-in http2 client, POST /, 11-byte body,
server sees method=POST, body bytes flow through handle-thing's data
path, EOF path completes without the -4095 crash, client receives
200 OK with the response body.
Co-authored-by: Cursor <cursoragent@cursor.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.
Summary
Two follow-ups to #N (the deceiver fix that loaded
HTTPParserfromnode:_http_common). Both surfaced when bringing upgcp2-dev03-00on Node 24.
1. handle-thing bump (
^1.2.5→^2.0.1)handle-thing@1.2.5callssocket._handle.onread(nread, buffer).Since Node 11.1 the handle's
onreadisinternal/stream_base_commons.onStreamRead(arrayBuffer), whichreads
nreadfromstreamBaseState[kReadBytesOrError]. PassingUV_EOF(-4095) as the arrayBuffer arg madeonStreamReadconstruct
new FastBuffer(-4095, ...)and crash on the request's'end'event:handle-thing@2.0.1(spdy-http2/handle-thing#13) translates thecall: it sets
streamBaseState[kReadBytesOrError]first, theninvokes
onreadwith just the buffer. It also adds.writevwhich modern
net.Socketneeds.2. Method-list source in deceiver
The previous deceiver fallback used
require('http').METHODSforthe methods list.
http.METHODSis alphabetically sorted, butthe parser's index space is the llhttp/http_parser order (DELETE,
GET, HEAD, POST, …). Sending the alphabetical index made Node
interpret POST as ACL etc.
Use
_http_common.methods(ornode:_http_common.methods), whichis the real parser-indexed list.
Verification (Node v24.11.1)
spdy.createServerwith TLS + h2 ALPN, hit with the built-inhttp2client. Before this PR:RangeError: Invalid typed array length: -4095on request end. After: