lib: use Web IDL interface brand checks · nodejs/node@5b6ac71 · GitHub
Skip to content

Commit 5b6ac71

Browse files
panvaaduh95
panva
authored and
aduh95
committed
lib: use Web IDL interface brand checks
Use explicit brand predicates for interface conversion instead of prototype ancestry. Update CryptoKey and AbortSignal together with the shared converter contract. Read internal AbortSignal state during composition. Preserve genuine signals after prototype changes without invoking shadowed getters. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: GitHub Copilot PR-URL: #65846 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent 2b502e0 commit 5b6ac71

7 files changed

Lines changed: 111 additions & 29 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 17 additions & 14 deletions

‎lib/internal/crypto/webidl.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ const {
1818
const {
1919
isUint32,
2020
} = require('internal/validators');
21-
const { CryptoKey } = require('internal/crypto/webcrypto');
2221
const {
2322
getCryptoKeyAlgorithm,
2423
getCryptoKeyType,
24+
isCryptoKey,
2525
} = require('internal/crypto/keys');
2626
const {
2727
bigIntArrayToUnsignedInt,
@@ -642,7 +642,7 @@ converters.AesCtrParams = createDictionaryConverter(
642642
]);
643643

644644
converters.CryptoKey = createInterfaceConverter(
645-
'CryptoKey', CryptoKey.prototype);
645+
'CryptoKey', isCryptoKey);
646646

