When https://github.com/nodejs/node/pull/57917 added support for sending
raw header arrays, Http2Stream#sentHeaders was set only for header
objects. This change also sets it for raw headers by lazily
instantiating the property to avoid any performance impact on the fast
path.
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/59244
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58669
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58602
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Refs: https://datatracker.ietf.org/doc/html/rfc9113#section-5.3.1
PR-URL: https://github.com/nodejs/node/pull/58293
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58449
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
This is a followup cleanup on a previous PR. Current source code and doc
explicitly states that `selectPadding` has been removed for all exports.
Refs: https://github.com/nodejs/node/pull/29144
Refs: https://nodejs.org/api/http2.html
PR-URL: https://github.com/nodejs/node/pull/58373
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58390
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58317
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58292
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/58246
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/57916
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: James M Snell <jasnell@gmail.com>
This also notably changes error handling for request(). Previously some
invalid header values (but not all) would cause the session to be
unnecessarily destroyed automatically, e.g. passing an unparseable
header name to request(). This is no longer the case: header validation
failures will throw an error, but will not destroy the session or emit
'error' events.
PR-URL: https://github.com/nodejs/node/pull/57917
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This change adds proper tracking of HTTP / 2 server sessions
to ensure they are gracefully closed when the server is
shut down.It implements:
- A new kSessions symbol for tracking active sessions
- Adding/removing sessions from a SafeSet in the server
- A closeAllSessions helper function to close active sessions
- Updates to Http2Server and Http2SecureServer close methods
Breaking Change: any client trying to create new requests
on existing connections will not be able to do so once
server close is initiated
Refs: https://datatracker.ietf.org/doc/html/rfc7540\#section-9.1
Refs: https://nodejs.org/api/http.html\#serverclosecallback
- improve HTTP/2 server shutdown to prevent race conditions
1. Fix server shutdown race condition
- Stop listening for new connections before closing existing ones
- Ensure server.close() properly completes in all scenarios
2. Improve HTTP/2 tests
- Replace setTimeout with event-based flow control
- Simplify test logic for better readability
- Add clear state tracking for event ordering
- Improve assertions to verify correct shutdown sequence
This eliminates a race condition where new sessions could connect
between the time existing sessions are closed and the server stops
listening, potentially preventing the server from fully shutting down.
- fix cross-platform test timing issues
Fix test-http2-server-http1-client.js failure on Ubuntu
by deferring server.close() to next event loop cycle.
The issue only affected Ubuntu where session close occurs
before error emission, causing the test to miss errors
when HTTP/1 clients connect to HTTP/2 servers.
Using setImmediate() ensures error events fire before
server close across all platforms while maintaining
recent session handling improvements.
PR-URL: https://github.com/nodejs/node/pull/57586
Fixes: https://github.com/nodejs/node/issues/57611
Refs: https://datatracker.ietf.org/doc/html/rfc7540#section-9.1
Refs: https://nodejs.org/api/http.html#serverclosecallback
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit fixes a memory leak when the socket is
suddenly closed by the peer (without GOAWAY notification)
and when invalid header (by nghttp2) is identified and the
connection is terminated by peer.
Refs: https://hackerone.com/reports/2841362
PR-URL: https://github.com/nodejs-private/node-private/pull/650
Reviewed-By: James M Snell <jasnell@gmail.com>
CVE-ID: CVE-2025-23085
Create and store an AsyncResource for each stream, following a similar
approach as used in HttpAgent.
Fixes: https://github.com/nodejs/node/issues/55376
PR-URL: https://github.com/nodejs/node/pull/55460
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
PR-URL: https://github.com/nodejs/node/pull/53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
When Http2SecureServer is configured with `allowHTTP1=true`, it calls
`setupConnectionsTracking` to start monitoring for idle HTTP1
connections. `setupConnectionsTracking` expects to have
`this.connectionsCheckingInterval` property defined, but it does not
exist on `Http2SecureServer`. This `undefined` value is passed to
`setInterval`, which results in `checkConnections` being called on
every tick, creating significant additional load on the server CPU.
The fix is to define `this.connectionsCheckingInterval` on the
Http2SecureServer instance.
Refs: https://github.com/nodejs/node/pull/51569
PR-URL: https://github.com/nodejs/node/pull/52713
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
PR-URL: https://github.com/nodejs/node/pull/52625
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
This commit gives node.js the ability to also receive custom settings,
in addition to sending, them which was implemented before.
The custom settings received are limited to setting ids,
that were specified before, when creating the session eithers through
the server or the client.
PR-URL: https://github.com/nodejs/node/pull/51323
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Currently, node.js http/2 is limited in sending SETTINGs,
that are currently implemented by nghttp2.
However, nghttp2 has the ability to send arbitary SETTINGs,
that are not known beforehand.
This patch adds this feature including a fall back mechanism,
if a SETTING is implemented in a later nghttp2 or node version.
Fixes: https://github.com/nodejs/node/issues/1337
PR-URL: https://github.com/nodejs/node/pull/49025
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
A detailed analysis of the cause of this bug is in my linked comment on
the corresponding issue. The primary fix is the new setImmediate call in
Http2Stream#_destroy, which prevents a re-entrant call into
Http2Session::SendPendingData when sending trailers after the
Http2Session has been shut down, allowing the trailer data to be flushed
properly before the socket is closed.
As a result of this change, writes can be initiated later in the
lifetime of the Http2Session. So, when a JSStreamSocket is used as the
underlying socket reference for an Http2Session, it needs to be able to
accept write calls after it is closed.
In addition, now that outgoing data can be flushed differently after a
session is closed, in two tests clients receive errors that they
previously did not receive. I believe the new errors are more correct,
so I changed the tests to match.
Fixes: https://github.com/nodejs/node/issues/42713
Refs: https://github.com/nodejs/node/issues/42713#issuecomment-1756140062
PR-URL: https://github.com/nodejs/node/pull/50202
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: https://github.com/nodejs/node/pull/48338
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Use validateBuffer to remove duplicate implementation.
PR-URL: https://github.com/nodejs/node/pull/46489
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
fixup: add support for `Object.create(null)`
fixup: extend to any 1-argument Object.create call
fixup: add tests
PR-URL: https://github.com/nodejs/node/pull/46083
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
`kEmptyObject` is more suitable than {} if options don't
need mutation.
PR-URL: https://github.com/nodejs/node/pull/46011
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
