2025-02-13, Version 23.8.0 (Current) - #57005
Merged
targos merged 115 commits intoFeb 13, 2025
Merged
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #56328 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: #56564 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
PR-URL: #56599 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #56739 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Distros may compile with a different openssl security level than the default. In addition there has been some discussion with respect to shipping with a different default security security level in different Node.js versions in order to main stabilty. Exposing the default openssl security level with let us have tests that work in these situations as well as allow applications to better cope with the avialable crypto algorithms. - add API to get openssl security level - modify one test to use security level instead of openssl version as an example Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: #56601 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Add an option to suppress cases where warnings are on by default Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: #56647 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #56779 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
`test-esm-loader-hooks-inspect-wait` is occasionally crashing on more platforms than Linux. Mark the test as flaky on those platforms as well. PR-URL: #56803 Refs: #54346 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #56783 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
The following tests - `test/parallel/test-https-client-renegotiation-limit.js` - `test/parallel/test-tls-alert-handling.js` - `test/parallel/test-tls-ocsp-callback.js` no longer use the OpenSSL CLI. Refs: #56714 (comment) PR-URL: #56784 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com>
PR-URL: #56786 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
`StringUtil::CharacterCount` should return the length of underlying representation storage of a protocol string. `StringUtil::CharacterCount` is only used in DictionaryValue serialization. Only `Network.Headers` is an object type, represented with DictionaryValue. PR-URL: #56788 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
Refs: #56751 Refs: https://ci.nodejs.org/job/node-test-binary-windows-js-suites/32254/RUN_SUBSET=0,nodes=win2019-COMPILED_BY-vs2022/testReport/(root)/parallel/test_without_async_context_frame/ PR-URL: #56753 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Since the last security release, the resource check has been flaky on Windows. This commit temporarily disables those checks to unblock the next regular release. PR-URL: #56789 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #56675 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com>
The cctest file `test_encoding_binding.cc` is never tested and it is not a valid test. Binding functions should never be tested with V8 API circumvented. A binding function should only be tested with JS calls. PR-URL: #56791 Refs: #55275 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
To prevent a new cctest missing from the `node.gyp`, search cctest files with tool `search_files.py` at configure time. PR-URL: #56791 Refs: #55275 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
`errorMessage` needs to be printed instead of `successMessage` in console.error(). ESM example is only fixed in previous PR. Refs: #56720 PR-URL: #56769 Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #56798 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #56799 Refs: #56794 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #56807 Refs: nodejs/undici#4032 Refs: c1ccade Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
PR-URL: #56673 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Respectively to v0.9.1 and v0.0.7. PR-URL: #56815 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #56250 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
Copy link
Copy Markdown
Member
|
I mean, Node.js core could have an unforeseen breaking change in the 115 commits of this PR. That's the whole point of running citgm. I never insinuated that there was a problem in ws itself. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
I understand, that's the whole point of CITGM and ws test suite could have unmasked a hidden breaking change, but I'm confident that this is not the case. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
@targos see https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3541/, same result for an older 23.x release. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
Considering the test is under parallel I think it could still be possible. In any case it seems the crash goes away after a rerun? |
Sorry, something went wrong.
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
marco-ippolito
approved these changes
Feb 13, 2025
Copy link
Copy Markdown
Member
|
Release build: https://ci-release.nodejs.org/job/iojs+release/10812/ |
Sorry, something went wrong.
RafaelGSS
approved these changes
Feb 13, 2025
targos
pushed a commit
that referenced
this pull request
Feb 13, 2025
Notable changes: crypto: * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) #56833 * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) #56599 deps: * update timezone to 2025a (Node.js GitHub Bot) #56876 sqlite: * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) #56790 src: * set signal inspector io thread name (RafaelGSS) #56416 * set thread name for main thread and v8 worker (RafaelGSS) #56416 * set worker thread name using worker.name (RafaelGSS) #56416 * use a default thread name for inspector (RafaelGSS) #56416 url: * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) #56452 zlib: * (SEMVER-MINOR) add zstd support (Jan Krems) #52100 PR-URL: #57005
targos
added a commit
to targos/nodejs.org
that referenced
this pull request
Feb 13, 2025
github-merge-queue Bot
pushed a commit
to nodejs/nodejs.org
that referenced
this pull request
Feb 13, 2025
Copy link
Copy Markdown
Member
|
@targos I was looking at the ws failure in CITGM and it seems to be caused by a wrong initial build step in Jenkins. The following excerpt is from https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/nodes=ubuntu2204-64/3553/console + find /home/iojs/tmp -mtime +3 + xargs rm -rf + temp=/home/iojs/tmp/citgm_tmp + rm -rf '/home/iojs/tmp/citgm_tmp/*' + mkdir /home/iojs/tmp/citgm_tmp mkdir: cannot create directory ‘/home/iojs/tmp/citgm_tmp’: No such file or directory + true which comes from the last build step in https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/configure cc: @nodejs/build |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
I think I finally found the reason why some ws tests fail with listen EINVAL in CITGM. The socket path length cannot exceed 107 characters on linux (see https://linux.die.net/man/7/unix) and on macOS the limit is even lower. Currently the listen paths are like the following on linux /home/iojs/tmp/citgm_tmp/74503795-169e-4372-afbd-fdde2a7d1e45/npm_config_tmp/ws-cabe2823f5ace7bd432969fdbf53a90e.sock and like the following on macOS /Users/admin/node-tmp/citgm_tmp/39e5fbf0-b373-462c-8e99-a0da26f046c8/npm_config_tmp/ws-679f5bb21c1e3eb56b36e331400c3b0f.sock I think I really have to avoid using os.tmpdir() and hardcode it to /tmp on unices. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
Oh, good catch! Maybe we should just use a shorter path in citgm. We don't need all the bits of a uuid. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
Please don't hardcode /tmp as the whole reason we redirect the temp dir for CITGM is that many modules didn't clean up their use of the temp dir and we slowly filled up the disk space on the machines. |
Sorry, something went wrong.
Copy link
Copy Markdown
Member
|
I'll also try shorten this https://github.com/websockets/ws/blob/976c53c4065c49ede73bfba824caf5a6e0f290cb/test/websocket.test.js#L1869. Four bytes should be sufficient. Anyway using /tmp should not be a problem in this case as these are socket paths that do not take space. |
Sorry, something went wrong.
lpinca
added a commit
to lpinca/citgm
that referenced
this pull request
Feb 20, 2025
4 tasks
lpinca
added a commit
to lpinca/citgm
that referenced
this pull request
Feb 20, 2025
lpinca
added a commit
to lpinca/citgm
that referenced
this pull request
Feb 20, 2025
lpinca
added a commit
to websockets/ws
that referenced
this pull request
Feb 20, 2025
Shorten the length of temporary file and UNIX domain socket paths. Refs: nodejs/node#57005 (comment)
targos
pushed a commit
to nodejs/citgm
that referenced
this pull request
Feb 21, 2025
acidiney
pushed a commit
to acidiney/node
that referenced
this pull request
Feb 23, 2025
Notable changes: crypto: * (SEMVER-MINOR) support --use-system-ca on Windows (Joyee Cheung) nodejs#56833 * (SEMVER-MINOR) added support for reading certificates from macOS system store (Tim Jacomb) nodejs#56599 deps: * update timezone to 2025a (Node.js GitHub Bot) nodejs#56876 sqlite: * (SEMVER-MINOR) allow returning `ArrayBufferView`s from user-defined functions (René) nodejs#56790 src: * set signal inspector io thread name (RafaelGSS) nodejs#56416 * set thread name for main thread and v8 worker (RafaelGSS) nodejs#56416 * set worker thread name using worker.name (RafaelGSS) nodejs#56416 * use a default thread name for inspector (RafaelGSS) nodejs#56416 url: * (SEMVER-MINOR) add URLPattern implementation (Yagiz Nizipli) nodejs#56452 zlib: * (SEMVER-MINOR) add zstd support (Jan Krems) nodejs#52100 PR-URL: nodejs#57005
4 tasks
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
Reviewers
Assignees
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.
2025-02-13, Version 23.8.0 (Current), @targos
Notable Changes
Support for using system CA certificates store on macOS and Windows
This version adds the --use-system-ca command-line flag, which instructs Node.js
to use the trusted CA certificates present in the system store along with
the --use-bundled-ca, --use-openssl-ca options.
This option is available on macOS and Windows for now.
Contributed by Tim Jacomb in #56599
and Joyee Cheung in #56833.
Introduction of the URL Pattern API
An implementation of the URL Pattern API
is now available.
The URLPattern constructor is exported from the node:url module and will be
available as a global in Node.js 24.
Contributed by Yagiz Nizipli and Daniel Lemire in #56452.
Support for the zstd compression algorithm
Node.js now includes support for the Zstandard (zstd) compression algorithm.
Various APIs have been added to the node:zlib module for both compression and decompression
of zstd streams.
Contributed by Jan Krems in #52100.
Node.js thread names
Threads created by the Node.js process are now named to improve the debugging experience.
Worker threads will use the name option that can be passed to the Worker constructor.
Contributed by Rafael Gonzaga in #56416.
Timezone data has been updated to 2025a
Included changes:
Other Notable Changes
Commits