647647
converters.EcdhKeyDeriveParams = createDictionaryConverter(
648648
'EcdhKeyDeriveParams', [

‎lib/internal/streams/iter/webidl.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ const {
55
convertToInt,
66
createDictionaryConverter,
77
createEnumConverter,
8-
createInterfaceConverter,
98
createSequenceConverter,
109
} = require('internal/webidl');
11-
const { AbortSignal } = require('internal/abort_controller');
10+
// Load AbortSignal to register its Web IDL converter in baseConverters.
11+
require('internal/abort_controller');
1212
const { isUint8Array } = require('internal/util/types');
1313

1414
const converters = { __proto__: null };
@@ -38,8 +38,7 @@ function allowStreamBufferOptions(options) {
3838
};
3939
}
4040

41-
converters.AbortSignal = createInterfaceConverter(
42-
'AbortSignal', AbortSignal.prototype);
41+
converters.AbortSignal = baseConverters.AbortSignal;
4342
converters.BackpressurePolicy = createEnumConverter('BackpressurePolicy', [
4443
'strict',
4544
'unbounded',

‎lib/internal/webidl.js‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ const {
2121
NumberMAX_SAFE_INTEGER,
2222
NumberMIN_SAFE_INTEGER,
2323
ObjectPrototypeHasOwnProperty,
24-
ObjectPrototypeIsPrototypeOf,
2524
SafeArrayIterator,
2625
SafeSet,
2726
String,
@@ -873,13 +872,13 @@ function createSequenceConverter(converter) {
873872
* Creates a converter for a Web IDL interface type.
874873
* @see https://webidl.spec.whatwg.org/#js-interface
875874
* @param {string} name Interface identifier.
876-
* @param {object} prototype Interface prototype object.
875+
* @param {(value: any) => boolean} brandCheck Interface brand predicate.
877876
* @returns {Converter}
878877
*/
879-
function createInterfaceConverter(name, prototype) {
878+
function createInterfaceConverter(name, brandCheck) {
880879
return (V, options = kEmptyObject) => {
881880
// Web IDL interface conversion step 1: return V if it implements I.
882-
if (ObjectPrototypeIsPrototypeOf(prototype, V)) {
881+
if (brandCheck(V)) {
883882
return V;
884883
}
885884
// Step 2: otherwise throw.

‎test/parallel/test-internal-webidl.js‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Flags: --expose-internals
22
'use strict';
33

4-
require('../common');
4+
const common = require('../common');
55
const assert = require('assert');
66
const vm = require('vm');
77
const webidl = require('internal/webidl');
@@ -511,10 +511,16 @@ assert.throws(() => webidl.requiredArguments(1, 2, opts), {
511511
}
512512

513513
{
514-
class Example {}
514+
class Example {
515+
#brand;
516+
517+
static is(value) {
518+
return typeof value === 'object' && value !== null && #brand in value;
519+
}
520+
}
515521
const converter = webidl.createInterfaceConverter(
516522
'Example',
517-
Example.prototype);
523+
Example.is);
518524
const example = new Example();
519525

520526
assert.strictEqual(converter(example), example);
@@ -523,6 +529,42 @@ assert.throws(() => webidl.requiredArguments(1, 2, opts), {
523529
code: 'ERR_INVALID_ARG_TYPE',
524530
message: 'Prefix: Context is not of type Example.',
525531
});
532+
assertInvalidArgType(() => converter({ __proto__: Example.prototype }));
533+
assertInvalidArgType(() => converter(new Proxy(example, {})));
534+
Object.setPrototypeOf(example, null);
535+
assert.strictEqual(converter(example), example);
536+
}
537+
538+
{
539+
const signal = AbortSignal.abort('reason');
540+
for (const value of [
541+
Object.create(AbortSignal.prototype, { aborted: { value: false } }),
542+
{ __proto__: signal },
543+
Object.create(AbortSignal.prototype, Object.getOwnPropertyDescriptors(signal)),
544+
new Proxy(signal, {}),
545+
]) {
546+
assertInvalidArgType(() => converters.AbortSignal(value));
547+
assertInvalidArgType(() => AbortSignal.any([value]));
548+
}
549+
550+
Object.setPrototypeOf(signal, null);
551+
assert.strictEqual(converters.AbortSignal(signal), signal);
552+
const composite = AbortSignal.any([signal]);
553+
assert.strictEqual(composite.aborted, true);
554+
assert.strictEqual(composite.reason, 'reason');
555+
}
556+
557+
{
558+
const controller = new AbortController();
559+
Object.defineProperties(controller.signal, {
560+
aborted: { get: common.mustNotCall('Unexpected aborted getter') },
561+
reason: { get: common.mustNotCall('Unexpected reason getter') },
562+
});
563+
const composite = AbortSignal.any([controller.signal]);
564+
assert.strictEqual(composite.aborted, false);
565+
controller.abort('reason');
566+
assert.strictEqual(composite.aborted, true);
567+
assert.strictEqual(composite.reason, 'reason');
526568
}
527569

528570
{

‎test/parallel/test-webcrypto-cryptokey-brand-check.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const { subtle } = globalThis.crypto;
5454
assert.strictEqual(Object.getPrototypeOf(internalProto), CryptoKey.prototype);
5555

5656
const invalidThis = { code: 'ERR_INVALID_THIS', name: 'TypeError' };
57+
const invalidArgType = { code: 'ERR_INVALID_ARG_TYPE', name: 'TypeError' };
5758

5859
// Plain object receiver.
5960
Object.entries(getters).forEach(([, getter]) => {
@@ -94,10 +95,10 @@ const { subtle } = globalThis.crypto;
9495
assert.strictEqual(isCryptoKey(spoofed), false);
9596
await assert.rejects(
9697
subtle.sign('HMAC', spoofed, Buffer.from('payload')),
97-
invalidThis);
98+
invalidArgType);
9899
await assert.rejects(
99100
subtle.exportKey('jwk', spoofed),
100-
invalidThis);
101+
invalidArgType);
101102

102103
// Subvert `instanceof CryptoKey` via Symbol.hasInstance, then
103104
// invoke the native getters on a forged object. The C++ tag

‎test/parallel/test-webcrypto-webidl-brand.js‎

Lines changed: 38 additions & 0 deletions

0 commit comments

Comments
 (0)