From 669b81c68bd7b9befc9c55f364b802bce59abf74 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 13 Mar 2021 01:13:36 +0200 Subject: [PATCH] net,tls: add abort signal support to connect Add documentation for net.connect AbortSignal, and add the support to tls.connect as well PR-URL: https://github.com/nodejs/node/pull/37735 Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell --- doc/api/net.md | 6 ++ lib/_tls_wrap.js | 2 + test/parallel/test-http2-client-destroy.js | 45 +++++++++ test/parallel/test-https-abortcontroller.js | 55 +++++++++++ .../test-https-agent-abort-controller.js | 87 +++++++++++++++++ .../test-net-connect-abort-controller.js | 96 ++++++++++++++++++ .../test-tls-connect-abort-controller.js | 97 +++++++++++++++++++ 7 files changed, 388 insertions(+) create mode 100644 test/parallel/test-https-agent-abort-controller.js create mode 100644 test/parallel/test-net-connect-abort-controller.js create mode 100644 test/parallel/test-tls-connect-abort-controller.js diff --git a/doc/api/net.md b/doc/api/net.md index 231dde85428a9c..50ea9ab6c45bc3 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -496,6 +496,10 @@ it to interact with the client. ### `new net.Socket([options])` * `options` {Object} Available options are: @@ -508,6 +512,8 @@ added: v0.3.4 otherwise ignored. **Default:** `false`. * `writable` {boolean} Allow writes on the socket when an `fd` is passed, otherwise ignored. **Default:** `false`. + * `signal` {AbortSignal} An Abort signal that may be used to destroy the + socket. * Returns: {net.Socket} Creates a new socket object. diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index c31d69117c6bd4..869bbda42f7898 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -515,6 +515,7 @@ function TLSSocket(socket, opts) { manualStart: true, highWaterMark: tlsOptions.highWaterMark, onread: !socket ? tlsOptions.onread : null, + signal: tlsOptions.signal, }]); // Proxy for API compatibility @@ -1627,6 +1628,7 @@ exports.connect = function connect(...args) { pskCallback: options.pskCallback, highWaterMark: options.highWaterMark, onread: options.onread, + signal: options.signal, }); tlssock[kConnectOptions] = options; diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js index 52b0744cd2190d..189a6f55a596da 100644 --- a/test/parallel/test-http2-client-destroy.js +++ b/test/parallel/test-http2-client-destroy.js @@ -240,3 +240,48 @@ const { getEventListeners } = require('events'); req.on('close', common.mustCall(() => server.close())); })); } + + +// Destroy ClientHttpSession with AbortSignal +{ + function testH2ConnectAbort(secure) { + const server = secure ? h2.createSecureServer() : h2.createServer(); + const controller = new AbortController(); + + server.on('stream', common.mustNotCall()); + server.listen(0, common.mustCall(() => { + const { signal } = controller; + const protocol = secure ? 'https' : 'http'; + const client = h2.connect(`${protocol}://localhost:${server.address().port}`, { + signal, + }); + client.on('close', common.mustCall()); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + + client.on('error', common.mustCall(common.mustCall((err) => { + assert.strictEqual(err.code, 'ABORT_ERR'); + assert.strictEqual(err.name, 'AbortError'); + }))); + + const req = client.request({}, {}); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + + req.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_CANCEL'); + assert.strictEqual(err.name, 'Error'); + assert.strictEqual(req.aborted, false); + assert.strictEqual(req.destroyed, true); + })); + req.on('close', common.mustCall(() => server.close())); + + assert.strictEqual(req.aborted, false); + assert.strictEqual(req.destroyed, false); + // Signal listener attached + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + + controller.abort(); + })); + } + testH2ConnectAbort(false); + testH2ConnectAbort(true); +} diff --git a/test/parallel/test-https-abortcontroller.js b/test/parallel/test-https-abortcontroller.js index 19aa9a6fca2bdf..420bf9217e8f5b 100644 --- a/test/parallel/test-https-abortcontroller.js +++ b/test/parallel/test-https-abortcontroller.js @@ -14,6 +14,7 @@ const options = { cert: fixtures.readKey('agent1-cert.pem') }; +// Check async post-aborted (async () => { const { port, server } = await new Promise((resolve) => { const server = https.createServer(options, () => {}); @@ -38,3 +39,57 @@ const options = { server.close(); } })().then(common.mustCall()); + +// Check sync post-aborted signal +(async () => { + const { port, server } = await new Promise((resolve) => { + const server = https.createServer(options, () => {}); + server.listen(0, () => resolve({ port: server.address().port, server })); + }); + try { + const ac = new AbortController(); + const { signal } = ac; + const req = https.request({ + host: 'localhost', + port, + path: '/', + method: 'GET', + rejectUnauthorized: false, + signal, + }); + assert.strictEqual(getEventListeners(ac.signal, 'abort').length, 1); + ac.abort(); + const [ err ] = await once(req, 'error'); + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.code, 'ABORT_ERR'); + } finally { + server.close(); + } +})().then(common.mustCall()); + +// Check pre-aborted signal +(async () => { + const { port, server } = await new Promise((resolve) => { + const server = https.createServer(options, () => {}); + server.listen(0, () => resolve({ port: server.address().port, server })); + }); + try { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const req = https.request({ + host: 'localhost', + port, + path: '/', + method: 'GET', + rejectUnauthorized: false, + signal, + }); + assert.strictEqual(getEventListeners(ac.signal, 'abort').length, 0); + const [ err ] = await once(req, 'error'); + assert.strictEqual(err.name, 'AbortError'); + assert.strictEqual(err.code, 'ABORT_ERR'); + } finally { + server.close(); + } +})().then(common.mustCall()); diff --git a/test/parallel/test-https-agent-abort-controller.js b/test/parallel/test-https-agent-abort-controller.js new file mode 100644 index 00000000000000..14331e7036d66d --- /dev/null +++ b/test/parallel/test-https-agent-abort-controller.js @@ -0,0 +1,87 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const https = require('https'); +const { once } = require('events'); +const Agent = https.Agent; +const fixtures = require('../common/fixtures'); + +const { getEventListeners } = require('events'); +const agent = new Agent(); + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}; + +const server = https.createServer(options); + +server.listen(0, common.mustCall(async () => { + const port = server.address().port; + const host = 'localhost'; + const options = { + port: port, + host: host, + rejectUnauthorized: false, + _agentKey: agent.getName({ port, host }) + }; + + async function postCreateConnection() { + const ac = new AbortController(); + const { signal } = ac; + const connection = agent.createConnection({ ...options, signal }); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + const [err] = await once(connection, 'error'); + assert.strictEqual(err.name, 'AbortError'); + } + + async function preCreateConnection() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const connection = agent.createConnection({ ...options, signal }); + const [err] = await once(connection, 'error'); + assert.strictEqual(err.name, 'AbortError'); + } + + + async function agentAsParam() { + const ac = new AbortController(); + const { signal } = ac; + const request = https.get({ + port: server.address().port, + path: '/hello', + agent: agent, + signal, + }); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + const [err] = await once(request, 'error'); + assert.strictEqual(err.name, 'AbortError'); + } + + async function agentAsParamPreAbort() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const request = https.get({ + port: server.address().port, + path: '/hello', + agent: agent, + signal, + }); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + const [err] = await once(request, 'error'); + assert.strictEqual(err.name, 'AbortError'); + } + + await postCreateConnection(); + await preCreateConnection(); + await agentAsParam(); + await agentAsParamPreAbort(); + server.close(); +})); diff --git a/test/parallel/test-net-connect-abort-controller.js b/test/parallel/test-net-connect-abort-controller.js new file mode 100644 index 00000000000000..9c259cc3fc2c15 --- /dev/null +++ b/test/parallel/test-net-connect-abort-controller.js @@ -0,0 +1,96 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); +const server = net.createServer(); +const { getEventListeners, once } = require('events'); + +const liveConnections = new Set(); + +server.listen(0, common.mustCall(async () => { + const port = server.address().port; + const host = 'localhost'; + const socketOptions = (signal) => ({ port, host, signal }); + server.on('connection', (connection) => { + liveConnections.add(connection); + connection.on('close', () => { + liveConnections.delete(connection); + }); + }); + + const assertAbort = async (socket, testName) => { + try { + await once(socket, 'close'); + assert.fail(`close ${testName} should have thrown`); + } catch (err) { + assert.strictEqual(err.name, 'AbortError'); + } + }; + + async function postAbort() { + const ac = new AbortController(); + const { signal } = ac; + const socket = net.connect(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + await assertAbort(socket, 'postAbort'); + } + + async function preAbort() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = net.connect(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + await assertAbort(socket, 'preAbort'); + } + + async function tickAbort() { + const ac = new AbortController(); + const { signal } = ac; + setImmediate(() => ac.abort()); + const socket = net.connect(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + await assertAbort(socket, 'tickAbort'); + } + + async function testConstructor() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = new net.Socket(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + await assertAbort(socket, 'testConstructor'); + } + + async function testConstructorPost() { + const ac = new AbortController(); + const { signal } = ac; + const socket = new net.Socket(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + await assertAbort(socket, 'testConstructorPost'); + } + + async function testConstructorPostTick() { + const ac = new AbortController(); + const { signal } = ac; + const socket = new net.Socket(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + setImmediate(() => ac.abort()); + await assertAbort(socket, 'testConstructorPostTick'); + } + + await postAbort(); + await preAbort(); + await tickAbort(); + await testConstructor(); + await testConstructorPost(); + await testConstructorPostTick(); + + // Killing the net.socket without connecting hangs the server. + for (const connection of liveConnections) { + connection.destroy(); + } + server.close(common.mustCall()); +})); diff --git a/test/parallel/test-tls-connect-abort-controller.js b/test/parallel/test-tls-connect-abort-controller.js new file mode 100644 index 00000000000000..f9a37f1603a7e3 --- /dev/null +++ b/test/parallel/test-tls-connect-abort-controller.js @@ -0,0 +1,97 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const tls = require('tls'); +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const { getEventListeners, once } = require('events'); + +const serverOptions = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}; +const server = tls.createServer(serverOptions); +server.listen(0, common.mustCall(async () => { + const port = server.address().port; + const host = 'localhost'; + const connectOptions = (signal) => ({ + port, + host, + signal, + rejectUnauthorized: false, + }); + + const assertAbort = async (socket, testName) => { + try { + await once(socket, 'close'); + assert.fail(`close ${testName} should have thrown`); + } catch (err) { + assert.strictEqual(err.name, 'AbortError'); + } + }; + + async function postAbort() { + const ac = new AbortController(); + const { signal } = ac; + const socket = tls.connect(connectOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + await assertAbort(socket, 'postAbort'); + } + + async function preAbort() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = tls.connect(connectOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + await assertAbort(socket, 'preAbort'); + } + + async function tickAbort() { + const ac = new AbortController(); + const { signal } = ac; + const socket = tls.connect(connectOptions(signal)); + setImmediate(() => ac.abort()); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + await assertAbort(socket, 'tickAbort'); + } + + async function testConstructor() { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + await assertAbort(socket, 'testConstructor'); + } + + async function testConstructorPost() { + const ac = new AbortController(); + const { signal } = ac; + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + await assertAbort(socket, 'testConstructorPost'); + } + + async function testConstructorPostTick() { + const ac = new AbortController(); + const { signal } = ac; + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + setImmediate(() => ac.abort()); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + await assertAbort(socket, 'testConstructorPostTick'); + } + + await postAbort(); + await preAbort(); + await tickAbort(); + await testConstructor(); + await testConstructorPost(); + await testConstructorPostTick(); + + server.close(common.mustCall()); +}));