Using a direct approach to create the readline async iterator
allowed an iteration over 20 to 58% faster.
**BREAKING CHANGE**: With that change, the async iteterator
obtained from the readline interface doesn't have the
property "stream" any longer. This happened because it's no
longer created through a Readable, instead, the async
iterator is created directly from the events of the readline
interface instance, so, if anyone is using that property,
this change will break their code.
Also, the Readable added a backpressure control that is
fairly compensated by the use of FixedQueue + monitoring
its size. This control wasn't really precise with readline
before, though, because it only pauses the reading of the
original stream, but the lines generated from the last
message received from it was still emitted. For example:
if the readable was paused at 1000 messages but the last one
received generated 10k lines, but no further messages were
emitted again until the queue was lower than the readable
highWaterMark. A similar behavior still happens with the
new implementation, but the highWaterMark used is fixed: 1024,
and the original stream is resumed again only after the queue
is cleared.
Before making that change, I created a package implementing
the same concept used here to validate it. You can find it
[here](https://github.com/Farenheith/faster-readline-iterator)
if this helps anyhow.
PR-URL: https://github.com/nodejs/node/pull/41276
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
V8 already parses the source map magic comments. Currently, only scripts
and functions expose the parsed source map URLs. It is unnecessary to
parse the source map magic comments again when the parsed information is
available.
PR-URL: https://github.com/nodejs/node/pull/44798
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
closed promise is subscribed to first so will be
resolved first, before any read promise.
This causes data after EOF error to be thrown.
Remove the push null from the closed promise handler.
The push null gets done from the read handler
when it detects done.
PR-URL: https://github.com/nodejs/node/pull/45026
Fixes: https://github.com/nodejs/node/issues/42694
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
It doesn't make sense from a performance perspective to retain an
arraybuffer with the ALPN byte string and look it up as a property on
the JS context object for every TLS handshake.
Store the byte string in the C++ TLSWrap object instead. That's both
a lot faster and a lot simpler.
PR-URL: https://github.com/nodejs/node/pull/44875
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Limit the effect of users mutating `node:util`, `node:inspector`, or
`Array.prototype`.
PR-URL: https://github.com/nodejs/node/pull/45041
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fixes: https://github.com/nodejs/node/issues/45035
PR-URL: https://github.com/nodejs/node/pull/45044
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Co-authored-by: Rich Trott <rtrott@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/21128
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Adds an optional third argument to `defineEventHandler()` to specify
an event name that is separate from the name used for the property.
This will be used, for instance, by the quic implementation to support
generating the `on...` events where the event name itself is hyphenated.
For instance `defineEventHandler(target, 'streamreset', 'stream-reset')`
Separated out from https://github.com/nodejs/node/pull/44325
Signed-off-by: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/45032
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
If a port is not a number, throw rather than treating the `:` that
delineates the port as part of the path. This is consistent with WHATWG
URL and also mitigates hostname-spoofing.
Concerns about hostname-spoofing were raised and presented in excellent
detail by pyozzi-toss (pyozzi@toss.im/Security-Tech Team in Toss).
PR-URL: https://github.com/nodejs/node/pull/45012
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make the url.parse() hostname parsing closer to that of WHATWG URL
parsing. This mitigates for cases where hostname spoofing becomes
possible if your code checks the hostname using one API but the library
you use to send the request uses the other API.
Concerns about hostname-spoofing were raised and presented in excellent
detail by pyozzi-toss (pyozzi@toss.im/Security-Tech Team in Toss).
PR-URL: https://github.com/nodejs/node/pull/45011
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
PR-URL: https://github.com/nodejs/node/pull/44918
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Instead of doing the initializations of worker threads using small
helper functions that are also used by the main thread initialization,
wrap everything into a common prepareExecution() function with
an isMainThread switch to turn off initializations that shouldn't
be done for worker threads, so that we don't have to replicate
all the initialization steps in the worker code, which can be
error-prone.
PR-URL: https://github.com/nodejs/node/pull/44869
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This change adds validation to reject an edge case where the
child_process API argument strings might contain null bytes
somewhere in between. Such strings were being silently truncated
before, so throwing an error should prevent misuses of this API.
Fixes: https://github.com/nodejs/node/issues/44768
Signed-off-by: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44782
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44883
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44945
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44946
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
This reverts commit 736a7d8d60.
The patch attempted to fix an issue, signaled by a warning, caused by
an invalid API usage. However it introduced a behavior change that is
breaking userland modules.
It is a user error, so restore the original behavior and have the user
investigate and fix the issue in their code.
Refs: https://github.com/nodejs/node/pull/43587#issuecomment-1272092166
Refs: https://github.com/nodejs/node/pull/43587#issuecomment-1272094883
PR-URL: https://github.com/nodejs/node/pull/44921
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bethanyngriggs@gmail.com>
Reviewed-By: Tierney Cyren <hello@bnb.im>
The `size` function returned from the `size` getter of
`ByteLengthQueuingStrategy` or `CountQueuingStrategy` should not have
a prototype property, nor be a constructor.
Refs: https://streams.spec.whatwg.org/#blqs-size
Refs: https://streams.spec.whatwg.org/#cqs-size
Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44867
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44787
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
id |= 0 turns unsigned 32-bit integer values exceeding the unsigned
31-bit range into negative integers, causing a crash. Use id >>>= 0
instead, which works properly for all unsigned 32-bit integers.
Refs: https://github.com/nodejs/node/pull/36786
PR-URL: https://github.com/nodejs/node/pull/44910
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Erick Wendel <erick.workspace@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Users can set a default
value for every expected
input argument
PR-URL: https://github.com/nodejs/node/pull/44631
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This is a follow up of doc-only deprecation
https://github.com/nodejs/node/pull/43738.
Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com
PR-URL: https://github.com/nodejs/node/pull/44711
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
As long as data of the in-flight response is not yet written
to the socket, we can reply an error response without corrupting
the client.
PR-URL: https://github.com/nodejs/node/pull/44818
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
This significantly reduces the peak memory for the promise
based readFile operation by reusing a single memory chunk after
each read and strinigifying that chunk immediately.
Signed-off-by: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/44295
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add an ExitCode enum class and use it throughout the code base
instead of hard-coding the exit codes everywhere. At the moment,
the exit codes used in many places do not actually conform to
what the documentation describes. With the new enums (which
are also available to the JS land as constants in an internal
binding) we could migrate to a more consistent usage of the
codes, and eventually expose the constants to the user land
when they are stable enough.
PR-URL: https://github.com/nodejs/node/pull/44746
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/44859
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
This commit removes the experimental warning that is emitted
when the test runner is used. The test runner feature is still
considered experimental, but this change makes its output
easier to read.
PR-URL: https://github.com/nodejs/node/pull/44844
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Exposes `PerformanceEntry`, `PerformanceMark`, `PerformanceMeasure`,
`PerformanceObserver`, `PerformanceObserverEntryList`,
and `PerformanceResourceTiming` to the global scope.
PR-URL: https://github.com/nodejs/node/pull/44483
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
1. Enforce receiver checks on IDL interfaces.
2. Avoid prototype manipulation on constructing IDL interfaces with
`ReflectConstruct`.
3. `defineReplaceableAttribute` should create IDL getter/setter.
4. Corrected `PerformanceResourceTiming` to inherit the public interface
`PerformanceEntry` instead of the internal interface
`InternalPerformanceResourceTiming`.
5. `detail` is not a specified attribute on `PerfomanceEntry`. Node.js
specific extensions are moved to a subclass of `PerformanceEntry` as
`PerformanceNodeEntry`.
PR-URL: https://github.com/nodejs/node/pull/44483
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Event handler properties defined by `defineEventHandler` should check
if the receiver is a valid `EventTarget`.
PR-URL: https://github.com/nodejs/node/pull/44483
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
