Skip to content
Closed
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
2 changes: 1 addition & 1 deletion src/codegen/bindgen-emit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1536,7 +1536,7 @@ function cppThrowIfArgumentsMissing(signature: Signature): string {
if (names.length === 0) return "";
const message = `The ${new Intl.ListFormat("en").format(names)} argument${names.length > 1 ? "s" : ""} must be specified`;
return ` if (callFrame->argumentCount() < ${names.length}) [[unlikely]]
return Bun::ERR::MISSING_ARGS(scope, globalObject, ${asciiLiteral(message)});
RELEASE_AND_RETURN(scope, Bun::throwMissingArgsError(globalObject, ${asciiLiteral(message)}));
`;
}

Expand Down
7 changes: 7 additions & 0 deletions src/jsc/bindings/ErrorCode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2086,6 +2086,13 @@ JSC::EncodedJSValue Bun::throwInvalidThisCallError(JSC::JSGlobalObject* globalOb
return {};
}

JSC::EncodedJSValue Bun::throwMissingArgsError(JSC::JSGlobalObject* globalObject, const ASCIILiteral message)
{
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
return ERR::MISSING_ARGS(scope, globalObject, message);
}

void Bun::throwInvalidThisError(JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, JSC::JSValue thisValue, const ASCIILiteral typeName)
{
auto& vm = JSC::getVM(globalObject);
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/bindings/ErrorCode.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ JSC::JSObject* createError(VM& vm, Zig::GlobalObject* globalObject, ErrorCode co
void throwInvalidThisError(JSGlobalObject* globalObject, JSC::ThrowScope&, JSValue thisValue, const ASCIILiteral typeName);
// throwInvalidThisError(callFrame->thisValue()) and returns the empty value; one call at each generated host-function's invalid-this branch.
JSC::EncodedJSValue throwInvalidThisCallError(JSGlobalObject* globalObject, JSC::CallFrame* callFrame, const ASCIILiteral typeName);
// ERR::MISSING_ARGS in a ThrowScope of its own, out of line, so a generated host function lends no scope to its missing-arguments branch.
NEVER_INLINE JSC::EncodedJSValue throwMissingArgsError(JSGlobalObject* globalObject, const ASCIILiteral message);
JSObject* createInvalidThisError(JSGlobalObject* globalObject, const String& message);

JSC_DECLARE_HOST_FUNCTION(jsFunctionMakeErrorWithCode);
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/webcore/Crypto.bind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ export const Crypto = b.class({
rustPath: "crate::webcore::crypto::Crypto",
construct: "illegal",
proto: {
/** What is not an integer typed array throws WebCrypto's `TypeMismatchError`, which no type here throws. */
getRandomValues: { args: { array: b.any }, returns: b.any, capability: "js", throws: true },
/** The IDL type is `ArrayBufferView`, which no type here is. The body refuses what is not a view, then a view that is not an integer typed array with WebCrypto's `TypeMismatchError`. */
getRandomValues: { args: { array: b.any }, returns: b.any, capability: "js", throws: true, requiredArguments: 1 },
randomUUID: { returns: b.string },
timingSafeEqual: equal,
},
Expand Down
28 changes: 20 additions & 8 deletions src/runtime/webcore/Crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bun_core::String as BunString;
use bun_jsc::bindgen::{self, JsHeap, JsLocal, JsScope, StringOrJsBytes};
use bun_jsc::uuid::{self, UUID, UUID5, UUID7};
use bun_jsc::virtual_machine::VirtualMachine;
use bun_jsc::{JSGlobalObject, JSType, JSValue, JsClass as _};
use bun_jsc::{JSGlobalObject, JSType, JSValue, JsClass as _, JsError};

use crate::generated_bindgen::crypto::{EncodedUUID, UUIDEncodingOrTimestamp, UUIDTimestamp};
use crate::node::Encoding;
Expand All @@ -18,7 +18,7 @@ impl Crypto {
let global = cx.global();
// https://w3c.github.io/webcrypto/#Crypto-method-getRandomValues accepts only
// integer-typed views. This is an allow-list: DataView, ArrayBuffer and SharedArrayBuffer
// all pass `as_array_buffer` but must still raise TypeMismatchError.
// all pass `as_array_buffer` but are refused.
let array_buffer = array.to_js().as_array_buffer(global).filter(|_| {
matches!(
array.to_js().js_type(),
Expand All @@ -34,12 +34,7 @@ impl Crypto {
)
});
let Some(mut array_buffer) = array_buffer else {
return Err(global
.throw_dom_exception(
bun_jsc::DOMExceptionCode::TypeMismatchError,
format_args!("The data argument must be an integer-type TypedArray"),
)
.into());
return Err(throw_not_integer_view(global, array.to_js()).into());
};

random_data(global, array_buffer.byte_slice_mut());
Expand Down Expand Up @@ -70,6 +65,23 @@ impl Crypto {
}
}

/// WebIDL's conversion refuses what is not a view. The method's own steps refuse the other views.
#[cold]
fn throw_not_integer_view(global: &JSGlobalObject, value: JSValue) -> JsError {
if matches!(
value.js_type(),
JSType::Float16Array | JSType::Float32Array | JSType::Float64Array | JSType::DataView
) {
return global.throw_dom_exception(
bun_jsc::DOMExceptionCode::TypeMismatchError,
format_args!("The data argument must be an integer-type TypedArray"),
);
}
global.throw_invalid_arguments(format_args!(
"The \"array\" argument must be an instance of ArrayBufferView"
))
}

pub(crate) fn timing_safe_equal<'a>(
heap: &JsHeap<'a>,
buf1: &[u8],
Expand Down
5 changes: 4 additions & 1 deletion test/js/node/test/parallel/test-webcrypto-random.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ const { Buffer } = require('buffer');
const assert = require('assert');
const { crypto } = globalThis;

for (const value of [undefined, null, '', 1, {}, [], new ArrayBuffer(1)]) {
assert.throws(() => crypto.getRandomValues(value), TypeError);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

[
undefined, null, '', 1, {}, [],
new Float32Array(1),
new Float64Array(1),
new DataView(new ArrayBuffer(1)),
Expand Down
99 changes: 86 additions & 13 deletions test/js/web/crypto/web-crypto.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { spawnSync } from "bun";
import { describe, expect, it } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { createSecretKey, type webcrypto } from "node:crypto";
import { createSecretKey, getRandomValues as nodeGetRandomValues, type webcrypto } from "node:crypto";

type AlgorithmIdentifier = webcrypto.AlgorithmIdentifier;
type HmacImportParams = webcrypto.HmacImportParams;
Expand Down Expand Up @@ -1448,9 +1448,28 @@ describe("CryptoKey.usages ordering", () => {
});
});

// getRandomValues takes integer-typed views only; every other ArrayBufferView-ish
// argument raises TypeMismatchError rather than being filled.
// The IDL argument of getRandomValues is an ArrayBufferView. As for every WebIDL operation, a
// call without it and a value that is not a view are a TypeError. The method's own steps then
// raise TypeMismatchError for a view that is not an integer typed array.
describe("crypto.getRandomValues argument types", () => {
/** What `call` throws, or `undefined` when it returns. */
const thrownBy = (call: () => unknown) => {
try {
call();
} catch (error: any) {
return { class: error.constructor.name, code: error.code, message: error.message };
}
};
const notAView = {
class: "TypeError",
code: "ERR_INVALID_ARG_TYPE",
message: 'The "array" argument must be an instance of ArrayBufferView',
};
const typeMismatch = expect.objectContaining({
name: "TypeMismatchError",
message: "The data argument must be an integer-type TypedArray",
});

const integerViews = [
["Int8Array", () => new Int8Array(4)],
["Uint8Array", () => new Uint8Array(4)],
Expand All @@ -1470,26 +1489,80 @@ describe("crypto.getRandomValues argument types", () => {
});
}

const rejected = [
const otherViews = [
["Float16Array", () => new Float16Array(4)],
["Float32Array", () => new Float32Array(4)],
["Float64Array", () => new Float64Array(4)],
["DataView", () => new DataView(new ArrayBuffer(4))],
] as const;

for (const [name, make] of otherViews) {
it(`rejects ${name} with TypeMismatchError`, () => {
expect(() => crypto.getRandomValues(make() as any)).toThrow(typeMismatch);
});
}

// The IDL argument has no [AllowShared] or [AllowResizable], so WebIDL and Node.js refuse every
// view on these buffers with a TypeError. Bun does not test the buffer: it fills an integer
// view, and a view of another type gets the same TypeMismatchError as on a plain buffer.
const otherBuffers = [
["a SharedArrayBuffer", () => new SharedArrayBuffer(32)],
["a resizable ArrayBuffer", () => new ArrayBuffer(32, { maxByteLength: 64 })],
] as const;

for (const [name, make] of otherBuffers) {
it(`fills a Uint8Array on ${name}`, () => {
const view = new Uint8Array(make());
expect(crypto.getRandomValues(view)).toBe(view);
expect(view.some(byte => byte !== 0)).toBe(true);
});

it(`rejects a Float32Array and a DataView on ${name} with TypeMismatchError`, () => {
expect(() => crypto.getRandomValues(new Float32Array(make()))).toThrow(typeMismatch);
expect(() => crypto.getRandomValues(new DataView(make()))).toThrow(typeMismatch);
});
}

const notViews = [
["undefined", () => undefined],
["null", () => null],
["a string", () => ""],
["a number", () => 1],
["a plain object", () => ({})],
["an array", () => []],
["ArrayBuffer", () => new ArrayBuffer(4)],
["SharedArrayBuffer", () => new SharedArrayBuffer(4)],
["plain object", () => ({})],
["a Proxy of a Uint8Array", () => new Proxy(new Uint8Array(4), {})],
] as const;

for (const [name, make] of rejected) {
it(`rejects ${name} with TypeMismatchError`, () => {
expect(() => crypto.getRandomValues(make() as any)).toThrow(
expect.objectContaining({
name: "TypeMismatchError",
message: "The data argument must be an integer-type TypedArray",
}),
);
for (const [name, make] of notViews) {
it(`rejects ${name} with a TypeError`, () => {
expect(thrownBy(() => crypto.getRandomValues(make() as any))).toEqual(notAView);
});
}

it("rejects a call without an argument with a TypeError", () => {
// @ts-expect-error
expect(thrownBy(() => crypto.getRandomValues())).toEqual({
class: "TypeError",
code: "ERR_MISSING_ARGS",
message: 'The "array" argument must be specified',
});
});

it("checks `this` before the arguments", () => {
// @ts-expect-error
expect(thrownBy(() => crypto.getRandomValues.call({}))).toMatchObject({
class: "TypeError",
code: "ERR_INVALID_THIS",
});
});

// It passes its one parameter on, so the argument is never missing.
it("node:crypto's getRandomValues() without an argument is a TypeError", () => {
// @ts-expect-error
expect(thrownBy(() => nodeGetRandomValues())).toEqual(notAView);
});
});

describe("exception scope discipline", () => {
Expand Down
Loading