relocate the status checking code before verifying if the stream is
destroyed
PR-URL: https://github.com/nodejs/node/pull/54032
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RFC 5246 section-7.2.1 requires that the implementation must immediately
stop reading from the stream, as it is no longer TLS-encrypted. The
underlying stream is permitted to still pump events (and errors) to
other users, but those are now unencrypted, so we should not process
them here. But therefore, we do not want to stop the underlying stream,
as there could be another user of it, but we do need to remove ourselves
as a listener.
Per TLS v1.2, we should have also destroy the TLS state entirely here
(including the writing side), but this was revised in TLS v1.3 to permit
the stream to continue to flush output.
There appears to be some inconsistencies in the way nodejs handles
ownership of the underlying stream, with `TLS.close()` on the write side
also calling shutdown on the underlying stream (thus assuming other
users of the underlying stream are not permitted), while receiving EOF
on the read side leaves the underlying channel open. These
inconsistencies are left for a later person to resolve, if the extra
functionality is needed (as described in #35904). The current goal here
is to the fix the occasional CI exceptions depending on the timing of
these kernel messages through the TCP stack.
PR-URL: https://github.com/nodejs/node/pull/36111
Fixes: https://github.com/nodejs/node/pull/35946
Refs: https://github.com/libuv/libuv/pull/3036
Refs: https://github.com/nodejs/node/issues/35904
Co-authored-by: Momtchil Momtchev <momtchil@momtchev.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Signed-off-by: Darshan Sen <darshan.sen@postman.com>
PR-URL: https://github.com/nodejs/node/pull/38468
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Don't assume the reader of the code will know where to find the issue
tracker. Provide the full URL. This is especially important if the
issue tracker should move again.
PR-URL: https://github.com/nodejs/node/pull/34686
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
CallJSOnreadMethod expects the return value to be undefined or
a new buffer, so make sure to return nothing, even when an error
causes us to destroy the stream.
Fixes: https://github.com/nodejs/node/issues/34346
PR-URL: https://github.com/nodejs/node/pull/34375
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Function setTimeout in net.Socket should return this,
not undefined, as doc said.
PR-URL: https://github.com/nodejs/node/pull/32722
Refs: https://nodejs.org/api/net.html#net_socket_settimeout_timeout_callback
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
The stream will be destroyed upstream through the proper error
flow.
PR-URL: https://github.com/nodejs/node/pull/29179
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/30635
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Co-Authored-By: Anna Henningsen <anna@addaleax.net>
PR-URL: https://github.com/nodejs/node/pull/25436
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
`debuglog()` depends on `process.pid` and `process.env.NODE_DEBUG`,
so it needs to be called lazily in top scopes of internal modules
that may be loaded before these run time states are allowed to
be accessed. This patch makes its implementation lazy by default,
the process states are only accessed when the returned debug
function is called for the first time.
PR-URL: https://github.com/nodejs/node/pull/27281
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
In addition, use process.stderr instead of console.error when
there is no need to swallow the error.
PR-URL: https://github.com/nodejs/node/pull/27112
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This adds the actual callback that is passed through to the error
message in case an ERR_INVALID_CALLBACK error is thrown.
PR-URL: https://github.com/nodejs/node/pull/27048
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
The function did not only validate the timer but it caused side
effects like a warning and potentially returned a different value
than the input value. Thus the name `validate` did not seem to be
appropriate.
PR-URL: https://github.com/nodejs/node/pull/26809
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This introduces TLS1.3 support and makes it the default max protocol,
but also supports CLI/NODE_OPTIONS switches to disable it if necessary.
TLS1.3 is a major update to the TLS protocol, with many security
enhancements. It should be preferred over TLS1.2 whenever possible.
TLS1.3 is different enough that even though the OpenSSL APIs are
technically API/ABI compatible, that when TLS1.3 is negotiated, the
timing of protocol records and of callbacks broke assumptions hard-coded
into the 'tls' module.
This change introduces no API incompatibilities when TLS1.2 is
negotiated. It is the intention that it be backported to current and LTS
release lines with the default maximum TLS protocol reset to 'TLSv1.2'.
This will allow users of those lines to explicitly enable TLS1.3 if they
want.
API incompatibilities between TLS1.2 and TLS1.3 are:
- Renegotiation is not supported by TLS1.3 protocol, attempts to call
`.renegotiate()` will always fail.
- Compiling against a system OpenSSL lower than 1.1.1 is no longer
supported (OpenSSL-1.1.0 used to be supported with configure flags).
- Variations of `conn.write('data'); conn.destroy()` have undefined
behaviour according to the streams API. They may or may not send the
'data', and may or may not cause a ERR_STREAM_DESTROYED error to be
emitted. This has always been true, but conditions under which the write
suceeds is slightly but observably different when TLS1.3 is negotiated
vs when TLS1.2 or below is negotiated.
- If TLS1.3 is negotiated, and a server calls `conn.end()` in its
'secureConnection' listener without any data being written, the client
will not receive session tickets (no 'session' events will be emitted,
and `conn.getSession()` will never return a resumable session).
- The return value of `conn.getSession()` API may not return a resumable
session if called right after the handshake. The effect will be that
clients using the legacy `getSession()` API will resume sessions if
TLS1.2 is negotiated, but will do full handshakes if TLS1.3 is
negotiated. See https://github.com/nodejs/node/pull/25831 for more
information.
PR-URL: https://github.com/nodejs/node/pull/26209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Use a common `kHandle` for all `StreamBase`-based streams.
PR-URL: https://github.com/nodejs/node/pull/26491
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Allow passing a name to validateTimerDuration() so that error
messages can reflect the name of the thing being validated.
PR-URL: https://github.com/nodejs/node/pull/26215
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Improve performance by transferring information about write status
to JS through an `AliasedBuffer`, rather than object properties
set from C++.
PR-URL: https://github.com/nodejs/node/pull/23843
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Improve performance by providing JS with the raw ingridients
for the read data, i.e. an `ArrayBuffer` + offset + length
fields, instead of creating `Buffer` instances in C++ land.
PR-URL: https://github.com/nodejs/node/pull/23797
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
internalBinding is used so often that it should just automatically be
available for usage in internals.
PR-URL: https://github.com/nodejs/node/pull/23025
Refs: https://github.com/nodejs/node/commit/2a9eb31
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Merge error handling for `net.Socket`s and `Http2Stream`s,
and align the callback property names as `callback`.
Refs: https://github.com/nodejs/node/issues/19060
PR-URL: https://github.com/nodejs/node/pull/19734
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
