From 64abc04b3e923096e755943a58584cd6fed73110 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Thu, 11 Mar 2021 23:25:45 +0100 Subject: [PATCH 1/6] lib: add brand checks to AbortController and AbortSignal --- lib/internal/abort_controller.js | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 5a16a56a4a7eb8..83264cf3f05e75 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -23,6 +23,9 @@ const { customInspectSymbol, } = require('internal/util'); const { inspect } = require('internal/util/inspect'); +const { + ERR_INVALID_THIS +} = require('internal/errors').codes; const kAborted = Symbol('kAborted'); @@ -37,13 +40,21 @@ function customInspect(self, obj, depth, options) { return `${self.constructor.name} ${inspect(obj, opts)}`; } +function validateAbortSignal(obj) { + if (obj == null || obj[kAborted] === undefined) + throw new ERR_INVALID_THIS('AbortSignal'); +} + class AbortSignal extends EventTarget { constructor() { // eslint-disable-next-line no-restricted-syntax throw new TypeError('Illegal constructor'); } - get aborted() { return !!this[kAborted]; } + get aborted() { + validateAbortSignal(this); + return !!this[kAborted]; + } [customInspectSymbol](depth, options) { return customInspect(this, { @@ -89,13 +100,26 @@ function abortSignal(signal) { // initializers for now: // https://bugs.chromium.org/p/v8/issues/detail?id=10704 const kSignal = Symbol('signal'); + +function validateAbortController(obj) { + if (obj == null || obj[kSignal] === undefined) + throw new ERR_INVALID_THIS('AbortController'); +} + class AbortController { constructor() { this[kSignal] = createAbortSignal(); } - get signal() { return this[kSignal]; } - abort() { abortSignal(this[kSignal]); } + get signal() { + validateAbortController(this); + return this[kSignal]; + } + + abort() { + validateAbortController(this); + abortSignal(this[kSignal]); + } [customInspectSymbol](depth, options) { return customInspect(this, { From 048ab0028f107c904275e8b4a19f882d71c96d4d Mon Sep 17 00:00:00 2001 From: Mattias Buelens <649348+MattiasBuelens@users.noreply.github.com> Date: Thu, 11 Mar 2021 23:56:10 +0100 Subject: [PATCH 2/6] lib: use optional chaining in brand checks Co-authored-by: Antoine du Hamel --- lib/internal/abort_controller.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 83264cf3f05e75..f5356042d4cc69 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -41,7 +41,7 @@ function customInspect(self, obj, depth, options) { } function validateAbortSignal(obj) { - if (obj == null || obj[kAborted] === undefined) + if (obj?.[kAborted] === undefined) throw new ERR_INVALID_THIS('AbortSignal'); } @@ -102,7 +102,7 @@ function abortSignal(signal) { const kSignal = Symbol('signal'); function validateAbortController(obj) { - if (obj == null || obj[kSignal] === undefined) + if (obj?.[kSignal] === undefined) throw new ERR_INVALID_THIS('AbortController'); } From 1de2c665f1a1eaa2b85bac11cbf6072a1a8a3edc Mon Sep 17 00:00:00 2001 From: Mattias Buelens <649348+MattiasBuelens@users.noreply.github.com> Date: Fri, 12 Mar 2021 00:03:59 +0100 Subject: [PATCH 3/6] lib: reformat require for internal/errors Co-authored-by: James M Snell --- lib/internal/abort_controller.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index f5356042d4cc69..6c80aa7bf4f2b3 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -24,8 +24,10 @@ const { } = require('internal/util'); const { inspect } = require('internal/util/inspect'); const { - ERR_INVALID_THIS -} = require('internal/errors').codes; + codes: { + ERR_INVALID_THIS, + } +} = require('internal/errors'); const kAborted = Symbol('kAborted'); From bc871642b1292a64a300f967e693b2b25305f81e Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Sun, 14 Mar 2021 23:32:06 +0100 Subject: [PATCH 4/6] test: check if AbortController and AbortSignal validate the receiver --- test/parallel/test-abortcontroller.js | 60 +++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 2b36da332e44aa..502b45a4880056 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -72,3 +72,63 @@ const { ok, strictEqual, throws } = require('assert'); const signal = AbortSignal.abort(); ok(signal.aborted); } + +{ + // Test that AbortController properties and methods + // validate the receiver object + const acSignalGet = Object.getOwnPropertyDescriptor( + AbortController.prototype, + 'signal' + ).get; + const acAbort = AbortController.prototype.abort; + + const goodController = new AbortController(); + ok(acSignalGet.call(goodController)); + acAbort.call(goodController); + + const badAbortControllers = [ + null, + 0, + NaN, + true, + 'AbortController', + Object.create(AbortController.prototype) + ]; + for (const badController of badAbortControllers) { + throws( + () => acSignalGet.call(badController), + { code: 'ERR_INVALID_THIS', name: 'TypeError' } + ); + throws( + () => acAbort.call(badController), + { code: 'ERR_INVALID_THIS', name: 'TypeError' } + ); + } +} + +{ + // Test that AbortSignal properties + // validate the receiver object + const signalAbortedGet = Object.getOwnPropertyDescriptor( + AbortSignal.prototype, + 'aborted' + ).get; + + const goodSignal = new AbortController().signal; + strictEqual(signalAbortedGet.call(goodSignal), false); + + const badAbortSignals = [ + null, + 0, + NaN, + true, + 'AbortSignal', + Object.create(AbortSignal.prototype) + ]; + for (const badSignal of badAbortSignals) { + throws( + () => signalAbortedGet.call(badSignal), + { code: 'ERR_INVALID_THIS', name: 'TypeError' } + ); + } +} From 2469b73890850b8ca24e467c682b6cc188b43cc6 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Mon, 15 Mar 2021 22:29:32 +0100 Subject: [PATCH 5/6] test: reword comments --- test/parallel/test-abortcontroller.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 502b45a4880056..82a58c6d8a0705 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -74,8 +74,7 @@ const { ok, strictEqual, throws } = require('assert'); } { - // Test that AbortController properties and methods - // validate the receiver object + // Test that AbortController properties and methods validate the receiver const acSignalGet = Object.getOwnPropertyDescriptor( AbortController.prototype, 'signal' @@ -107,8 +106,7 @@ const { ok, strictEqual, throws } = require('assert'); } { - // Test that AbortSignal properties - // validate the receiver object + // Test that AbortSignal properties validate the receiver const signalAbortedGet = Object.getOwnPropertyDescriptor( AbortSignal.prototype, 'aborted' From 8fa7c549db5b87becd93d782c54d8a4179589062 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Tue, 16 Mar 2021 01:40:17 +0100 Subject: [PATCH 6/6] test: check undefined as well --- test/parallel/test-abortcontroller.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 82a58c6d8a0705..d26ec8641a671e 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -87,6 +87,7 @@ const { ok, strictEqual, throws } = require('assert'); const badAbortControllers = [ null, + undefined, 0, NaN, true, @@ -117,6 +118,7 @@ const { ok, strictEqual, throws } = require('assert'); const badAbortSignals = [ null, + undefined, 0, NaN, true,