From cfa6d572286021687c0f3710585b5374d5ff2334 Mon Sep 17 00:00:00 2001 From: pupilTong Date: Sun, 15 May 2022 21:54:06 +0800 Subject: [PATCH 1/5] net: add ability to reset a tcp socket make it possible to forcibly rest a tcp socket: * add a new method `Socket.prototype.resetAndDestroy` * add a new flag `resetAndClosing` to make `_destroy` calls the `reset` instead of close while destroying a Socket. * add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset` * change `HandleWrap::state_` from private to protected. This is essential for keeping the same behaviour between `TCPWrap::Reset` and `HandleWrap::Close` Fixes: https://github.com/nodejs/node/issues/27428 --- lib/net.js | 28 ++++++++++++++++++++++++---- src/handle_wrap.h | 2 +- src/tcp_wrap.cc | 26 ++++++++++++++++++++++++++ src/tcp_wrap.h | 2 ++ 4 files changed, 53 insertions(+), 5 deletions(-) diff --git a/lib/net.js b/lib/net.js index 1c98cf4820178f..aa8d317472c593 100644 --- a/lib/net.js +++ b/lib/net.js @@ -89,6 +89,7 @@ const { ERR_INVALID_ARG_VALUE, ERR_INVALID_FD_TYPE, ERR_INVALID_IP_ADDRESS, + ERR_INVALID_HANDLE_TYPE, ERR_SERVER_ALREADY_LISTEN, ERR_SERVER_NOT_RUNNING, ERR_SOCKET_CLOSED, @@ -640,6 +641,16 @@ Socket.prototype.end = function(data, encoding, callback) { return this; }; +Socket.prototype.resetAndDestroy = function() { + if (!(this._handle instanceof TCP)) + throw new ERR_INVALID_HANDLE_TYPE(); + if (this._handle) { + debug('reset'); + this.resetAndClosing = true; + return this.destroy(); + } + return this; +}; Socket.prototype.pause = function() { if (this[kBuffer] && !this.connecting && this._handle && @@ -710,10 +721,19 @@ Socket.prototype._destroy = function(exception, cb) { this[kBytesRead] = this._handle.bytesRead; this[kBytesWritten] = this._handle.bytesWritten; - this._handle.close(() => { - debug('emit close'); - this.emit('close', isException); - }); + if (this.resetAndClosing) { + const err = this._handle.reset(() => { + debug('emit close'); + this.emit('close', isException); + }); + if (err) + throw errnoException(err, 'reset'); + } else { + this._handle.close(() => { + debug('emit close'); + this.emit('close', isException); + }); + } this._handle.onread = noop; this._handle = null; this._sockname = null; diff --git a/src/handle_wrap.h b/src/handle_wrap.h index 2e06829b7bd885..a86f8b41c44a72 100644 --- a/src/handle_wrap.h +++ b/src/handle_wrap.h @@ -97,6 +97,7 @@ class HandleWrap : public AsyncWrap { } static void OnClose(uv_handle_t* handle); + enum { kInitialized, kClosing, kClosed } state_; private: friend class Environment; @@ -109,7 +110,6 @@ class HandleWrap : public AsyncWrap { // refer to `doc/contributing/node-postmortem-support.md` friend int GenDebugSymbols(); ListNode handle_wrap_queue_; - enum { kInitialized, kClosing, kClosed } state_; uv_handle_t* const handle_; }; diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index 747d3e028c23cc..bd3c92b75976dc 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -97,6 +97,7 @@ void TCPWrap::Initialize(Local target, GetSockOrPeerName); env->SetProtoMethod(t, "setNoDelay", SetNoDelay); env->SetProtoMethod(t, "setKeepAlive", SetKeepAlive); + env->SetProtoMethod(t, "reset", Reset); #ifdef _WIN32 env->SetProtoMethod(t, "setSimultaneousAccepts", SetSimultaneousAccepts); @@ -134,6 +135,7 @@ void TCPWrap::RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(GetSockOrPeerName); registry->Register(SetNoDelay); registry->Register(SetKeepAlive); + registry->Register(Reset); #ifdef _WIN32 registry->Register(SetSimultaneousAccepts); #endif @@ -339,7 +341,31 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args, args.GetReturnValue().Set(err); } +void TCPWrap::Reset(const FunctionCallbackInfo& args) { + TCPWrap* wrap; + ASSIGN_OR_RETURN_UNWRAP(&wrap, + args.Holder(), + args.GetReturnValue().Set(UV_EBADF)); + + int err = wrap->Reset(args[0]); + args.GetReturnValue().Set(err); +} + +int TCPWrap::Reset(Local close_callback) { + if (state_ != kInitialized) + return 0; + + int err = uv_tcp_close_reset(&handle_, OnClose); + state_ = kClosing; + if (!err & !close_callback.IsEmpty() && close_callback->IsFunction() && + !persistent().IsEmpty()) { + object()->Set(env()->context(), + env()->handle_onclose_symbol(), + close_callback).Check(); + } + return err; +} // also used by udp_wrap.cc MaybeLocal AddressToJS(Environment* env, diff --git a/src/tcp_wrap.h b/src/tcp_wrap.h index 3abf4ded19fd7c..b561fef0d31593 100644 --- a/src/tcp_wrap.h +++ b/src/tcp_wrap.h @@ -88,6 +88,8 @@ class TCPWrap : public ConnectionWrap { const v8::FunctionCallbackInfo& args, int family, std::function uv_ip_addr); + static void Reset(const v8::FunctionCallbackInfo& args); + int Reset(v8::Local close_callback = v8::Local()); #ifdef _WIN32 static void SetSimultaneousAccepts( From a4179777766dda432b96a78221536362e3d8bfe7 Mon Sep 17 00:00:00 2001 From: pupilTong Date: Sun, 15 May 2022 21:54:06 +0800 Subject: [PATCH 2/5] net: add ability to reset a tcp socket make it possible to forcibly rest a tcp socket: * add a new method `Socket.prototype.resetAndDestroy` * add a new flag `resetAndClosing` to make `_destroy` calls the `reset` instead of close while destroying a Socket. * add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset` * change `HandleWrap::state_` from private to protected. This is essential for keeping the same behaviour between `TCPWrap::Reset` and `HandleWrap::Close` Fixes: https://github.com/nodejs/node/issues/27428 --- doc/api/net.md | 14 ++++++++ lib/net.js | 24 +++++++++---- .../test-net-connect-reset-after-destroy.js | 29 +++++++++++++++ ...test-net-connect-reset-before-connected.js | 13 +++++++ .../test-net-connect-reset-until-connected.js | 20 +++++++++++ test/parallel/test-net-connect-reset.js | 13 +++++++ test/parallel/test-net-server-reset.js | 36 +++++++++++++++++++ test/parallel/test-net-socket-reset-send.js | 30 ++++++++++++++++ test/parallel/test-net-socket-reset-twice.js | 15 ++++++++ 9 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-net-connect-reset-after-destroy.js create mode 100644 test/parallel/test-net-connect-reset-before-connected.js create mode 100644 test/parallel/test-net-connect-reset-until-connected.js create mode 100644 test/parallel/test-net-connect-reset.js create mode 100644 test/parallel/test-net-server-reset.js create mode 100644 test/parallel/test-net-socket-reset-send.js create mode 100644 test/parallel/test-net-socket-reset-twice.js diff --git a/doc/api/net.md b/doc/api/net.md index 16a3a547fbb581..a08f23076a99a1 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -1089,6 +1089,20 @@ added: v0.5.10 The numeric representation of the remote port. For example, `80` or `21`. +### `socket.resetAndDestroy()` + + + +* Returns: {net.Socket} + +Close the TCP connection by sending an RST packet and destroy the stream. +If this TCP socket is in connecting status, it will send an RST packet and destroy this TCP socket once it is connected. +Otherwise, it will call `socket.destroy` this socket with an `ERR_SOCKET_CLOSED` Error. +If this is not a TCP socket (for example, a pipe), calling this method will immediately throw an `ERR_INVALID_HANDLE_TYPE` Error. +See [`writable.destroy()`][] for further details. + ### `socket.resume()` * Returns: {net.Socket} The socket itself. diff --git a/lib/net.js b/lib/net.js index aa8d317472c593..c49665c092c69f 100644 --- a/lib/net.js +++ b/lib/net.js @@ -642,12 +642,17 @@ Socket.prototype.end = function(data, encoding, callback) { }; Socket.prototype.resetAndDestroy = function() { - if (!(this._handle instanceof TCP)) - throw new ERR_INVALID_HANDLE_TYPE(); if (this._handle) { - debug('reset'); - this.resetAndClosing = true; - return this.destroy(); + if (!(this._handle instanceof TCP)) + throw new ERR_INVALID_HANDLE_TYPE(); + if (this.connecting) { + debug('reset wait for connection'); + this.once('connect', () => this._reset()); + } else { + this._reset(); + } + } else { + this.destroy(new ERR_SOCKET_CLOSED()); } return this; }; @@ -722,12 +727,13 @@ Socket.prototype._destroy = function(exception, cb) { this[kBytesWritten] = this._handle.bytesWritten; if (this.resetAndClosing) { + this.resetAndClosing = false; const err = this._handle.reset(() => { debug('emit close'); this.emit('close', isException); }); if (err) - throw errnoException(err, 'reset'); + this.emit('error', errnoException(err, 'reset')); } else { this._handle.close(() => { debug('emit close'); @@ -752,6 +758,12 @@ Socket.prototype._destroy = function(exception, cb) { } }; +Socket.prototype._reset = function() { + debug('reset connection'); + this.resetAndClosing = true; + return this.destroy(); +}; + Socket.prototype._getpeername = function() { if (!this._handle || !this._handle.getpeername) { return this._peername || {}; diff --git a/test/parallel/test-net-connect-reset-after-destroy.js b/test/parallel/test-net-connect-reset-after-destroy.js new file mode 100644 index 00000000000000..89e459229ab1bd --- /dev/null +++ b/test/parallel/test-net-connect-reset-after-destroy.js @@ -0,0 +1,29 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); + +const server = net.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const conn = net.createConnection(port); + server.on('connection', (socket) => { + socket.on('error', common.expectsError({ + code: 'ECONNRESET', + message: 'read ECONNRESET', + name: 'Error' + })); + }); + + conn.on('connect', common.mustCall(function() { + assert.strictEqual(conn, conn.resetAndDestroy().destroy()); + conn.on('error', common.mustNotCall()); + + conn.write(Buffer.from('fzfzfzfzfz'), common.expectsError({ + code: 'ERR_STREAM_DESTROYED', + message: 'Cannot call write after a stream was destroyed', + name: 'Error' + })); + server.close(); + })); +})); diff --git a/test/parallel/test-net-connect-reset-before-connected.js b/test/parallel/test-net-connect-reset-before-connected.js new file mode 100644 index 00000000000000..1dc2b98183ce31 --- /dev/null +++ b/test/parallel/test-net-connect-reset-before-connected.js @@ -0,0 +1,13 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); + +const server = net.createServer(); +server.listen(0); +const port = server.address().port; +const socket = net.connect(port, common.localhostIPv4, common.mustNotCall()); +socket.on('error', common.mustNotCall()); +server.close(); +socket.resetAndDestroy(); +// `reset` waiting socket connected to sent the RST packet +socket.destroy(); diff --git a/test/parallel/test-net-connect-reset-until-connected.js b/test/parallel/test-net-connect-reset-until-connected.js new file mode 100644 index 00000000000000..e40ec05f6ce1e9 --- /dev/null +++ b/test/parallel/test-net-connect-reset-until-connected.js @@ -0,0 +1,20 @@ +'use strict'; + +const common = require('../common'); +const net = require('net'); + +const server = net.createServer(); +server.listen(0, common.mustCall(function() { + const port = server.address().port; + const conn = net.createConnection(port); + conn.on('close', common.mustCall()); + server.on('connection', (socket) => { + socket.on('error', common.expectsError({ + code: 'ECONNRESET', + message: 'read ECONNRESET', + name: 'Error' + })); + server.close(); + }); + conn.resetAndDestroy(); +})); diff --git a/test/parallel/test-net-connect-reset.js b/test/parallel/test-net-connect-reset.js new file mode 100644 index 00000000000000..1f3e806aa99b74 --- /dev/null +++ b/test/parallel/test-net-connect-reset.js @@ -0,0 +1,13 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); + +const socket = new net.Socket(); +socket.resetAndDestroy(); +// Emit error if socket is not connecting/connected +socket.on('error', common.mustCall( + common.expectsError({ + code: 'ERR_SOCKET_CLOSED', + name: 'Error' + })) +); diff --git a/test/parallel/test-net-server-reset.js b/test/parallel/test-net-server-reset.js new file mode 100644 index 00000000000000..ea78cd2743298e --- /dev/null +++ b/test/parallel/test-net-server-reset.js @@ -0,0 +1,36 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const sockets = []; + +const server = net.createServer(function(c) { + c.on('close', common.mustCall()); + + sockets.push(c); + + if (sockets.length === 2) { + assert.strictEqual(server.close(), server); + sockets.forEach((c) => c.resetAndDestroy()); + } +}); + +server.on('close', common.mustCall()); + +assert.strictEqual(server, server.listen(0, () => { + net.createConnection(server.address().port) + .on('error', common.mustCall( + common.expectsError({ + code: 'ECONNRESET', + name: 'Error' + })) + ); + net.createConnection(server.address().port) + .on('error', common.mustCall( + common.expectsError({ + code: 'ECONNRESET', + name: 'Error' + })) + ); +})); diff --git a/test/parallel/test-net-socket-reset-send.js b/test/parallel/test-net-socket-reset-send.js new file mode 100644 index 00000000000000..b7b9f66cb93d60 --- /dev/null +++ b/test/parallel/test-net-socket-reset-send.js @@ -0,0 +1,30 @@ +'use strict'; + +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); + +const server = net.createServer(); +server.listen(0, common.mustCall(() => { + const port = server.address().port; + const conn = net.createConnection(port); + server.on('connection', (socket) => { + socket.on('error', common.expectsError({ + code: 'ECONNRESET', + message: 'read ECONNRESET', + name: 'Error' + })); + }); + + conn.on('connect', common.mustCall(() => { + assert.strictEqual(conn, conn.resetAndDestroy().destroy()); + conn.on('error', common.mustNotCall()); + + conn.write(Buffer.from('fzfzfzfzfz'), common.expectsError({ + code: 'ERR_STREAM_DESTROYED', + message: 'Cannot call write after a stream was destroyed', + name: 'Error' + })); + server.close(); + })); +})); diff --git a/test/parallel/test-net-socket-reset-twice.js b/test/parallel/test-net-socket-reset-twice.js new file mode 100644 index 00000000000000..0292c5e3ab5448 --- /dev/null +++ b/test/parallel/test-net-socket-reset-twice.js @@ -0,0 +1,15 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); + +const server = net.createServer(); +server.listen(0); +const port = server.address().port; +const conn = net.createConnection(port); + +conn.on('error', common.mustCall(() => { + conn.resetAndDestroy(); +})); + +conn.on('close', common.mustCall()); +server.close(); From f49e8b0a5494ae269ea1102b7ffc9216d363c94a Mon Sep 17 00:00:00 2001 From: pupilTong Date: Sun, 15 May 2022 21:54:06 +0800 Subject: [PATCH 3/5] net: add ability to reset a tcp socket make it possible to forcibly rest a tcp socket: * add a new method `Socket.prototype.resetAndDestroy` * add a new flag `resetAndClosing` to make `_destroy` calls the `reset` instead of close while destroying a Socket. * add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset` * change `HandleWrap::state_` from private to protected. This is essential for keeping the same behaviour between `TCPWrap::Reset` and `HandleWrap::Close` Fixes: https://github.com/nodejs/node/issues/27428 --- doc/api/net.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/api/net.md b/doc/api/net.md index a08f23076a99a1..d3eb8ed7c7a7d2 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -1101,7 +1101,6 @@ Close the TCP connection by sending an RST packet and destroy the stream. If this TCP socket is in connecting status, it will send an RST packet and destroy this TCP socket once it is connected. Otherwise, it will call `socket.destroy` this socket with an `ERR_SOCKET_CLOSED` Error. If this is not a TCP socket (for example, a pipe), calling this method will immediately throw an `ERR_INVALID_HANDLE_TYPE` Error. -See [`writable.destroy()`][] for further details. ### `socket.resume()` From 4338ec383417cbd1008b4a4135243b41a0f14381 Mon Sep 17 00:00:00 2001 From: pupilTong Date: Sun, 15 May 2022 21:54:06 +0800 Subject: [PATCH 4/5] net: add ability to reset a tcp socket make it possible to forcibly rest a tcp socket: * add a new method `Socket.prototype.resetAndDestroy` * add a new flag `resetAndClosing` to make `_destroy` calls the `reset` instead of close while destroying a Socket. * add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset` * change `HandleWrap::state_` from private to protected. This is essential for keeping the same behaviour between `TCPWrap::Reset` and `HandleWrap::Close` Fixes: https://github.com/nodejs/node/issues/27428 --- doc/api/net.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/net.md b/doc/api/net.md index d3eb8ed7c7a7d2..de2933bbc43c4c 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -1099,7 +1099,7 @@ added: REPLACEME Close the TCP connection by sending an RST packet and destroy the stream. If this TCP socket is in connecting status, it will send an RST packet and destroy this TCP socket once it is connected. -Otherwise, it will call `socket.destroy` this socket with an `ERR_SOCKET_CLOSED` Error. +Otherwise, it will call `socket.destroy` with an `ERR_SOCKET_CLOSED` Error. If this is not a TCP socket (for example, a pipe), calling this method will immediately throw an `ERR_INVALID_HANDLE_TYPE` Error. ### `socket.resume()` From 751f2f9873c344a5571a495e493dea43db94fdb5 Mon Sep 17 00:00:00 2001 From: pupilTong Date: Sun, 15 May 2022 21:54:06 +0800 Subject: [PATCH 5/5] net: add ability to reset a tcp socket make it possible to forcibly rest a tcp socket: * add a new method `Socket.prototype.resetAndDestroy` * add a new flag `resetAndClosing` to make `_destroy` calls the `reset` instead of close while destroying a Socket. * add new methods `TCPWrap::Reset` to be a wrap of `uv_tcp_close_reset` * change `HandleWrap::state_` from private to protected. This is essential for keeping the same behaviour between `TCPWrap::Reset` and `HandleWrap::Close` Fixes: https://github.com/nodejs/node/issues/27428 --- src/tcp_wrap.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/tcp_wrap.cc b/src/tcp_wrap.cc index bd3c92b75976dc..f3163fc84cd72f 100644 --- a/src/tcp_wrap.cc +++ b/src/tcp_wrap.cc @@ -343,9 +343,8 @@ void TCPWrap::Connect(const FunctionCallbackInfo& args, } void TCPWrap::Reset(const FunctionCallbackInfo& args) { TCPWrap* wrap; - ASSIGN_OR_RETURN_UNWRAP(&wrap, - args.Holder(), - args.GetReturnValue().Set(UV_EBADF)); + ASSIGN_OR_RETURN_UNWRAP( + &wrap, args.Holder(), args.GetReturnValue().Set(UV_EBADF)); int err = wrap->Reset(args[0]); @@ -353,16 +352,15 @@ void TCPWrap::Reset(const FunctionCallbackInfo& args) { } int TCPWrap::Reset(Local close_callback) { - if (state_ != kInitialized) - return 0; + if (state_ != kInitialized) return 0; int err = uv_tcp_close_reset(&handle_, OnClose); state_ = kClosing; if (!err & !close_callback.IsEmpty() && close_callback->IsFunction() && !persistent().IsEmpty()) { - object()->Set(env()->context(), - env()->handle_onclose_symbol(), - close_callback).Check(); + object() + ->Set(env()->context(), env()->handle_onclose_symbol(), close_callback) + .Check(); } return err; }