Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions lib/internal/webidl.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ const {
isTypedArray,
} = require('internal/util/types');

const { getSharedArrayBufferGrowable } = internalBinding('util');

const BIGINT_2_63 = 1n << 63n;
const BIGINT_2_64 = 1n << 64n;

Expand Down Expand Up @@ -945,12 +947,8 @@ function validateBufferSourceBacking(buffer, options) {
function validateAllowGrowableSharedArrayBuffer(buffer, options) {
// SharedArrayBuffer and ArrayBufferView conversion step 3:
// IsFixedLengthArrayBuffer(buffer) must be true without [AllowResizable].
// Do not use a primordial getter here. When this module is included in the
// startup snapshot, an early-captured SharedArrayBuffer.prototype.growable
// getter does not detect growable buffers created after deserialization.
// Lazily capturing the getter would work, but it would observe the runtime
// prototype at first comparison, so it would not be an actual primordial.
if (!options.allowResizable && buffer.growable) {
if (!options.allowResizable &&
FunctionPrototypeCall(getSharedArrayBufferGrowable, buffer)) {
throw makeException(
'is backed by a growable SharedArrayBuffer, which is not allowed.',
options);
Expand Down
19 changes: 19 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,25 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);
Isolate* isolate = env->isolate();

{
const Local<Object> prototype =
SharedArrayBuffer::New(isolate, 0)->GetPrototypeV2().As<Object>();
const Local<Object> descriptor =
prototype
->GetOwnPropertyDescriptor(
context, FIXED_ONE_BYTE_STRING(isolate, "growable"))
.ToLocalChecked()
.As<Object>();
const Local<Value> getter =
descriptor->Get(context, env->get_string()).ToLocalChecked();
CHECK(getter->IsFunction());
target
->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "getSharedArrayBufferGrowable"),
getter)
.Check();
}
Comment on lines +500 to +517

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is storing this and invoking with FunctionPrototypeCall definitely quicker than a fast binding that returns sab->GetBackingStore()->IsResizableByUserJavaScript()?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't benchmarked against a fast binding. Copilot tried that predicate first, but it wasn't equivalent. Shared Wasm memory can expose fixed and growable SAB wrappers over the same backing store.

// For shared Wasm memories, this field never changes, but may differ from the
// value of the is_resizable_by_js field of SharedArrayBuffers it backs.
// WebAssembly.Memory can create multiple SharedArrayBuffers backed by the
// same BackingStore, some of which are exposed as growable, and some of which
// as fixed-length.

The getter checks the individual buffer's growability, which is what Web IDL requires.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

The flag check on the SAB itself would be sab.As<ArrayBuffer>()->IsResizableByUserJavaScript() – this checks the flag on the AB handle directly (https://github.com/v8/v8/blob/9f30cc5fc4ad6c3236c522fd23224ddc94591a20/src/api/api.cc#L4374-L4376). SAB handles are AB handles, so providing sab is known to be a SharedArrayBuffer, this should be directly equivalent to sab.growable.

Maybe worth a mini benchmark?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot ran a mini benchmark, about 10-24% faster for this check. But sab.As<ArrayBuffer>() fails with V8_ENABLE_CHECKS because the cast rejects shared buffers.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting, we already use this relationship elsewhere... although I think technically we cast v8::Value handles that are known to be SABs, rather than casting a v8::SharedArrayBuffer handle directly.


{
Local<ObjectTemplate> tmpl = ObjectTemplate::New(isolate);
#define V(PropertyName, _) \
Expand Down
88 changes: 87 additions & 1 deletion test/parallel/test-internal-webidl-buffer-source.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Flags: --expose-internals
'use strict';

require('../common');
const common = require('../common');
const assert = require('assert');
const { test } = require('node:test');
const vm = require('vm');
Expand Down Expand Up @@ -272,6 +272,92 @@ test('AllowSharedBufferSource handles growable shared buffers with explicit ' +
}
});

test('Shared buffer growability checks do not read JavaScript properties', () => {
for (const [buffer, growable] of [
[new SharedArrayBuffer(8), false],
[new SharedArrayBuffer(8, { maxByteLength: 8 }), true],
[new SharedArrayBuffer(8, { maxByteLength: 16 }), true],
[vm.runInNewContext('new SharedArrayBuffer(8)'), false],
[vm.runInNewContext('new SharedArrayBuffer(8, { maxByteLength: 16 })'), true],
]) {
const view = new Uint8Array(buffer);
const dataView = new DataView(buffer);
for (const mode of ['shadow', 'getter', 'prototype']) {
if (mode === 'shadow') {
Object.defineProperty(buffer, 'growable', {
value: !growable,
configurable: true,
});
} else if (mode === 'getter') {
Object.defineProperty(buffer, 'growable', {
get: common.mustNotCall('Unexpected growable getter'),
configurable: true,
});
} else {
delete buffer.growable;
Object.setPrototypeOf(buffer, null);
}

for (const value of [buffer, view, dataView]) {
if (growable) {
assert.throws(() => converters.AllowSharedBufferSource(value), {
code: 'ERR_INVALID_ARG_TYPE',
});
} else {
assert.strictEqual(converters.AllowSharedBufferSource(value), value);
}
assert.strictEqual(converters.AllowSharedBufferSource(value, {
allowResizable: true,
}), value);
}

if (growable) {
assert.throws(() => converters.Uint8Array(view, { allowShared: true }), {
code: 'ERR_INVALID_ARG_TYPE',
});
} else {
assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view);
}
assert.strictEqual(converters.Uint8Array(view, {
allowShared: true,
allowResizable: true,
}), view);
}
}
});

test('Shared WebAssembly buffer growability is checked per buffer', {
skip: typeof WebAssembly === 'undefined',
}, () => {
const memory = new WebAssembly.Memory({ initial: 1, maximum: 2, shared: true });
for (const [buffer, growable] of [
[memory.buffer, false],
[memory.toResizableBuffer(), true],
[memory.toFixedLengthBuffer(), false],
]) {
for (const value of [buffer, new Uint8Array(buffer), new DataView(buffer)]) {
if (growable) {
assert.throws(() => converters.AllowSharedBufferSource(value), {
code: 'ERR_INVALID_ARG_TYPE',
});
} else {
assert.strictEqual(converters.AllowSharedBufferSource(value), value);
}
assert.strictEqual(converters.AllowSharedBufferSource(value, {
allowResizable: true,
}), value);
}
const view = new Uint8Array(buffer);
if (growable) {
assert.throws(() => converters.Uint8Array(view, { allowShared: true }), {
code: 'ERR_INVALID_ARG_TYPE',
});
} else {
assert.strictEqual(converters.Uint8Array(view, { allowShared: true }), view);
}
}
});

test('BufferSource rejects objects with a forged @@toStringTag', () => {
const fake = { [Symbol.toStringTag]: 'Uint8Array' };
assert.throws(
Expand Down
1 change: 1 addition & 0 deletions typings/internalBinding/util.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export interface UtilBinding {
styleText(format: Array<string> | string, text: string): string;
isInsideNodeModules(frameLimit?: number): boolean;
constructSharedArrayBuffer(length?: number): SharedArrayBuffer;
getSharedArrayBufferGrowable(this: SharedArrayBuffer): boolean;

constants: {
kPending: 0;
Expand Down
Loading