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

Commit 98a7ff5

Browse files
panvapanva
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
1 parent f16b556 commit 98a7ff5

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
@@ -17,10 +17,10 @@ const {
1717
const {
1818
isUint32,
1919
} = require('internal/validators');
20-
const { CryptoKey } = require('internal/crypto/webcrypto');
2120
const {
2221
getCryptoKeyAlgorithm,
2322
getCryptoKeyType,
23+
isCryptoKey,
2424
} = require('internal/crypto/keys');
2525
const {
2626
bigIntArrayToUnsignedInt,
@@ -618,7 +618,7 @@ converters.AesCtrParams = createDictionaryConverter(
618618
]);
619619

620620
converters.CryptoKey = createInterfaceConverter(
621-
'CryptoKey', CryptoKey.prototype);
621+
'CryptoKey', isCryptoKey);
622622

623623
converters.EcdhKeyDeriveParams = createDictionaryConverter(
624624
'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)