Node v24.16.0 nsolid v6.3.0 release#473
Open
santigimeno wants to merge 233 commits into
Open
Conversation
Node.js 24 is using Undici 7 rather than the latest version. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs/node#62739 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#63011 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs/node#60854 Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Ilyas Shabi <ilyasshabi94@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: nodejs/node#61295 Refs: nodejs/node#60618 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
PR-URL: nodejs/node#61829 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: nodejs/node#62324 Reviewed-By: Richard Lau <richard.lau@ibm.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: nodejs/node#62474 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
On AIX libc++ is returning `EEXIST` instead of `EACCES` when using `std::filesystem::remove_all()` without appropriate permissions to recursively remove the directory. Co-authored-by: Abdirahim Musse <abdirahim.musse@ibm.com> Signed-off-by: Richard Lau <richard.lau@ibm.com> PR-URL: nodejs/node#62788 Refs: nodejs/node#62790 Reviewed-By: Abdirahim Musse <abdirahim.musse@ibm.com> Reviewed-By: Stewart X Addison <sxa@redhat.com>
Expose `f_frsize` from libuv's `uv_statfs_t` as `statfs.frsize`. Per POSIX, `f_blocks`, `f_bfree`, and `f_bavail` are expressed in units of `f_frsize`, not `f_bsize`. On most filesystems the two values are typically equal, but some filesystem drivers report a different `f_bsize`, making it impossible to compute accurate disk usage without `frsize`. Refs: libuv/libuv#4983 PR-URL: nodejs/node#62277 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
PR-URL: nodejs/node#62504 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
PR-URL: nodejs/node#62504 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
We have dedicated requirements about memory leaks when triaging DoS. These applies in generall to all types of DoS, and many recent reports about DoS attack vectors fail to meet them, resulting in a lot of extra back-and-forth in triaging. Clarify in the threat model by expanding these requirements to all DoS. Drive-by: clarify criteria of documented JavaScript behavior is that they are included in ECMA262. Also use "Node.js application developer" instead of "user" the refer to the party being vulnerable to avoid confusion. PR-URL: nodejs/node#62505 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs/node#62523 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Avoid registering AbortSignal.any() composites as dependants until they are actually observed. This fixes the long-lived source retention pattern from #62363 while preserving abort semantics through lazy refresh and follow paths. Also unregister fired timeout signals from the timeout finalization registry so timeout churn releases memory more promptly. PR-URL: nodejs/node#62367 Fixes: nodejs/node#62363 Refs: nodejs/node#54614 Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
PR-URL: nodejs/node#62524 Fixes: nodejs/node#62496 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Fixes: nodejs/node#62043 PR-URL: nodejs/node#62101 Fixes: nodejs/node#62043 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Apply suggestion from @jasnell PR-URL: nodejs/node#62510 Refs: nodejs/core-validate-commit#141 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Ruy Adorno <ruy@vlt.sh> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Bumps [actions/cache](https://github.com/actions/cache) from 5.0.3 to 5.0.4. - [Release notes](https://github.com/actions/cache/releases) - [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md) - [Commits](actions/cache@cdf6c1f...6682284) --- updated-dependencies: - dependency-name: actions/cache dependency-version: 5.0.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62543 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5.5.2 to 6.0.0. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@671740a...57e3a13) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: 6.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62545 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.32.4 to 4.35.1. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@89a39a4...c10b806) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.35.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62547 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 6.2.0 to 6.3.0. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](actions/setup-node@6044e13...53b8394) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: 6.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62548 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.15.0 to 2.16.1. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](step-security/harden-runner@a90bcbc...fe10465) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-version: 2.16.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62550 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Bumps the eslint group in /tools/eslint with 2 updates: [@eslint/markdown](https://github.com/eslint/markdown) and [eslint-plugin-jsdoc](https://github.com/gajus/eslint-plugin-jsdoc). Updates `@eslint/markdown` from 7.5.1 to 8.0.0 - [Release notes](https://github.com/eslint/markdown/releases) - [Changelog](https://github.com/eslint/markdown/blob/main/CHANGELOG.md) - [Commits](eslint/markdown@v7.5.1...v8.0.0) Updates `eslint-plugin-jsdoc` from 62.8.0 to 62.8.1 - [Release notes](https://github.com/gajus/eslint-plugin-jsdoc/releases) - [Commits](gajus/eslint-plugin-jsdoc@v62.8.0...v62.8.1) --- updated-dependencies: - dependency-name: "@eslint/markdown" dependency-version: 8.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: eslint - dependency-name: eslint-plugin-jsdoc dependency-version: 62.8.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: eslint ... Signed-off-by: dependabot[bot] <support@github.com> PR-URL: nodejs/node#62552 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs/node#62541 Fixes: nodejs/node#62481 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
When a socket with pipelined requests is destroyed then some requests will leak. PR-URL: nodejs/node#62534 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Add documentation for the `cancel` option of the `TransformStream` transformer, which allows users to specify a callback that will be called when the stream is canceled. See: https://streams.spec.whatwg.org/#transformer-api Fixes: nodejs/node#62540 PR-URL: nodejs/node#62566 Fixes: nodejs/node#62540 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: nabeel378 <mohammadnabeeljameel@gmail.com> PR-URL: nodejs/node#62553 Fixes: nodejs/node#62529 Refs: https://datatracker.ietf.org/doc/html/rfc9562#name-uuid-version-7 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Original commit message:
win: properly initialize OSVERSIONINFOW (#5107)
Otherwise calling `RtlGetVersion()` might produce UB. Problem was
causing random crashes in the node.js test suite with stack traces
like this one:
```
node.exe!__report_gsfailure(unsigned __int64 stack_cookie) Line 220
at D:\a\_work\1\s\src\vctools\crt\vcstartup\src\gs\gs_report.c(220)
...
```
Fixes: libuv/libuv#5106
Refs: libuv/libuv@aabb765
PR-URL: nodejs/node#62561
Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
PR-URL: nodejs/node#62460 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: nodejs/node#62828 Backport-PR-URL: nodejs/node#63173 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Fixes: nodejs/node#62899 Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: nodejs/node#62902 Backport-PR-URL: nodejs/node#63173 Fixes: nodejs/node#62899 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#61139 Backport-PR-URL: nodejs/node#63176 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#62145 Backport-PR-URL: nodejs/node#63176 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#62162 Backport-PR-URL: nodejs/node#63176 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com> PR-URL: nodejs/node#62616 Backport-PR-URL: nodejs/node#63176 Refs: nodejs/node#62145 Refs: nodejs/node#62162 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-Authored-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Signed-off-by: Antoine du Hamel <duhamelantoine1995@gmail.com> PR-URL: nodejs/node#62660 Backport-PR-URL: nodejs/node#63176 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Original commit message:
[maglev] Fix throwing node inside eager inlining
This commit refactors the exception handling logic to correctly identify
and associate nodes with their respective `catch` blocks, even
when multiple levels of inlining are involved.
Previously, the check `!IsInsideTryBlock() && !is_eager_inline()` was
insufficient to determine if catch block inside `CatchDetails` was
already created.
Specifically, consider the case where:
1. Function `bar` is non-eagerly inlined into `foo`.
2. `foo` contains a `catch` block.
3. `bar` calls `in_bar`, which is eagerly inlined.
4. A node within `in_bar` can `throw`.
In this scenario, `is_eager_inline` would be true when compiling
`in_bar`, leading to an incorrect assumption that the catch block didn't exist yet.
This change addresses the issue by propagating a boolean value via
`CatchDetails`. This boolean accurately indicates whether a `catch`
block is present in the call chain, allowing for correct exception
handling regardless of inlining depth or eagerness.
Fixed: 417768368
Change-Id: Ic52f72f302b4dc644bdcad939addf98111bc525b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6563500
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#100380}
Refs: v8/v8@657d8de
PR-URL: nodejs/node#62784
Reviewed-By: Xuguang Mei <meixuguang@gmail.com>
PR-URL: nodejs/node#61747 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: nodejs/node#62502 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Signed-off-by: Moshe Atlow <moshe@atlow.co.il> PR-URL: nodejs/node#62772 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: nodejs/node#63375 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Notable changes: crypto: * (SEMVER-MINOR) implement `randomUUIDv7()` (nabeel378) nodejs/node#62553 debugger: * (SEMVER-MINOR) add edit-free runtime expression probes to `node inspect` (Joyee Cheung) nodejs/node#62713 fs: * (SEMVER-MINOR) add `signal` option to `fs.stat()` (Mert Can Altin) nodejs/node#57775 * (SEMVER-MINOR) expose `frsize` field in `statfs` (Jinho Jang) nodejs/node#62277 http: * (SEMVER-MINOR) harden `ClientRequest` options merge (Matteo Collina) nodejs/node#63082 * (SEMVER-MINOR) add `req.signal` to `IncomingMessage` (Akshat) nodejs/node#62541 stream: * (SEMVER-MINOR) propagate destruction in `duplexPair` (Ahmed Elhor) nodejs/node#61098 test_runner: * (SEMVER-MINOR) support test order randomization (Pietro Marchini) nodejs/node#61747 * (SEMVER-MINOR) align mock timeout api (sangwook) nodejs/node#62820 * (SEMVER-MINOR) add mock-timers support for `AbortSignal.timeout` (DeveloperViraj) nodejs/node#60751 util: * (SEMVER-MINOR) colorize text with hex colors (Guilherme Araújo) nodejs/node#61556 PR-URL: nodejs/node#63263
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughRepository tooling, benchmark/docs, and transport-stack vendored dependencies are updated together. The PR also refreshes ngtcp2/nghttp3 APIs and implementations, adds QUIC-related workflow support, and bumps several embedded dependency versions. ChangesRepo tooling, workflows, and dependency metadata
Vendored QUIC and HTTP transport stack
Sequence Diagram(s)sequenceDiagram
participant ngtcp2_conn
participant ngtcp2_callbacks
participant ngtcp2_settings
participant ngtcp2_ratelim
participant ngtcp2_qpack
ngtcp2_conn->>ngtcp2_callbacks: convert latest callbacks/settings
ngtcp2_conn->>ngtcp2_settings: load versioned defaults
ngtcp2_conn->>ngtcp2_ratelim: initialize glitch limiter
ngtcp2_conn->>ngtcp2_qpack: seed encoder/decoder and set indexing strategy
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/commit-lint.yml:
- Around line 3-7: The branch glob for the GitHub Actions trigger under
on.pull_request.branches is using a regex-like pattern `v[0-9]+.x-staging` which
GitHub interprets as a glob, so replace that pattern with a proper glob (e.g.,
`v*.x-staging`) in the commit-lint workflow; update the branches array value
referenced as on.pull_request.branches to use the glob `v*.x-staging` so PRs to
versioned staging branches like v24.x-staging are matched.
In `@deps/llhttp/src/llhttp.c`:
- Around line 3488-3490: The transition in the state machine (case 10)
incorrectly routes an already-consumed '\n' into
s_n_llhttp__internal__n_req_http_complete_crlf via
s_n_llhttp__internal__n_invoke_test_lenient_flags_26; instead, ensure bare-LF
request lines are not sent through the CRLF-completion state: in the case 10
path (and the mirrored 8984-8990 region) detect that the '\n' was already
consumed and, unless the lenient_flags_8 "allow bare LF as CRLF" is set, do not
goto s_n_llhttp__internal__n_req_http_complete_crlf — instead handle it as the
"missing CR" path (or call the same missing-CR handler you use when a '\n'
arrives without a preceding '\r'), preserving existing behavior for
lenient_flags_1 only when appropriate; update the transition logic around
s_n_llhttp__internal__n_invoke_test_lenient_flags_26 to check lenient_flags_8
before routing into s_n_llhttp__internal__n_req_http_complete_crlf.
In `@deps/ngtcp2/nghttp3/lib/sfparse/sfparse.c`:
- Around line 724-727: The percent-decoding lookup table pct_tbl currently only
maps lowercase hex letters causing uppercase escapes (e.g. "%2F") to be
rejected; update the table construction used in pct_tbl (and the equivalent
tables around the other instances) so that HEXALPHAS includes uppercase hex
digits as well (or add the uppercase mapping alongside the existing HEXALPHAS
macro) so both 'a-f' and 'A-F' are accepted by the percent-decoder; ensure the
same change is applied to the other similar tables referenced in the diff (the
tables around the other occurrences).
In `@deps/ngtcp2/ngtcp2/crypto/ossl/ossl.c`:
- Around line 928-946: The current ngtcp2_crypto_set_local_transport_params
asserts and leaks/overwrites ossl_ctx->remote_params on a second non-empty call;
change it to allow replacement by removing the assert and freeing or
reallocating the previous buffer before assigning a new one. Specifically, in
ngtcp2_crypto_set_local_transport_params: if len != 0 and
ossl_ctx->remote_params is already set, free it (or use realloc) before
allocating the new buffer, handle malloc failure by leaving state consistent
(set remote_params to NULL on error), and ensure
SSL_set_quic_tls_transport_params(ossl_ctx->ssl, ossl_ctx->remote_params, len)
is still called with the correct pointer (use NULL when len == 0). This
preserves the documented ngtcp2_conn_set_local_transport_params() flow and
avoids leaks before ngtcp2_conn_install_tx_handshake_key().
- Around line 681-776: In ngtcp2_crypto_hkdf_extract, ngtcp2_crypto_hkdf_expand,
and ngtcp2_crypto_hkdf guard the allocations for EVP_KDF *kdf and EVP_KDF_CTX
*kctx after calling crypto_kdf_hkdf() and EVP_KDF_CTX_new(kdf): if kdf or kctx
is NULL, clean up any allocated resource (free kdf if allocated), set rv = -1
and return immediately instead of calling EVP_KDF_derive; ensure the same
NULL-check pattern is applied in each function around the existing
EVP_KDF_derive call so allocation failures or provider errors return -1 rather
than dereferencing NULL.
In `@deps/ngtcp2/ngtcp2/lib/includes/ngtcp2/ngtcp2.h`:
- Around line 5648-5687: The aggregate-packet API declares ngtcp2_write_pkt as
taking a void *user_data but ngtcp2_conn_write_aggregate_pkt_versioned (and
related overloads such as ngtcp2_conn_write_aggregate_pkt) do not accept or
forward any user_data, making the public API inconsistent; fix by adding a void
*user_data parameter to ngtcp2_conn_write_aggregate_pkt_versioned (and the other
aggregate helpers at lines ~6080-6084) and ensure the implementation forwards
that pointer into the ngtcp2_write_pkt calls and into the final
ngtcp2_conn_update_pkt_tx_time invocation so caller state passed to
ngtcp2_write_pkt is preserved.
In `@deps/ngtcp2/ngtcp2/lib/ngtcp2_conn.c`:
- Around line 13913-13920: The function currently returns a negative error from
write_pkt() even if prior iterations already wrote packets into buf; instead,
modify the loop handling around write_pkt (referencing write_pkt, nwrite, wbuf,
buf) so that if write_pkt() returns < 0 but some bytes have already been
produced (wbuf > buf), you convert that outcome into returning the number of
bytes produced (set nwrite = wbuf - buf and break from the loop) rather than
propagating the negative error; only propagate the negative error when no bytes
were produced. Apply the same fix to the analogous block around the second
occurrence mentioned (lines referencing nwrite and wbuf in the later iteration).
- Around line 5646-5669: The code is consuming glitch_rlim budget
(ngtcp2_ratelim_drain(&conn->glitch_rlim,...)) for idempotent/benign cases
(duplicate/older MAX_STREAM_DATA, CRYPTO/STREAM duplicates, repeated
RESET_STREAM/STOP_SENDING) before returning 0; remove those ngtcp2_ratelim_drain
calls from the early-return branches so duplicates are ignored without charging
the rate limiter and only use glitch_rlim for actual protocol errors.
Specifically, in the MAX_STREAM_DATA handling around
strm->tx.max_offset/fr->max_stream_data and the NGTCP2_STRM_FLAG_SHUT_WR branch
remove the ngtcp2_ratelim_drain+return sequence and simply return 0 for the
idempotent cases; apply the same change pattern to the other affected blocks
(the other listed ranges) and keep the conn_call_extend_max_stream_data(...) and
error paths unchanged.
- Around line 9898-9905: The condition in the migration check is comparing the
wrong endpoint: active migration should detect changes in the peer/remote
address, not the server's local socket. Update the predicate that currently
compares &conn->dcid.current.ps.path.local and &path->local to compare the
remote/peer addresses (e.g., &conn->dcid.current.ps.path.remote vs
&path->remote) and pass the remote address into
conn_server_preferred_addr_migration accordingly so the check disables migration
only when the peer/remote addr changed while disable_active_migration is set;
keep the existing ngtcp2_log_info message and return behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce602b49-0681-4142-b5b3-22a72d8bf6e3
⛔ Files ignored due to path filters (36)
deps/amaro/dist/index.jsis excluded by!**/dist/**deps/amaro/dist/package.jsonis excluded by!**/dist/**deps/corepack/dist/lib/corepack.cjsis excluded by!**/dist/**deps/icu-small/source/data/in/icudt78l.dat.bz2is excluded by!**/*.bz2deps/minimatch/dist/commonjs/ast.d.tsis excluded by!**/dist/**deps/minimatch/dist/commonjs/ast.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/ast.jsis excluded by!**/dist/**deps/minimatch/dist/commonjs/ast.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/escape.d.tsis excluded by!**/dist/**deps/minimatch/dist/commonjs/escape.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/escape.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/index.d.tsis excluded by!**/dist/**deps/minimatch/dist/commonjs/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/index.jsis excluded by!**/dist/**deps/minimatch/dist/commonjs/index.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/unescape.d.tsis excluded by!**/dist/**deps/minimatch/dist/commonjs/unescape.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/commonjs/unescape.jsis excluded by!**/dist/**deps/minimatch/dist/commonjs/unescape.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/ast.d.tsis excluded by!**/dist/**deps/minimatch/dist/esm/ast.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/ast.jsis excluded by!**/dist/**deps/minimatch/dist/esm/ast.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/escape.d.tsis excluded by!**/dist/**deps/minimatch/dist/esm/escape.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/escape.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/index.d.tsis excluded by!**/dist/**deps/minimatch/dist/esm/index.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/index.jsis excluded by!**/dist/**deps/minimatch/dist/esm/index.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/unescape.d.tsis excluded by!**/dist/**deps/minimatch/dist/esm/unescape.d.ts.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/dist/esm/unescape.jsis excluded by!**/dist/**deps/minimatch/dist/esm/unescape.js.mapis excluded by!**/dist/**,!**/*.mapdeps/minimatch/package-lock.jsonis excluded by!**/package-lock.jsondeps/nsolid-cli/node_modules/ini/LICENSEis excluded by!**/node_modules/**
📒 Files selected for processing (264)
.editorconfig.github/CODEOWNERS.github/actions/undici-wpt-current/action.yml.github/actions/undici-wpt-legacy/action.yml.github/dependabot.yml.github/workflows/auto-start-ci.yml.github/workflows/build-tarball.yml.github/workflows/close-stale-feature-requests.yml.github/workflows/close-stale-pull-requests.yml.github/workflows/codeql.yml.github/workflows/commit-lint.yml.github/workflows/commit-queue.yml.github/workflows/coverage-linux-without-intl.yml.github/workflows/coverage-linux.yml.github/workflows/coverage-windows.yml.github/workflows/create-release-proposal.yml.github/workflows/daily-wpt-fyi.yml.github/workflows/daily.yml.github/workflows/doc.yml.github/workflows/find-inactive-collaborators.yml.github/workflows/find-inactive-tsc.yml.github/workflows/license-builder.yml.github/workflows/lint-release-proposal.yml.github/workflows/notify-on-push.yml.github/workflows/post-release.yml.github/workflows/scorecard.yml.github/workflows/stale.yml.github/workflows/test-internet.yml.github/workflows/test-linux.yml.github/workflows/test-macos.yml.github/workflows/timezone-update.yml.github/workflows/tools.yml.github/workflows/update-openssl.yml.github/workflows/update-v8.yml.github/workflows/update-wpt.ymlCHANGELOG.mdbenchmark/misc/webidl-buffer-source.jsbenchmark/process/handled-rejections.jsbenchmark/url/url-searchparams-mutation.jsbenchmark/util/style-text.jsbenchmark/util/type-check.jsbenchmark/v8/startup-snapshot-callbacks.jscommon.gypiconfigure.pydeps/amaro/README.mddeps/amaro/package.jsondeps/corepack/CHANGELOG.mddeps/corepack/README.mddeps/corepack/package.jsondeps/googletest/include/gtest/gtest.hdeps/googletest/src/gtest-internal-inl.hdeps/googletest/src/gtest.ccdeps/icu-small/source/common/unicode/utfiterator.hdeps/icu-small/source/common/unicode/uvernum.hdeps/icu-small/source/i18n/nfrule.cppdeps/llhttp/CMakeLists.txtdeps/llhttp/LICENSEdeps/llhttp/README.mddeps/llhttp/cmake/llhttpConfig.cmake.indeps/llhttp/include/llhttp.hdeps/llhttp/src/api.cdeps/llhttp/src/llhttp.cdeps/minimatch/README.mddeps/minimatch/index.jsdeps/minimatch/package.jsondeps/nbytes/.release-please-manifest.jsondeps/nbytes/CHANGELOG.mddeps/nbytes/CMakeLists.txtdeps/nbytes/include/nbytes.hdeps/ncrypto/ncrypto.ccdeps/ngtcp2/README.mddeps/ngtcp2/nghttp3/lib/includes/nghttp3/nghttp3.hdeps/ngtcp2/nghttp3/lib/includes/nghttp3/version.hdeps/ngtcp2/nghttp3/lib/nghttp3_balloc.cdeps/ngtcp2/nghttp3/lib/nghttp3_balloc.hdeps/ngtcp2/nghttp3/lib/nghttp3_buf.cdeps/ngtcp2/nghttp3/lib/nghttp3_buf.hdeps/ngtcp2/nghttp3/lib/nghttp3_callbacks.cdeps/ngtcp2/nghttp3/lib/nghttp3_callbacks.hdeps/ngtcp2/nghttp3/lib/nghttp3_conn.cdeps/ngtcp2/nghttp3/lib/nghttp3_conn.hdeps/ngtcp2/nghttp3/lib/nghttp3_conv.cdeps/ngtcp2/nghttp3/lib/nghttp3_conv.hdeps/ngtcp2/nghttp3/lib/nghttp3_err.cdeps/ngtcp2/nghttp3/lib/nghttp3_frame.cdeps/ngtcp2/nghttp3/lib/nghttp3_frame.hdeps/ngtcp2/nghttp3/lib/nghttp3_gaptr.cdeps/ngtcp2/nghttp3/lib/nghttp3_gaptr.hdeps/ngtcp2/nghttp3/lib/nghttp3_http.cdeps/ngtcp2/nghttp3/lib/nghttp3_idtr.cdeps/ngtcp2/nghttp3/lib/nghttp3_idtr.hdeps/ngtcp2/nghttp3/lib/nghttp3_ksl.cdeps/ngtcp2/nghttp3/lib/nghttp3_ksl.hdeps/ngtcp2/nghttp3/lib/nghttp3_macro.hdeps/ngtcp2/nghttp3/lib/nghttp3_map.cdeps/ngtcp2/nghttp3/lib/nghttp3_map.hdeps/ngtcp2/nghttp3/lib/nghttp3_pq.cdeps/ngtcp2/nghttp3/lib/nghttp3_pq.hdeps/ngtcp2/nghttp3/lib/nghttp3_qpack.cdeps/ngtcp2/nghttp3/lib/nghttp3_qpack.hdeps/ngtcp2/nghttp3/lib/nghttp3_qpack_huffman.cdeps/ngtcp2/nghttp3/lib/nghttp3_qpack_huffman.hdeps/ngtcp2/nghttp3/lib/nghttp3_qpack_huffman_data.cdeps/ngtcp2/nghttp3/lib/nghttp3_range.cdeps/ngtcp2/nghttp3/lib/nghttp3_ratelim.cdeps/ngtcp2/nghttp3/lib/nghttp3_ratelim.hdeps/ngtcp2/nghttp3/lib/nghttp3_ringbuf.cdeps/ngtcp2/nghttp3/lib/nghttp3_ringbuf.hdeps/ngtcp2/nghttp3/lib/nghttp3_settings.cdeps/ngtcp2/nghttp3/lib/nghttp3_settings.hdeps/ngtcp2/nghttp3/lib/nghttp3_stream.cdeps/ngtcp2/nghttp3/lib/nghttp3_stream.hdeps/ngtcp2/nghttp3/lib/nghttp3_tnode.cdeps/ngtcp2/nghttp3/lib/nghttp3_tnode.hdeps/ngtcp2/nghttp3/lib/nghttp3_unreachable.cdeps/ngtcp2/nghttp3/lib/nghttp3_version.cdeps/ngtcp2/nghttp3/lib/sfparse/sfparse.cdeps/ngtcp2/nghttp3/lib/sfparse/sfparse.hdeps/ngtcp2/ngtcp2.gypdeps/ngtcp2/ngtcp2/crypto/includes/ngtcp2/ngtcp2_crypto.hdeps/ngtcp2/ngtcp2/crypto/includes/ngtcp2/ngtcp2_crypto_ossl.hdeps/ngtcp2/ngtcp2/crypto/includes/ngtcp2/ngtcp2_crypto_picotls.hdeps/ngtcp2/ngtcp2/crypto/ossl/ossl.cdeps/ngtcp2/ngtcp2/lib/includes/ngtcp2/ngtcp2.hdeps/ngtcp2/ngtcp2/lib/includes/ngtcp2/version.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_acktr.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_bbr.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_bbr.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_buf.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_buf.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_callbacks.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_callbacks.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_cc.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_cc.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_cid.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_cid.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_conn.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_conn.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_conn_stat.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_dcidtr.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_dcidtr.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_frame_chain.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_ksl.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_log.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_map.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_map.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_pcg.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_pcg.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_pkt.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_pkt.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_ppe.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_ppe.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_ratelim.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_ratelim.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_ringbuf.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_rob.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_rob.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_rst.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_rst.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_rtb.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_rtb.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_settings.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_settings.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_strm.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_strm.hdeps/ngtcp2/ngtcp2/lib/ngtcp2_transport_params.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_vec.cdeps/ngtcp2/ngtcp2/lib/ngtcp2_vec.hdeps/openssl/config/archs/BSD-x86/asm/configdata.pmdeps/openssl/config/archs/BSD-x86/asm/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86/asm/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86/asm/include/openssl/opensslv.hdeps/openssl/config/archs/BSD-x86/asm_avx2/configdata.pmdeps/openssl/config/archs/BSD-x86/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/BSD-x86/no-asm/configdata.pmdeps/openssl/config/archs/BSD-x86/no-asm/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/BSD-x86_64/asm/configdata.pmdeps/openssl/config/archs/BSD-x86_64/asm/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86_64/asm/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86_64/asm/include/openssl/opensslv.hdeps/openssl/config/archs/BSD-x86_64/asm_avx2/configdata.pmdeps/openssl/config/archs/BSD-x86_64/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86_64/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86_64/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/BSD-x86_64/no-asm/configdata.pmdeps/openssl/config/archs/BSD-x86_64/no-asm/crypto/buildinf.hdeps/openssl/config/archs/BSD-x86_64/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/BSD-x86_64/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN32/asm/configdata.pmdeps/openssl/config/archs/VC-WIN32/asm/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN32/asm/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN32/asm/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN32/asm_avx2/configdata.pmdeps/openssl/config/archs/VC-WIN32/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN32/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN32/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN32/no-asm/configdata.pmdeps/openssl/config/archs/VC-WIN32/no-asm/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN32/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN32/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN64-ARM/no-asm/configdata.pmdeps/openssl/config/archs/VC-WIN64-ARM/no-asm/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN64-ARM/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN64-ARM/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN64A/asm/configdata.pmdeps/openssl/config/archs/VC-WIN64A/asm/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN64A/asm/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN64A/asm/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN64A/asm_avx2/configdata.pmdeps/openssl/config/archs/VC-WIN64A/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN64A/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN64A/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/VC-WIN64A/no-asm/configdata.pmdeps/openssl/config/archs/VC-WIN64A/no-asm/crypto/buildinf.hdeps/openssl/config/archs/VC-WIN64A/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/VC-WIN64A/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/aix64-gcc-as/asm/configdata.pmdeps/openssl/config/archs/aix64-gcc-as/asm/crypto/buildinf.hdeps/openssl/config/archs/aix64-gcc-as/asm/include/openssl/cmp.hdeps/openssl/config/archs/aix64-gcc-as/asm/include/openssl/opensslv.hdeps/openssl/config/archs/aix64-gcc-as/asm_avx2/configdata.pmdeps/openssl/config/archs/aix64-gcc-as/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/aix64-gcc-as/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/aix64-gcc-as/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/aix64-gcc-as/no-asm/configdata.pmdeps/openssl/config/archs/aix64-gcc-as/no-asm/crypto/buildinf.hdeps/openssl/config/archs/aix64-gcc-as/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/aix64-gcc-as/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin-i386-cc/asm/configdata.pmdeps/openssl/config/archs/darwin-i386-cc/asm/crypto/buildinf.hdeps/openssl/config/archs/darwin-i386-cc/asm/include/openssl/cmp.hdeps/openssl/config/archs/darwin-i386-cc/asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin-i386-cc/asm_avx2/configdata.pmdeps/openssl/config/archs/darwin-i386-cc/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/darwin-i386-cc/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/darwin-i386-cc/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/darwin-i386-cc/no-asm/configdata.pmdeps/openssl/config/archs/darwin-i386-cc/no-asm/crypto/buildinf.hdeps/openssl/config/archs/darwin-i386-cc/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/darwin-i386-cc/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin64-arm64-cc/asm/configdata.pmdeps/openssl/config/archs/darwin64-arm64-cc/asm/crypto/buildinf.hdeps/openssl/config/archs/darwin64-arm64-cc/asm/crypto/sm4/vpsm4_ex-armv8.Sdeps/openssl/config/archs/darwin64-arm64-cc/asm/include/openssl/cmp.hdeps/openssl/config/archs/darwin64-arm64-cc/asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/configdata.pmdeps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/crypto/buildinf.hdeps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/crypto/sm4/vpsm4_ex-armv8.Sdeps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/include/openssl/cmp.hdeps/openssl/config/archs/darwin64-arm64-cc/asm_avx2/include/openssl/opensslv.hdeps/openssl/config/archs/darwin64-arm64-cc/no-asm/configdata.pmdeps/openssl/config/archs/darwin64-arm64-cc/no-asm/crypto/buildinf.hdeps/openssl/config/archs/darwin64-arm64-cc/no-asm/include/openssl/cmp.hdeps/openssl/config/archs/darwin64-arm64-cc/no-asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin64-x86_64-cc/asm/configdata.pmdeps/openssl/config/archs/darwin64-x86_64-cc/asm/crypto/buildinf.hdeps/openssl/config/archs/darwin64-x86_64-cc/asm/include/openssl/cmp.hdeps/openssl/config/archs/darwin64-x86_64-cc/asm/include/openssl/opensslv.hdeps/openssl/config/archs/darwin64-x86_64-cc/asm_avx2/configdata.pmdeps/openssl/config/archs/darwin64-x86_64-cc/asm_avx2/crypto/buildinf.h
💤 Files with no reviewable changes (7)
- deps/ngtcp2/nghttp3/lib/nghttp3_tnode.h
- .github/workflows/close-stale-feature-requests.yml
- deps/ngtcp2/ngtcp2/lib/ngtcp2_frame_chain.h
- .github/workflows/close-stale-pull-requests.yml
- deps/ngtcp2/nghttp3/lib/nghttp3_pq.c
- deps/ngtcp2/nghttp3/lib/nghttp3_pq.h
- deps/amaro/README.md
Comment on lines
+3
to
+7
| on: | ||
| pull_request: | ||
| branches: | ||
| - main | ||
| - v[0-9]+.x-staging |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In GitHub Actions on.pull_request.branchesmatching, doesv[0-9]+.x-stagingtreat+as a regex quantifier or a literal character? Would it matchv24.x-staging?
💡 Result:
In GitHub Actions, the on.pull_request.branches filter uses glob patterns rather than regular expressions [1][2]. The + character is treated as a literal character in this context unless it is intended to function as part of a glob pattern (where it is specifically listed as a special character that may require escaping for literal matches) [1]. Because GitHub Actions uses glob matching, your pattern v[0-9]+.x-staging will not function as a regex that matches "one or more digits". Instead, it will be evaluated as a glob pattern that literally looks for the characters [, 0, -, 9, ], +, ., x, -, s, t, a, g, i, n, g [1]. It would not match v24.x-staging. If you need to match branch names using regex, you should not use the branches filter directly [3][4]. Instead, you can: 1. Use a standard on.pull_request trigger without branch filters (or with broad filters). 2. Add a job with an if conditional that uses the github.ref_name or github.event.pull_request.head.ref context along with a regex evaluation [3][5]. Since GitHub Actions expressions do not have a built-in matches or regex function for strings, you can use the contains function for simple logic [5] or use a dedicated third-party action that supports regex matching to perform the validation [6][7].
Citations:
- 1: https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions
- 2: https://support.huaweicloud.com/intl/en-us/usermanual-pipeline/pipeline_01_0012.html
- 3: https://stackoverflow.com/questions/58862864/github-actions-ci-conditional-regex
- 4: https://stackoverflow.com/questions/68078004/why-doesnt-my-github-action-trigger-on-a-regex-tag
- 5: https://stackoverflow.com/questions/58139406/only-run-job-on-specific-branch-with-github-actions
- 6: https://github.com/IamPekka058/branchMatchRegex
- 7: https://github.com/marketplace/actions/conditional-regex-search-and-replace
Fix PR branch glob pattern in .github/workflows/commit-lint.yml
GitHub Actions on.pull_request.branches uses glob patterns, not regex. The value v[0-9]+.x-staging won’t be interpreted as “one or more digits” (so it won’t match v24.x-staging), which can cause PRs to the staging branches to be missed. Use a glob like v*.x-staging instead.
Suggested fix
branches:
- main
- - v[0-9]+.x-staging
+ - v*.x-staging📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| on: | |
| pull_request: | |
| branches: | |
| - main | |
| - v[0-9]+.x-staging | |
| on: | |
| pull_request: | |
| branches: | |
| - main | |
| - v*.x-staging |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/commit-lint.yml around lines 3 - 7, The branch glob for
the GitHub Actions trigger under on.pull_request.branches is using a regex-like
pattern `v[0-9]+.x-staging` which GitHub interprets as a glob, so replace that
pattern with a proper glob (e.g., `v*.x-staging`) in the commit-lint workflow;
update the branches array value referenced as on.pull_request.branches to use
the glob `v*.x-staging` so PRs to versioned staging branches like v24.x-staging
are matched.
Comment on lines
3488
to
+3490
| case 10: { | ||
| p++; | ||
| goto s_n_llhttp__internal__n_invoke_test_lenient_flags_25; | ||
| goto s_n_llhttp__internal__n_invoke_test_lenient_flags_26; |
There was a problem hiding this comment.
Don't route bare-LF request lines through the CRLF state.
Line 8987 sends the already-consumed \n path into s_n_llhttp__internal__n_req_http_complete_crlf. That state is for the byte after \r, so the next header byte falls into the "missing LF after CR" path unless lenient_flags_8 is also set. In practice, lenient_flags_1 alone no longer permits bare-LF request lines.
💡 Suggested fix
s_n_llhttp__internal__n_invoke_test_lenient_flags_26: {
switch (llhttp__internal__c_test_lenient_flags_1(state, p, endp)) {
case 1:
- goto s_n_llhttp__internal__n_req_http_complete_crlf;
+ goto s_n_llhttp__internal__n_headers_start;
default:
goto s_n_llhttp__internal__n_error_69;
}
UNREACHABLE;
}Also applies to: 8984-8990
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/llhttp/src/llhttp.c` around lines 3488 - 3490, The transition in the
state machine (case 10) incorrectly routes an already-consumed '\n' into
s_n_llhttp__internal__n_req_http_complete_crlf via
s_n_llhttp__internal__n_invoke_test_lenient_flags_26; instead, ensure bare-LF
request lines are not sent through the CRLF-completion state: in the case 10
path (and the mirrored 8984-8990 region) detect that the '\n' was already
consumed and, unless the lenient_flags_8 "allow bare LF as CRLF" is set, do not
goto s_n_llhttp__internal__n_req_http_complete_crlf — instead handle it as the
"missing CR" path (or call the same missing-CR handler you use when a '\n'
arrives without a preceding '\r'), preserving existing behavior for
lenient_flags_1 only when appropriate; update the transition logic around
s_n_llhttp__internal__n_invoke_test_lenient_flags_26 to check lenient_flags_8
before routing into s_n_llhttp__internal__n_req_http_complete_crlf.
Comment on lines
+724
to
+727
| static const uint8_t pct_tbl[256] = { | ||
| DIGITS(1), | ||
| HEXALPHAS(2), | ||
| }; |
There was a problem hiding this comment.
Accept uppercase hex digits in percent-decoding.
The lookup/conversion path only recognizes a-f, so uppercase escapes (e.g. %2F) are rejected in display-string decoding.
🔧 Proposed fix
-#define HEXALPHAS(N) \
- ['a'] = (N), ['b'] = (N), ['c'] = (N), ['d'] = (N), ['e'] = (N), ['f'] = (N)
+#define HEXALPHAS(N) \
+ ['A'] = (N), ['B'] = (N), ['C'] = (N), ['D'] = (N), ['E'] = (N), ['F'] = (N), \
+ ['a'] = (N), ['b'] = (N), ['c'] = (N), ['d'] = (N), ['e'] = (N), ['f'] = (N)
@@
- c = (uint8_t)((b - 'a' + 10) << 4);
+ c = (uint8_t)((((b | 0x20) - 'a') + 10) << 4);
@@
- c |= (uint8_t)(b - 'a' + 10);
+ c |= (uint8_t)(((b | 0x20) - 'a') + 10);Also applies to: 732-740, 750-758
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/nghttp3/lib/sfparse/sfparse.c` around lines 724 - 727, The
percent-decoding lookup table pct_tbl currently only maps lowercase hex letters
causing uppercase escapes (e.g. "%2F") to be rejected; update the table
construction used in pct_tbl (and the equivalent tables around the other
instances) so that HEXALPHAS includes uppercase hex digits as well (or add the
uppercase mapping alongside the existing HEXALPHAS macro) so both 'a-f' and
'A-F' are accepted by the percent-decoder; ensure the same change is applied to
the other similar tables referenced in the diff (the tables around the other
occurrences).
Comment on lines
+681
to
+776
| int ngtcp2_crypto_hkdf_extract(uint8_t *dest, const ngtcp2_crypto_md *md, | ||
| const uint8_t *secret, size_t secretlen, | ||
| const uint8_t *salt, size_t saltlen) { | ||
| const EVP_MD *prf = md->native_handle; | ||
| EVP_KDF *kdf = crypto_kdf_hkdf(); | ||
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | ||
| int mode = EVP_KDF_HKDF_MODE_EXTRACT_ONLY; | ||
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | ||
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | ||
| (char *)EVP_MD_get0_name(prf), 0), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | ||
| secretlen), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | ||
| saltlen), | ||
| OSSL_PARAM_construct_end(), | ||
| }; | ||
| int rv = 0; | ||
|
|
||
| if (!crypto_initialized) { | ||
| EVP_KDF_free(kdf); | ||
| } | ||
|
|
||
| if (EVP_KDF_derive(kctx, dest, (size_t)EVP_MD_size(prf), params) <= 0) { | ||
| rv = -1; | ||
| } | ||
|
|
||
| EVP_KDF_CTX_free(kctx); | ||
|
|
||
| return rv; | ||
| } | ||
|
|
||
| int ngtcp2_crypto_hkdf_expand(uint8_t *dest, size_t destlen, | ||
| const ngtcp2_crypto_md *md, const uint8_t *secret, | ||
| size_t secretlen, const uint8_t *info, | ||
| size_t infolen) { | ||
| const EVP_MD *prf = md->native_handle; | ||
| EVP_KDF *kdf = crypto_kdf_hkdf(); | ||
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | ||
| int mode = EVP_KDF_HKDF_MODE_EXPAND_ONLY; | ||
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | ||
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | ||
| (char *)EVP_MD_get0_name(prf), 0), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | ||
| secretlen), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | ||
| infolen), | ||
| OSSL_PARAM_construct_end(), | ||
| }; | ||
| int rv = 0; | ||
|
|
||
| if (!crypto_initialized) { | ||
| EVP_KDF_free(kdf); | ||
| } | ||
|
|
||
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | ||
| rv = -1; | ||
| } | ||
|
|
||
| EVP_KDF_CTX_free(kctx); | ||
|
|
||
| return rv; | ||
| } | ||
|
|
||
| int ngtcp2_crypto_hkdf(uint8_t *dest, size_t destlen, | ||
| const ngtcp2_crypto_md *md, const uint8_t *secret, | ||
| size_t secretlen, const uint8_t *salt, size_t saltlen, | ||
| const uint8_t *info, size_t infolen) { | ||
| const EVP_MD *prf = md->native_handle; | ||
| EVP_KDF *kdf = crypto_kdf_hkdf(); | ||
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | ||
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | ||
| (char *)EVP_MD_get0_name(prf), 0), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | ||
| secretlen), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | ||
| saltlen), | ||
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | ||
| infolen), | ||
| OSSL_PARAM_construct_end(), | ||
| }; | ||
| int rv = 0; | ||
|
|
||
| if (!crypto_initialized) { | ||
| EVP_KDF_free(kdf); | ||
| } | ||
|
|
||
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | ||
| rv = -1; | ||
| } | ||
|
|
||
| EVP_KDF_CTX_free(kctx); | ||
|
|
||
| return rv; |
There was a problem hiding this comment.
Guard the HKDF fetch/context allocations before EVP_KDF_derive.
Line 685 / Line 718 / Line 751 can return a NULL EVP_KDF *, and Line 686 / Line 719 / Line 752 can return a NULL EVP_KDF_CTX *. All three helpers dereference those values unconditionally, so an OpenSSL provider failure or OOM turns handshake setup into a crash instead of -1.
Suggested fix pattern
EVP_KDF *kdf = crypto_kdf_hkdf();
- EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf);
+ EVP_KDF_CTX *kctx;
+ if (kdf == NULL) {
+ return -1;
+ }
+ kctx = EVP_KDF_CTX_new(kdf);
+ if (kctx == NULL) {
+ if (!crypto_initialized) {
+ EVP_KDF_free(kdf);
+ }
+ return -1;
+ }Apply the same guard in ngtcp2_crypto_hkdf_extract, ngtcp2_crypto_hkdf_expand, and ngtcp2_crypto_hkdf.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| int ngtcp2_crypto_hkdf_extract(uint8_t *dest, const ngtcp2_crypto_md *md, | |
| const uint8_t *secret, size_t secretlen, | |
| const uint8_t *salt, size_t saltlen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | |
| int mode = EVP_KDF_HKDF_MODE_EXTRACT_ONLY; | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | |
| saltlen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| if (EVP_KDF_derive(kctx, dest, (size_t)EVP_MD_size(prf), params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| } | |
| int ngtcp2_crypto_hkdf_expand(uint8_t *dest, size_t destlen, | |
| const ngtcp2_crypto_md *md, const uint8_t *secret, | |
| size_t secretlen, const uint8_t *info, | |
| size_t infolen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | |
| int mode = EVP_KDF_HKDF_MODE_EXPAND_ONLY; | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | |
| infolen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| } | |
| int ngtcp2_crypto_hkdf(uint8_t *dest, size_t destlen, | |
| const ngtcp2_crypto_md *md, const uint8_t *secret, | |
| size_t secretlen, const uint8_t *salt, size_t saltlen, | |
| const uint8_t *info, size_t infolen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx = EVP_KDF_CTX_new(kdf); | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | |
| saltlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | |
| infolen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| int ngtcp2_crypto_hkdf_extract(uint8_t *dest, const ngtcp2_crypto_md *md, | |
| const uint8_t *secret, size_t secretlen, | |
| const uint8_t *salt, size_t saltlen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx; | |
| int mode = EVP_KDF_HKDF_MODE_EXTRACT_ONLY; | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | |
| saltlen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (kdf == NULL) { | |
| return -1; | |
| } | |
| kctx = EVP_KDF_CTX_new(kdf); | |
| if (kctx == NULL) { | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| return -1; | |
| } | |
| if (EVP_KDF_derive(kctx, dest, (size_t)EVP_MD_size(prf), params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| } | |
| int ngtcp2_crypto_hkdf_expand(uint8_t *dest, size_t destlen, | |
| const ngtcp2_crypto_md *md, const uint8_t *secret, | |
| size_t secretlen, const uint8_t *info, | |
| size_t infolen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx; | |
| int mode = EVP_KDF_HKDF_MODE_EXPAND_ONLY; | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_int(OSSL_KDF_PARAM_MODE, &mode), | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | |
| infolen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (kdf == NULL) { | |
| return -1; | |
| } | |
| kctx = EVP_KDF_CTX_new(kdf); | |
| if (kctx == NULL) { | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| return -1; | |
| } | |
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| } | |
| int ngtcp2_crypto_hkdf(uint8_t *dest, size_t destlen, | |
| const ngtcp2_crypto_md *md, const uint8_t *secret, | |
| size_t secretlen, const uint8_t *salt, size_t saltlen, | |
| const uint8_t *info, size_t infolen) { | |
| const EVP_MD *prf = md->native_handle; | |
| EVP_KDF *kdf = crypto_kdf_hkdf(); | |
| EVP_KDF_CTX *kctx; | |
| OSSL_PARAM params[] = { | |
| OSSL_PARAM_construct_utf8_string(OSSL_KDF_PARAM_DIGEST, | |
| (char *)EVP_MD_get0_name(prf), 0), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_KEY, (void *)secret, | |
| secretlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_SALT, (void *)salt, | |
| saltlen), | |
| OSSL_PARAM_construct_octet_string(OSSL_KDF_PARAM_INFO, (void *)info, | |
| infolen), | |
| OSSL_PARAM_construct_end(), | |
| }; | |
| int rv = 0; | |
| if (kdf == NULL) { | |
| return -1; | |
| } | |
| kctx = EVP_KDF_CTX_new(kdf); | |
| if (kctx == NULL) { | |
| if (!crypto_initialized) { | |
| EVP_KDF_free(kdf); | |
| } | |
| return -1; | |
| } | |
| if (EVP_KDF_derive(kctx, dest, destlen, params) <= 0) { | |
| rv = -1; | |
| } | |
| EVP_KDF_CTX_free(kctx); | |
| return rv; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/crypto/ossl/ossl.c` around lines 681 - 776, In
ngtcp2_crypto_hkdf_extract, ngtcp2_crypto_hkdf_expand, and ngtcp2_crypto_hkdf
guard the allocations for EVP_KDF *kdf and EVP_KDF_CTX *kctx after calling
crypto_kdf_hkdf() and EVP_KDF_CTX_new(kdf): if kdf or kctx is NULL, clean up any
allocated resource (free kdf if allocated), set rv = -1 and return immediately
instead of calling EVP_KDF_derive; ensure the same NULL-check pattern is applied
in each function around the existing EVP_KDF_derive call so allocation failures
or provider errors return -1 rather than dereferencing NULL.
Comment on lines
+928
to
+946
| int ngtcp2_crypto_set_local_transport_params(void *tls, const uint8_t *buf, | ||
| size_t len) { | ||
| ngtcp2_crypto_ossl_ctx *ossl_ctx = tls; | ||
|
|
||
| if (len) { | ||
| assert(!ossl_ctx->remote_params); | ||
|
|
||
| ossl_ctx->remote_params = malloc(len); | ||
| if (!ossl_ctx->remote_params) { | ||
| return -1; | ||
| } | ||
|
|
||
| memcpy(ossl_ctx->remote_params, buf, len); | ||
| } | ||
|
|
||
| if (SSL_set_quic_tls_transport_params(ossl_ctx->ssl, ossl_ctx->remote_params, | ||
| len) != 1) { | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
Allow local transport parameters to be replaced before handshake keys are installed.
Line 933 hard-fails on a second non-empty call, and in release builds Lines 935-940 overwrite the old buffer without freeing it. That conflicts with the documented ngtcp2_conn_set_local_transport_params() flow where the server may update transport parameters after ALPN selection but before ngtcp2_conn_install_tx_handshake_key().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/crypto/ossl/ossl.c` around lines 928 - 946, The current
ngtcp2_crypto_set_local_transport_params asserts and leaks/overwrites
ossl_ctx->remote_params on a second non-empty call; change it to allow
replacement by removing the assert and freeing or reallocating the previous
buffer before assigning a new one. Specifically, in
ngtcp2_crypto_set_local_transport_params: if len != 0 and
ossl_ctx->remote_params is already set, free it (or use realloc) before
allocating the new buffer, handle malloc failure by leaving state consistent
(set remote_params to NULL on error), and ensure
SSL_set_quic_tls_transport_params(ossl_ctx->ssl, ossl_ctx->remote_params, len)
is still called with the correct pointer (use NULL when len == 0). This
preserves the documented ngtcp2_conn_set_local_transport_params() flow and
avoids leaks before ngtcp2_conn_install_tx_handshake_key().
Comment on lines
+5648
to
+5687
| typedef ngtcp2_ssize (*ngtcp2_write_pkt)(ngtcp2_conn *conn, ngtcp2_path *path, | ||
| ngtcp2_pkt_info *pi, uint8_t *dest, | ||
| size_t destlen, ngtcp2_tstamp ts, | ||
| void *user_data); | ||
|
|
||
| /** | ||
| * @function | ||
| * | ||
| * `ngtcp2_conn_write_aggregate_pkt` is a helper function to write | ||
| * multiple packets in the provided buffer, which is suitable to be | ||
| * sent at once in GSO. This function returns the number of bytes | ||
| * written to the buffer pointed by |buf| of length |buflen|. | ||
| * |buflen| must be at least | ||
| * `ngtcp2_conn_get_path_max_tx_udp_payload_size(conn) | ||
| * <ngtcp2_conn_get_path_max_tx_udp_payload_size>` bytes long. It is | ||
| * recommended to pass the buffer at least | ||
| * `ngtcp2_conn_get_max_tx_udp_payload_size(conn) | ||
| * <ngtcp2_conn_get_max_tx_udp_payload_size>` bytes in order to send a | ||
| * PMTUD packet. This function only writes multiple packets if the | ||
| * first packet is `ngtcp2_conn_get_path_max_tx_udp_payload_size(conn) | ||
| * <ngtcp2_conn_get_path_max_tx_udp_payload_size>` bytes long. The | ||
| * application can adjust the length of the buffer to limit the number | ||
| * of packets to aggregate. If this function returns positive | ||
| * integer, all packets share the same :type:`ngtcp2_path` and | ||
| * :type:`ngtcp2_pkt_info` values, and they are assigned to the | ||
| * objects pointed by |path| and |pi| respectively. The length of all | ||
| * packets other than the last packet is assigned to |*pgsolen|. The | ||
| * length of last packet is equal to or less than |*pgsolen|. | ||
| * |write_pkt| must write a single packet. After all packets are | ||
| * written, this function calls `ngtcp2_conn_update_pkt_tx_time`. | ||
| * | ||
| * This function returns the number of bytes written to the buffer, or | ||
| * a negative error code returned by |write_pkt|. | ||
| * | ||
| * This function has been available since v1.15.0. | ||
| */ | ||
| NGTCP2_EXTERN ngtcp2_ssize ngtcp2_conn_write_aggregate_pkt_versioned( | ||
| ngtcp2_conn *conn, ngtcp2_path *path, int pkt_info_version, | ||
| ngtcp2_pkt_info *pi, uint8_t *buf, size_t buflen, size_t *pgsolen, | ||
| ngtcp2_write_pkt write_pkt, ngtcp2_tstamp ts); |
There was a problem hiding this comment.
The aggregate-packet helper exposes no way to pass ngtcp2_write_pkt context.
Lines 5648-5651 require write_pkt(..., void *user_data), but Lines 5684-5687 and 6080-6084 do not accept any matching argument. As written, the new helper cannot forward caller state into the callback, so this public API is inconsistent on first release.
Suggested API alignment
NGTCP2_EXTERN ngtcp2_ssize ngtcp2_conn_write_aggregate_pkt_versioned(
ngtcp2_conn *conn, ngtcp2_path *path, int pkt_info_version,
ngtcp2_pkt_info *pi, uint8_t *buf, size_t buflen, size_t *pgsolen,
- ngtcp2_write_pkt write_pkt, ngtcp2_tstamp ts);
+ ngtcp2_write_pkt write_pkt, ngtcp2_tstamp ts, void *user_data); `#define` ngtcp2_conn_write_aggregate_pkt(CONN, PATH, PI, BUF, BUFLEN, PGSOLEN, \
- WRITE_PKT, TS) \
+ WRITE_PKT, TS, USER_DATA) \
ngtcp2_conn_write_aggregate_pkt_versioned( \
(CONN), (PATH), NGTCP2_PKT_INFO_VERSION, (PI), (BUF), (BUFLEN), (PGSOLEN), \
- (WRITE_PKT), (TS))
+ (WRITE_PKT), (TS), (USER_DATA))Also applies to: 6080-6084
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/lib/includes/ngtcp2/ngtcp2.h` around lines 5648 - 5687,
The aggregate-packet API declares ngtcp2_write_pkt as taking a void *user_data
but ngtcp2_conn_write_aggregate_pkt_versioned (and related overloads such as
ngtcp2_conn_write_aggregate_pkt) do not accept or forward any user_data, making
the public API inconsistent; fix by adding a void *user_data parameter to
ngtcp2_conn_write_aggregate_pkt_versioned (and the other aggregate helpers at
lines ~6080-6084) and ensure the implementation forwards that pointer into the
ngtcp2_write_pkt calls and into the final ngtcp2_conn_update_pkt_tx_time
invocation so caller state passed to ngtcp2_write_pkt is preserved.
Comment on lines
+5646
to
5669
| if (strm->tx.max_offset >= fr->max_stream_data) { | ||
| if (ngtcp2_ratelim_drain(&conn->glitch_rlim, 1, ts) != 0) { | ||
| return NGTCP2_ERR_INTERNAL; | ||
| } | ||
|
|
||
| rv = conn_call_extend_max_stream_data(conn, strm, fr->stream_id, | ||
| fr->max_stream_data); | ||
| if (rv != 0) { | ||
| return rv; | ||
| return 0; | ||
| } | ||
|
|
||
| strm->tx.max_offset = fr->max_stream_data; | ||
|
|
||
| /* Don't call callback if stream is half-closed local */ | ||
| if (strm->flags & NGTCP2_STRM_FLAG_SHUT_WR) { | ||
| if (ngtcp2_ratelim_drain(&conn->glitch_rlim, 1, ts) != 0) { | ||
| return NGTCP2_ERR_INTERNAL; | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| rv = conn_call_extend_max_stream_data(conn, strm, fr->stream_id, | ||
| fr->max_stream_data); | ||
| if (rv != 0) { | ||
| return rv; | ||
| } |
There was a problem hiding this comment.
Don't spend glitch_rlim budget on valid retransmissions.
These branches are all reachable through normal QUIC loss/reordering: older MAX_STREAM_DATA, duplicate CRYPTO/STREAM, and repeated RESET_STREAM/STOP_SENDING are idempotent and should be ignored. Charging them against glitch_rlim means ordinary PTO/retransmit traffic can eventually surface as NGTCP2_ERR_INTERNAL and tear down a healthy connection.
Also applies to: 7154-7160, 7171-7195, 7240-7245, 7432-7446, 7491-7500, 7690-7695, 7822-7828
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/lib/ngtcp2_conn.c` around lines 5646 - 5669, The code is
consuming glitch_rlim budget (ngtcp2_ratelim_drain(&conn->glitch_rlim,...)) for
idempotent/benign cases (duplicate/older MAX_STREAM_DATA, CRYPTO/STREAM
duplicates, repeated RESET_STREAM/STOP_SENDING) before returning 0; remove those
ngtcp2_ratelim_drain calls from the early-return branches so duplicates are
ignored without charging the rate limiter and only use glitch_rlim for actual
protocol errors. Specifically, in the MAX_STREAM_DATA handling around
strm->tx.max_offset/fr->max_stream_data and the NGTCP2_STRM_FLAG_SHUT_WR branch
remove the ngtcp2_ratelim_drain+return sequence and simply return 0 for the
idempotent cases; apply the same change pattern to the other affected blocks
(the other listed ranges) and keep the conn_call_extend_max_stream_data(...) and
error paths unchanged.
Comment on lines
+9898
to
+9905
| if (conn->server && conn->local.transport_params.disable_active_migration && | ||
| !ngtcp2_addr_eq(&conn->dcid.current.ps.path.local, &path->local) && | ||
| !conn_server_preferred_addr_migration(conn, &path->local)) { | ||
| ngtcp2_log_info(&conn->log, NGTCP2_LOG_EVENT_PKT, | ||
| "packet is discarded because active migration is disabled"); | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
disable_active_migration is checking the wrong side of the path.
This predicate compares path->local, but active migration is about the peer changing its address. As written, a client can still migrate while keeping the same server socket, and benign server-local address changes get dropped instead.
[suggested fix]
Patch sketch
- if (conn->server && conn->local.transport_params.disable_active_migration &&
- !ngtcp2_addr_eq(&conn->dcid.current.ps.path.local, &path->local) &&
- !conn_server_preferred_addr_migration(conn, &path->local)) {
+ if (conn->server && conn->local.transport_params.disable_active_migration &&
+ !ngtcp2_addr_eq(&conn->dcid.current.ps.path.remote, &path->remote)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (conn->server && conn->local.transport_params.disable_active_migration && | |
| !ngtcp2_addr_eq(&conn->dcid.current.ps.path.local, &path->local) && | |
| !conn_server_preferred_addr_migration(conn, &path->local)) { | |
| ngtcp2_log_info(&conn->log, NGTCP2_LOG_EVENT_PKT, | |
| "packet is discarded because active migration is disabled"); | |
| return 0; | |
| } | |
| if (conn->server && conn->local.transport_params.disable_active_migration && | |
| !ngtcp2_addr_eq(&conn->dcid.current.ps.path.remote, &path->remote)) { | |
| ngtcp2_log_info(&conn->log, NGTCP2_LOG_EVENT_PKT, | |
| "packet is discarded because active migration is disabled"); | |
| return 0; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/lib/ngtcp2_conn.c` around lines 9898 - 9905, The condition
in the migration check is comparing the wrong endpoint: active migration should
detect changes in the peer/remote address, not the server's local socket. Update
the predicate that currently compares &conn->dcid.current.ps.path.local and
&path->local to compare the remote/peer addresses (e.g.,
&conn->dcid.current.ps.path.remote vs &path->remote) and pass the remote address
into conn_server_preferred_addr_migration accordingly so the check disables
migration only when the peer/remote addr changed while disable_active_migration
is set; keep the existing ngtcp2_log_info message and return behavior.
Comment on lines
+13913
to
+13920
| nwrite = write_pkt(conn, path, pi, wbuf, wbuflen, ts, conn->user_data); | ||
| if (nwrite < 0) { | ||
| break; | ||
| } | ||
|
|
||
| if (nwrite == 0) { | ||
| nwrite = wbuf - buf; | ||
| break; |
There was a problem hiding this comment.
Don't return a negative error after earlier packets were already produced.
After the first successful write_pkt() call, packet numbers, retransmission state, and frame queues have already advanced. If a later iteration fails and this function returns that negative value, the caller will treat the whole aggregate write as failed even though buf already contains committed packets. That can desynchronize sender state from what the caller actually sends or discards.
Also applies to: 13963-13967
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/ngtcp2/ngtcp2/lib/ngtcp2_conn.c` around lines 13913 - 13920, The
function currently returns a negative error from write_pkt() even if prior
iterations already wrote packets into buf; instead, modify the loop handling
around write_pkt (referencing write_pkt, nwrite, wbuf, buf) so that if
write_pkt() returns < 0 but some bytes have already been produced (wbuf > buf),
you convert that outcome into returning the number of bytes produced (set nwrite
= wbuf - buf and break from the loop) rather than propagating the negative
error; only propagate the negative error when no bytes were produced. Apply the
same fix to the analogous block around the second occurrence mentioned (lines
referencing nwrite and wbuf in the later iteration).
EHortua
previously approved these changes
Jun 2, 2026
santigimeno
force-pushed
the
node-v24.16.0-nsolid-v6.3.0-release
branch
from
110a5e1 to
bc0a99c
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
santigimeno
force-pushed
the
node-v24.16.0-nsolid-v6.3.0-release
branch
from
bc0a99c to
8e90ca3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SECURITY.md (1)
20-33:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClean unresolved merge conflicts in security policy.
Line 20 through Line 33, Line 48 through Line 53, and Line 438 contain conflict markers. This makes the vulnerability reporting policy ambiguous and error-prone for reporters.
Also applies to: 48-53, 438-438
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SECURITY.md` around lines 20 - 33, The file SECURITY.md contains unresolved git merge conflict markers (e.g., <<<<<<< HEAD, =======, >>>>>>> v24.16.0) around the "Node.js bug bounty program" and "Reporting a bug in a third-party module" sections; clean these by choosing and merging the correct text: remove all conflict markers and duplicate blocks, consolidate the intended paragraphs under the "## Reporting a bug in a third‑party module" and "### Node.js bug bounty program" headings so the policy reads coherently (ensure the 14‑day escalation sentence is preserved if desired), and verify there are no remaining markers at the referenced locations (lines ~20–33, ~48–53, ~438) before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/linters.yml:
- Around line 54-55: There are unresolved Git conflict markers (<<<<<<<,
=======, >>>>>>>) left in the workflow YAML which break parsing; open the
.github workflow definition that contains these markers and remove the conflict
markers, merge the two conflicting blocks into the correct intended content
(choose the desired version of the steps/keys or combine them sensibly), and
ensure the resulting YAML for the linters workflow contains only valid
keys/values and passes a YAML/CI linter; specifically target the conflict
markers "<<<<<<<", "=======", and ">>>>>>>" in the linters.yml content and
resolve them so the file is valid YAML.
In `@CONTRIBUTING.md`:
- Around line 13-17: Remove the unresolved Git merge conflict markers (e.g.,
<<<<<<< HEAD, =======, and >>>>>>>) from the CONTRIBUTING.md content and resolve
the conflicting sections into a single coherent paragraph; specifically, delete
the marker lines and choose the correct wording for the Node.js build
instructions (or merge both sensible parts) and remove the duplicate/conflicting
block around the other occurrence near line 43 so the file contains no conflict
artifacts and reads as one consistent contributor guide.
In `@README.md`:
- Around line 42-44: Remove the unresolved Git conflict markers (<<<<<<<,
=======, >>>>>>>) in the README and choose the correct content for the
conflicting section (e.g., restore the intended "Contributing to the project"
link syntax or merge both sides sensibly), then delete the leftover markers and
any duplicate lines; also search for and fix the second occurrence around the
other reported spot (the block referenced at 817) so the markdown is valid and
the link reference [Contributing to the project][] is correctly defined or
converted to an inline link.
---
Outside diff comments:
In `@SECURITY.md`:
- Around line 20-33: The file SECURITY.md contains unresolved git merge conflict
markers (e.g., <<<<<<< HEAD, =======, >>>>>>> v24.16.0) around the "Node.js bug
bounty program" and "Reporting a bug in a third-party module" sections; clean
these by choosing and merging the correct text: remove all conflict markers and
duplicate blocks, consolidate the intended paragraphs under the "## Reporting a
bug in a third‑party module" and "### Node.js bug bounty program" headings so
the policy reads coherently (ensure the 14‑day escalation sentence is preserved
if desired), and verify there are no remaining markers at the referenced
locations (lines ~20–33, ~48–53, ~438) before committing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0e2f9c0-315a-4ff7-bd86-cea0feda157d
📒 Files selected for processing (10)
.github/workflows/commit-queue.yml.github/workflows/coverage-linux-without-intl.yml.github/workflows/coverage-linux.yml.github/workflows/linters.ymlCONTRIBUTING.mdREADME.mdSECURITY.mdagents/otlp/src/otlp_common.cccommon.gypiconfigure.py
💤 Files with no reviewable changes (3)
- common.gypi
- agents/otlp/src/otlp_common.cc
- configure.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/commit-queue.yml
2026-05-21 Node.js v24.16.0 Krypton (LTS) Release Git-EVTag-v0-SHA512: b4de726340b510927f6c7918e3bf8e8d3e9934bcec0032f3bc7c065491c7edaf60527d3358f6839a92eb6a39370ee10674162d3d333ac44802281c4707af8919
santigimeno
force-pushed
the
node-v24.16.0-nsolid-v6.3.0-release
branch
from
8e90ca3 to
fb1a189
Compare
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
20 participants
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 by CodeRabbit
Release Notes
New Features
Improvements
Chores