From b58a604034b9ba4509f9ad2a13cc180aa55096b2 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Mon, 2 Jan 2023 00:07:34 +0100 Subject: [PATCH] net: handle socket.write(cb) edge case Make sure that when calling `write()` on a connecting socket, the callback is called if the socket is destroyed before the connection is established. Fixes: https://github.com/nodejs/node/issues/30841 PR-URL: https://github.com/nodejs/node/pull/45922 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Minwoo Jung Reviewed-By: Paolo Insogna Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca --- doc/api/errors.md | 8 ++++++ lib/internal/errors.js | 3 +++ lib/net.js | 6 +++++ ...-net-write-cb-on-destroy-before-connect.js | 26 +++++++++++++++++++ 4 files changed, 43 insertions(+) create mode 100644 test/parallel/test-net-write-cb-on-destroy-before-connect.js diff --git a/doc/api/errors.md b/doc/api/errors.md index b277a53e66cd6d..3271c818c14a5f 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -2573,6 +2573,13 @@ could not be determined. An attempt was made to operate on an already closed socket. + + +### `ERR_SOCKET_CLOSED_BEFORE_CONNECTION` + +When calling [`net.Socket.write()`][] on a connecting socket and the socket was +closed before the connection was established. + ### `ERR_SOCKET_DGRAM_IS_CONNECTED` @@ -3586,6 +3593,7 @@ The native call from `process.cpuUsage` could not be processed. [`http`]: http.md [`https`]: https.md [`libuv Error handling`]: https://docs.libuv.org/en/v1.x/errors.html +[`net.Socket.write()`]: net.md#socketwritedata-encoding-callback [`net`]: net.md [`new URL(input)`]: url.md#new-urlinput-base [`new URLSearchParams(iterable)`]: url.md#new-urlsearchparamsiterable diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 5820747e0d7fcf..381fa9f15d92f6 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1566,6 +1566,9 @@ E('ERR_SOCKET_BUFFER_SIZE', 'Could not get or set buffer size', SystemError); E('ERR_SOCKET_CLOSED', 'Socket is closed', Error); +E('ERR_SOCKET_CLOSED_BEFORE_CONNECTION', + 'Socket closed before the connection was established', + Error); E('ERR_SOCKET_DGRAM_IS_CONNECTED', 'Already connected', Error); E('ERR_SOCKET_DGRAM_NOT_CONNECTED', 'Not connected', Error); E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running', Error); diff --git a/lib/net.js b/lib/net.js index fa0a56e482b712..22ecc52ddf8fc7 100644 --- a/lib/net.js +++ b/lib/net.js @@ -98,6 +98,7 @@ const { ERR_SERVER_ALREADY_LISTEN, ERR_SERVER_NOT_RUNNING, ERR_SOCKET_CLOSED, + ERR_SOCKET_CLOSED_BEFORE_CONNECTION, ERR_MISSING_ARGS, }, aggregateErrors, @@ -897,8 +898,13 @@ Socket.prototype._writeGeneric = function(writev, data, encoding, cb) { this._pendingData = data; this._pendingEncoding = encoding; this.once('connect', function connect() { + this.off('close', onClose); this._writeGeneric(writev, data, encoding, cb); }); + function onClose() { + cb(new ERR_SOCKET_CLOSED_BEFORE_CONNECTION()); + } + this.once('close', onClose); return; } this._pendingData = null; diff --git a/test/parallel/test-net-write-cb-on-destroy-before-connect.js b/test/parallel/test-net-write-cb-on-destroy-before-connect.js new file mode 100644 index 00000000000000..99efb660346c4f --- /dev/null +++ b/test/parallel/test-net-write-cb-on-destroy-before-connect.js @@ -0,0 +1,26 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +const server = net.createServer(); +server.listen(0, common.mustCall(() => { + const socket = new net.Socket(); + + socket.on('connect', common.mustNotCall()); + + socket.connect({ + port: server.address().port, + }); + + assert(socket.connecting); + + socket.write('foo', common.expectsError({ + code: 'ERR_SOCKET_CLOSED_BEFORE_CONNECTION', + name: 'Error' + })); + + socket.destroy(); + server.close(); +}));