Skip to content

2024-11-12, Version 18.20.5 'Hydrogen' (LTS) - #55768

Merged
aduh95 merged 76 commits into
v18.xfrom
v18.20.5-proposal
Nov 12, 2024
Merged

2024-11-12, Version 18.20.5 'Hydrogen' (LTS)#55768
aduh95 merged 76 commits into
v18.xfrom
v18.20.5-proposal

Conversation

@aduh95

@aduh95aduh95 commented Nov 7, 2024

Copy link
Copy Markdown
Contributor

targosand others added 30 commits July 19, 2024 03:15
@targos@richardlau
benchmark: fix napi/ref addon
c2e6a8f
Refs: #53212 (comment)
PR-URL: #53233
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@ArsalanDotMe@richardlau
http: handle multi-value content-disposition header
0092358
Headers in nodejs can be arrays and current workaround for
content-disposition header do not take this into account.
This change fixes that and makes sure array values are handled
properly.
PR-URL: #50977
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@richardlau
test: update tests for OpenSSL 3.0.14
9f521f4
Starting from OpenSSL 3.0.14, 3.1.6, 3.2.2, and 3.3.1, OpenSSL was fixed
to return an error reason string for bad/unknown application protocols.
Update tests to handle both the old `ECONNRESET` error on older versions
of OpenSSL and the new `ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL` on
newer versions of OpenSSL.
Refs: openssl/openssl#24338
PR-URL: #53373
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@joyeecheung@richardlau
test: fix unreliable assumption in js-native-api/test_cannot_run_js
627d399
Previously the test assumes that when the queued finalizer is run,
it must be run at a point where env->can_call_into_js() is false
(typically, during Environment shutdown), which is not certain.
If GC kicks in early and the second pass finalizer is queued before
the event loop runs the check callbacks, the finalizer would then
be called in check callbacks (via native immediates), where
the finalizer can still call into JS. Essentially, addons can't
make assumptions about where the queued finalizer would be called.
This patch updates the assertions in the test to account for that.
PR-URL: #51898
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@richardlau
src: fix dynamically linked OpenSSL version
6615fe5
Report the version of OpenSSL that Node.js is running with instead
of the version of OpenSSL that Node.js was compiled against.
PR-URL: #53456
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@richardlau
test: update TLS tests for OpenSSL 3.2
fc43c68
Update the following TLS tests to account for error code changes
in OpenSSL 3.2 and later.
- `parallel/test-tls-empty-sni-context`
- `parallel/test-tls-psk-circuit`
PR-URL: #53384
Refs: #53382
Refs: openssl/openssl#19950
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
@richardlau
test: check against run-time OpenSSL version
cc3cdf7
Update `common.hasOpenSSL3*` to check against the run-time version of
OpenSSL instead of the version of OpenSSL that Node.js was compiled
against.
Add a generalized `common.hasOpenSSL()` so we do not need to keep adding
new checks for each new major/minor of OpenSSL.
PR-URL: #53456
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@richardlau
test: update TLS trace tests for OpenSSL >= 3.2
1320fb9
Update tests to allow for a slight change to the TLS trace messages
starting from OpenSSL 3.2.
Refs: openssl/openssl@45aac10
PR-URL: #53229
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
@sonsurim@richardlau
test: use assert.{s,deepS}trictEqual()
e9e3306
Use `asset.strictEqual()` and `asset.deepStrictEqual()` in
`test/parallel/test-tls-set-sigalgs.js`.
PR-URL: #54208
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@mhdawson@richardlau
test: fix test-tls-client-auth test for OpenSSL32
a1f0c87
Refs: #53382
Refs: #53384
Same change as in 53384 where OpenSSL32 returns a slightly
different error but for a different test.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54610
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@richardlau
test: update TLS test for OpenSSL 3.2
5316314
Update `parallel/test-tls-set-sigalgs` to account for error code changes
in OpenSSL 3.2 and later.
PR-URL: #54612
Refs: #53384
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@mhdawson@richardlau
test: increase key size for ca2-cert.pem
d5b73e5
Refs: #44498
Refs: #53382
Key sizes were increased to 2048 in PR 44498 including
the configuration file for the generation of ca2-cert.pem.
However, it seems like updating ca2-cert.pem and related files
themselves were missed as they were not updated in the PR and
the ca2-cert.pem reported as being associated with a 1024 bit key.
I believe that was the cause of some of the failures mentioned in
#53382 as OpenSSL 3.2
increased the default security level from 1 to 2 and that
would mean that certificates associated with keys of 1024 bits
would no longer be accepted.
This PR updates the key size for ca2-cert.pem. It was not
necessary to change the config, only run the generation for
the ca2-cert.pem and related files.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54599
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson@richardlau
test: fix test-tls-client-mindhsize for OpenSSL32
68156cb
Refs: #53382
- OpenSSL32 has a minimum dh key size by 2048 by
default.
- Create larter 3072 dh key needed for testing and
adjust tests to use it for builds with OpenSSL32
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54739
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson@richardlau
test: fix test test-tls-dhe for OpenSSL32
c7de027
Refs: #53382
- OpenSSL32 has a minimum dh key size by 2048 by default.
- Adjust test to use larger 3072 key instead of 1024
when OpenSSL32 is present.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54903
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson@richardlau
test: adjust tls test for OpenSSL32
e999738
Refs: #53382
Looks like test is forcing an error through bad data and
the error code we get is different for OpenSSL32. Adjust
test to cope with the variation across versions.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54909
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson@richardlau
test: adjust test-tls-junk-server for OpenSSL32
b097d85
Refs: #53382
OpenSSL32 returns different error text. Looking through
the test it seems like the expected error text has been adjusted
for different OpenSSL versions in the past and what the test
is testing is not related to the error being returned.
Update test to allow for error returned by OpenSSL32
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54926
Refs: #53382
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@panva@richardlau
crypto: reject Ed25519/Ed448 in Sign/Verify prototypes
d244204
fixes: #52097
PR-URL: #52340Fixes: #52097
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@JLHwung@richardlau
crypto: reject dh,x25519,x448 in {Sign,Verify}Final
6e7274f
Fixes: #53742
PR-URL: #53774
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@mhdawson@richardlau
test: update test to support OpenSSL32
75ff0cd
Refs: #53382
This test fails on OpenSSL32 because it complains the key being
used is too short.
It seems to have been missed when the test suite was udpated to have
a Makefile to generate key material as the keys are hard coded in the
test as opposed to being read in from the fixtures/key directory.
Update the test to use keys/certs from the fixtures directory and
to remove newlines at the end of the key and cert to retain the
inteded test.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54968
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@mhdawson@richardlau
test: adjust key sizes to support OpenSSL32
37a2f7e
Refs: #53382
This test fails on OpenSSL32 because it complains the key
being used is too short.
Adjust the key sizes so that they will pass on OpenSSL32 in
addition to other OpenSSL3 versions.
Since the keys are not public key related I don't think the
increase in key size will be too bad in terms of performance so
I've just increased versus guarding for OpenSSL32
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54972
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@mhdawson@richardlau
test: add asserts to validate test assumptions
341496a
Refs: #54968
Refs: #53382
Add additional asserts as suggestd by Richard in:
#54968
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #54997
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@mhdawson@richardlau
test: adjust tls-set-ciphers for OpenSSL32
1a4d497
Refs: #53382
The test failed as it was using AES128 which is not supported
in OpenSSL32 due to default security level and because
some error messages have changed.
Adjusted to use AES256 where it made sense and not run
tests on OpenSSL32 where test was specific to AES128.
Adjust to use the expected error messages based on version.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #55016
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@mhdawson@richardlau
test: update tls test to support OpenSSL32
9824827
Refs: #53382
OpenSSL32 does not support AES128 and DH 1024 to
update test to use newer algorithms.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #55030
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@richardlau
test: fix OpenSSL version checks
c8520ff
As per the original pull request that introduced the OpenSSL version
check in `parallel/test-crypto-dh`:
```
Error message change is test-only and uses the right error message for
versions >=3.0.12 in 3.0.x and >= 3.1.4 in 3.1.x series.
```
Fix the check so that:
- The older message is expected for OpenSSL 3.1.0.
- The newer message is expected for OpenSSL from 3.1.4 (e.g. 3.2.x).
Refs: #50395
PR-URL: #53503
Refs: #53382
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@mhdawson@richardlau
test: fix test-tls-junk-closes-server
ac3a390
Refs: #53382
TLS spec seems to indicate there should should be a response
sent when TLS handshake fails. See
https://datatracker.ietf.org/doc/html/rfc8446#page-85
When compiled with OpenSSL32 we see the
the following response '15 03 03 00 02 02 16' which
decodes as a fatal (0x02) TLS error alert number 22 (0x16).
which corresponds to TLS1_AD_RECORD_OVERFLOW which matches
the error we see if NODE_DEBUG is turned on once you get
through the define aliases.
If there is a response from the server the test used to
hang because the end event will not be emitted until after
the response is consumed. This PR fixes the test so
it consumes the response.
Some earlier OpenSSL versions did not seem to send a response but
the error handling seems to have been re-written/improved
in OpenSSL32.
Signed-off-by: Michael Dawson <midawson@redhat.com>
PR-URL: #55089
Refs: #52482
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jithil P Ponnan <jithil@outlook.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@codebytere@richardlau
src: account for OpenSSL unexpected version
d814fe9
PR-URL: #54038
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@avivkeller@aduh95
doc: add names next to release key bash commands
a4f6f09
PR-URL: #52878
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@aduh95
doc: add release key for aduh95
1c4decc
PR-URL: #55349
Refs: nodejs/Release#999
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targos@aduh95
tools: skip ruff on tools/gyp
fa72b2c
PR-URL: #50380
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@targos@aduh95
tools: update gyp-next to v0.16.1
0fb652e
PR-URL: #50380
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@avivkelleravivkeller added release Issues and PRs related to Node.js releases. and removed brotli Issues and PRs related to the brotli dependency. dependencies Pull requests that update a dependency file. labels Nov 7, 2024
@aduh95aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 11, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 11, 2024
@nodejs-github-bot

