From 707dd77d8636399aefb1cad14a56369a77d4db13 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sun, 2 May 2021 11:47:01 +0200 Subject: [PATCH] readline: validate `AbortSignal`s and remove unused event listeners PR-URL: https://github.com/nodejs/node/pull/37947 Fixes: https://github.com/nodejs/node/issues/37287 Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Robert Nagy --- lib/readline.js | 27 ++++++++++++++++++++++----- lib/readline/promises.js | 17 +++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/lib/readline.js b/lib/readline.js index 1d9e839f2c2a17..759b9ca2469201 100644 --- a/lib/readline.js +++ b/lib/readline.js @@ -48,6 +48,7 @@ const { inspect, } = require('internal/util/inspect'); const { promisify } = require('internal/util'); +const { validateAbortSignal } = require('internal/validators'); /** * @typedef {import('./stream.js').Readable} Readable @@ -130,13 +131,22 @@ Interface.prototype.question = function(query, options, cb) { options = typeof options === 'object' && options !== null ? options : {}; if (options.signal) { + validateAbortSignal(options.signal, 'options.signal'); if (options.signal.aborted) { return; } - options.signal.addEventListener('abort', () => { + const onAbort = () => { this[kQuestionCancel](); - }, { once: true }); + }; + options.signal.addEventListener('abort', onAbort, { once: true }); + const cleanup = () => { + options.signal.removeEventListener(onAbort); + }; + cb = typeof cb === 'function' ? (answer) => { + cleanup(); + return cb(answer); + } : cleanup; } if (typeof cb === 'function') { @@ -151,13 +161,20 @@ Interface.prototype.question[promisify.custom] = function(query, options) { } return new Promise((resolve, reject) => { - this.question(query, options, resolve); + let cb = resolve; if (options.signal) { - options.signal.addEventListener('abort', () => { + const onAbort = () => { reject(new AbortError()); - }, { once: true }); + }; + options.signal.addEventListener('abort', onAbort, { once: true }); + cb = (answer) => { + options.signal.removeEventListener('abort', onAbort); + resolve(answer); + }; } + + this.question(query, options, cb); }); }; diff --git a/lib/readline/promises.js b/lib/readline/promises.js index 90658e5a5e9c41..649f92f181758c 100644 --- a/lib/readline/promises.js +++ b/lib/readline/promises.js @@ -16,6 +16,7 @@ const { const { AbortError, } = require('internal/errors'); +const { validateAbortSignal } = require('internal/validators'); class Interface extends _Interface { // eslint-disable-next-line no-useless-constructor @@ -24,18 +25,26 @@ class Interface extends _Interface { } question(query, options = {}) { return new Promise((resolve, reject) => { - if (options.signal) { + let cb = resolve; + + if (options?.signal) { + validateAbortSignal(options.signal, 'options.signal'); if (options.signal.aborted) { return reject(new AbortError()); } - options.signal.addEventListener('abort', () => { + const onAbort = () => { this[kQuestionCancel](); reject(new AbortError()); - }, { once: true }); + }; + options.signal.addEventListener('abort', onAbort, { once: true }); + cb = (answer) => { + options.signal.removeEventListener('abort', onAbort); + resolve(answer); + }; } - super.question(query, resolve); + super.question(query, cb); }); } }