From 332ab4fe4207c8db5817be5c223e217bb470ee1a 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 --- doc/api/net.md | 6 + lib/_tls_wrap.js | 2 + test/parallel/test-http2-client-destroy.js | 46 ++++++++ .../test-https-agent-abort-controller.js | 109 +++++++++++++++++ .../test-net-connect-abort-controller.js | 110 +++++++++++++++++ .../test-tls-connect-abort-controller.js | 111 ++++++++++++++++++ 6 files changed, 384 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 231dde85428a9c0..50ea9ab6c45bc39 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 c31d69117c6bd44..869bbda42f78989 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 f257cf6f0006fa0..cbcec0307a75f4d 100644 --- a/test/parallel/test-http2-client-destroy.js +++ b/test/parallel/test-http2-client-destroy.js @@ -10,6 +10,7 @@ const h2 = require('http2'); const { kSocket } = require('internal/http2/util'); const { kEvents } = require('internal/event_target'); const Countdown = require('../common/countdown'); +const { getEventListeners } = require('events'); { const server = h2.createServer(); @@ -241,3 +242,48 @@ const Countdown = require('../common/countdown'); 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-agent-abort-controller.js b/test/parallel/test-https-agent-abort-controller.js new file mode 100644 index 000000000000000..52150bbdcbc3fb3 --- /dev/null +++ b/test/parallel/test-https-agent-abort-controller.js @@ -0,0 +1,109 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const https = require('https'); +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 }) + }; + + function postCreateConnection() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const connection = agent.createConnection({ ...options, signal }); + connection.on('error', common.mustCall((err) => { + assert(err); + assert.strictEqual(err.name, 'AbortError'); + res(); + })); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + }); + } + + function preCreateConnection() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const connection = agent.createConnection({ ...options, signal }); + connection.on('error', common.mustCall((err) => { + assert(err); + assert.strictEqual(err.name, 'AbortError'); + res(); + })); + }); + } + + // Blocked on https://github.com/nodejs/node/pull/37730 + // function agentAsParam() { + // return new Promise((res) => { + // const ac = new AbortController(); + // const { signal } = ac; + // const request = https.get({ + // port: server.address().port, + // path: '/hello', + // agent: agent, + // signal, + // }); + // request.on('error', common.mustCall((err) => { + // assert(err); + // assert.strictEqual(err.name, 'AbortError'); + // res(); + // })); + // assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + // ac.abort(); + // }); + // } + + // Blocked on https://github.com/nodejs/node/pull/37730 + // function agentAsParamPreAbort() { + // return new Promise((res) => { + // const ac = new AbortController(); + // const { signal } = ac; + // ac.abort(); + // const request = https.get({ + // port: server.address().port, + // path: '/hello', + // agent: agent, + // signal, + // }); + // request.on('error', common.mustCall((err) => { + // assert(err); + // assert.strictEqual(err.name, 'AbortError'); + // res(); + // })); + // assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + // }); + // } + + await postCreateConnection(); + await preCreateConnection(); + // Blocked on https://github.com/nodejs/node/pull/37730 + // await agentAsParam(); + // Blocked on https://github.com/nodejs/node/pull/37730 + // 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 000000000000000..733fbf1a8041081 --- /dev/null +++ b/test/parallel/test-net-connect-abort-controller.js @@ -0,0 +1,110 @@ +'use strict'; +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); +const server = net.createServer(); +const { getEventListeners } = 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 attachHandlers = (socket, res) => { + socket.on('error', common.mustCall((err) => { + assert.strictEqual(err.name, 'AbortError'); + })); + socket.on('close', () => { + res(); + }); + }; + + function postAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = net.connect(socketOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + }); + } + + function preAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = net.connect(socketOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + }); + } + + function tickAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + setImmediate(() => ac.abort()); + const socket = net.connect(socketOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + }); + } + + function testConstructor() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = new net.Socket(socketOptions(signal)); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + attachHandlers(socket, res); + }); + } + + function testConstructorPost() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = new net.Socket(socketOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + }); + } + + function testConstructorPostTick() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = new net.Socket(socketOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + setImmediate(() => { + ac.abort(); + }); + }); + } + + 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 000000000000000..027daadad34aa87 --- /dev/null +++ b/test/parallel/test-tls-connect-abort-controller.js @@ -0,0 +1,111 @@ +'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 } = 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 attachHandlers = (socket, res) => { + socket.on('error', common.mustCall((err) => { + assert.strictEqual(err.name, 'AbortError'); + })); + socket.on('close', () => { + res(); + }); + }; + + function postAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = tls.connect(connectOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + }); + } + + function preAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = tls.connect(connectOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + }); + } + + function tickAbort() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = tls.connect(connectOptions(signal)); + attachHandlers(socket, res); + setImmediate(() => ac.abort()); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + }); + } + + function testConstructor() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + ac.abort(); + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 0); + }); + } + + function testConstructorPost() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + attachHandlers(socket, res); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + ac.abort(); + }); + } + + function testConstructorPostTick() { + return new Promise((res) => { + const ac = new AbortController(); + const { signal } = ac; + const socket = new tls.TLSSocket(undefined, connectOptions(signal)); + attachHandlers(socket, res); + setImmediate(() => { + ac.abort(); + }); + assert.strictEqual(getEventListeners(signal, 'abort').length, 1); + }); + } + + await postAbort(); + await preAbort(); + await tickAbort(); + await testConstructor(); + await testConstructorPost(); + await testConstructorPostTick(); + + server.close(common.mustCall()); +}));