nodejs-github-bot commented Nov 11, 2024•
edited by aduh95
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/63513/
CITGM (v18.x): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3505/
CITGM (v18.20.5-proposal): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3506/
📦: https://ci-release.nodejs.org/job/iojs+release/10602/

@nodejs-github-bot

nodejs-github-bot commented Nov 11, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/63514/

mcollina
mcollina approved these changes Nov 11, 2024
View reviewed changes

@mcollinamcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nodejs-github-bot

nodejs-github-bot commented Nov 11, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/63519/

@nodejs-github-bot

nodejs-github-bot commented Nov 11, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/63520/

@richardlau

richardlau commented Nov 12, 2024

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/63520/

😡 https://ci.nodejs.org/job/node-test-commit-linuxone/nodes=rhel9-s390x/46737/console

21:37:43 Error: connect ENETUNREACH 2606:50c0:8003::154:44321:37:43 at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1555:16) {21:37:43 errno: -101,21:37:43 code: 'ENETUNREACH',21:37:43 syscall: 'connect',21:37:43 address: '2606:50c0:8003::154',21:37:43 port: 44321:37:43 }

is almost certainly nodejs/build#3950 which annoyingly was not happening on the rhel9-s390x machines last week (i.e. when I was not on leave).

@richardlaurichardlau mentioned this pull request Nov 12, 2024
Closed
@nodejs-github-bot

nodejs-github-bot commented Nov 12, 2024

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/63521/

@richardlaurichardlau mentioned this pull request Nov 12, 2024
Merged
8 tasks
@richardlau

richardlau commented Nov 12, 2024

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/63520/
is almost certainly nodejs/build#3950 which annoyingly was not happening on the rhel9-s390x machines last week (i.e. when I was not on leave).

I've extended nodejs/build#3952 to the rhel9-s390x machines.
https://ci.nodejs.org/job/node-test-commit-linuxone/46738/nodes=rhel9-s390x/ (for https://ci.nodejs.org/job/node-test-pull-request/63521/) passed (unstable, one flaky test).

@lpinca

lpinca commented Nov 12, 2024

Copy link
Copy Markdown
Member

I'm not sure what went wrong and I don't care but the author of this change e9e3306426 is me.

@aduh95
aduh95 merged commit f73db8f into v18.xNov 12, 2024
@aduh95
aduh95 deleted the v18.20.5-proposal branch November 12, 2024 21:34
aduh95 added a commit that referenced this pull request Nov 12, 2024
@aduh95
Working on v18.20.6
14b6317
PR-URL: #55768
aduh95 added a commit that referenced this pull request Nov 12, 2024
@aduh95
2024-11-12, Version 18.20.5 'Hydrogen' (LTS)
f17b9a4
Notable changes:
esm:
* mark import attributes and JSON module as stable (Nicolò Ribaudo) #55333
PR-URL: #55768
aduh95 added a commit to aduh95/nodejs.org that referenced this pull request Nov 12, 2024
@aduh95
Blog: v18.20.5 release post
a697ca5
Refs: nodejs/node#55768
aduh95 added a commit to aduh95/nodejs.org that referenced this pull request Nov 12, 2024
@aduh95
Blog: v18.20.5 release post
c68203a
Refs: nodejs/node#55768
@aduh95aduh95 mentioned this pull request Nov 12, 2024
Merged
5 tasks
@aduh95

aduh95 commented Nov 12, 2024

Copy link
Copy Markdown
ContributorAuthor

I'm not sure what went wrong and I don't care but the author of this change e9e3306426 is me.

My bad! I have honestly no idea how it happened

github-merge-queueBot pushed a commit to nodejs/nodejs.org that referenced this pull request Nov 13, 2024
@aduh95
Blog: v18.20.5 release post (#7221)
24925de
Refs: nodejs/node#55768
bmuenzenmeyer pushed a commit to nodejs/nodejs.org that referenced this pull request Nov 21, 2024
@aduh95@bmuenzenmeyer
Blog: v18.20.5 release post (#7221)
01fc8a7
Refs: nodejs/node#55768
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@anonriganonriganonrig approved these changes
@RafaelGSSRafaelGSSRafaelGSS approved these changes

Assignees

No one assigned

Labels

needs-ciPRs that need a full CI run.releaseIssues and PRs related to Node.js releases.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

19 participants

@aduh95@nodejs-github-bot@richardlau@lpinca@mcollina@anonrig@RafaelGSS@avivkeller@targos@ArsalanDotMe@joyeecheung@sonsurim@mhdawson@panva@JLHwung@codebytere@bmacnaughton@nicolo-ribaudo@npm-cli-bot