Skip to content

Commit

Permalink
Fixes #13311 (#13319)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Aug 15, 2024
1 parent a5bd94f commit 36fc324
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 22 deletions.
7 changes: 5 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ cmake_path(APPEND GLOBAL_ZIG_CACHE_DIR "global")

# Used in process.version, process.versions.node, napi, and elsewhere
set(REPORTED_NODEJS_VERSION "22.6.0")

# Used in process.versions.modules and compared while loading V8 modules
set(REPORTED_NODEJS_ABI_VERSION "127")

Expand Down Expand Up @@ -1631,12 +1632,14 @@ endif()

if(BUN_TIDY_ONLY)
find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*")
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*")
set_target_properties(${bun} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
endif()

if(BUN_TIDY_ONLY_EXTRA)
find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,performance-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*")

# -clang-analyzer-optin.core.EnumCastOutOfRange is disabled because it's too noisy, e.g. for JavaScriptCore/Lookup.h
set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "-checks=-*,clang-analyzer-*,performance-*,-clang-analyzer-webkit.UncountedLambdaCapturesChecker,-clang-analyzer-optin.core.EnumCastOutOfRange" "--fix" "--fix-errors" "--format-style=webkit" "--warnings-as-errors=*")
set_target_properties(${bun} PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}")
endif()
7 changes: 6 additions & 1 deletion packages/bun-uws/src/TopicTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,14 @@ template <typename T, typename B>
struct TopicTree {

enum IteratorFlags {
// To appease clang-analyzer
NONE = 0,

LAST = 1,
FIRST = 2
FIRST = 2,

// To appease clang-analyzer
FIRST_AND_LAST = FIRST | LAST
};

/* Whomever is iterating this topic is locked to not modify its own list */
Expand Down
3 changes: 3 additions & 0 deletions src/bun.js/api/server.classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,14 +143,17 @@ export default [
close: {
fn: "close",
length: 3,
passThis: true,
},
terminate: {
fn: "terminate",
length: 0,
passThis: true,
},
cork: {
fn: "cork",
length: 1,
passThis: true,
},
getBufferedAmount: {
fn: "getBufferedAmount",
Expand Down
22 changes: 15 additions & 7 deletions src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -4637,17 +4637,21 @@ pub const ServerWebSocket = struct {
this: *ServerWebSocket,
globalThis: *JSC.JSGlobalObject,
callframe: *JSC.CallFrame,
// Since we're passing the `this` value to the cork function, we need to
// make sure the `this` value is up to date.
this_value: JSC.JSValue,
) JSValue {
const args = callframe.arguments(1);
this.this_value = this_value;

if (args.len < 1) {
globalThis.throw("cork requires at least 1 argument", .{});
globalThis.throwNotEnoughArguments("cork", 1, 0);
return .zero;
}

const callback = args.ptr[0];
if (callback.isEmptyOrUndefinedOrNull() or !callback.isCallable(globalThis.vm())) {
globalThis.throw("cork requires a function", .{});
return .zero;
return globalThis.throwInvalidArgumentTypeValue("cork", "callback", callback);
}

if (this.isClosed()) {
Expand All @@ -4656,16 +4660,13 @@ pub const ServerWebSocket = struct {

var corker = Corker{
.globalObject = globalThis,
.this_value = this.this_value,
.this_value = this_value,
.callback = callback,
};
this.websocket().cork(&corker, Corker.run);

const result = corker.result;

if (result.isEmptyOrUndefinedOrNull())
return JSValue.jsUndefined();

if (result.isAnyError()) {
globalThis.throwValue(result);
return JSValue.jsUndefined();
Expand Down Expand Up @@ -5034,9 +5035,12 @@ pub const ServerWebSocket = struct {
this: *ServerWebSocket,
globalThis: *JSC.JSGlobalObject,
callframe: *JSC.CallFrame,
// Since close() can lead to the close() callback being called, let's always ensure the `this` value is up to date.
this_value: JSC.JSValue,
) JSValue {
const args = callframe.arguments(2);
log("close()", .{});
this.this_value = this_value;

if (this.isClosed()) {
return .undefined;
Expand Down Expand Up @@ -5078,12 +5082,16 @@ pub const ServerWebSocket = struct {
this: *ServerWebSocket,
globalThis: *JSC.JSGlobalObject,
callframe: *JSC.CallFrame,
// Since terminate() can lead to close() being called, let's always ensure the `this` value is up to date.
this_value: JSC.JSValue,
) JSValue {
_ = globalThis;
const args = callframe.arguments(2);
_ = args;
log("terminate()", .{});

this.this_value = this_value;

if (this.isClosed()) {
return .undefined;
}
Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/JSPropertyIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class JSPropertyIterator {
}

RefPtr<JSC::PropertyNameArrayData> properties;
JSC::VM& vm;
Ref<JSC::VM> vm;
static JSPropertyIterator* create(JSC::VM& vm, RefPtr<JSC::PropertyNameArrayData> data)
{
return new JSPropertyIterator(vm, data);
Expand Down
16 changes: 14 additions & 2 deletions src/bun.js/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "root.h"

#include "JavaScriptCore/Exception.h"
#include "ErrorCode+List.h"
#include "ErrorCode.h"
#include "JavaScriptCore/ThrowScope.h"
Expand Down Expand Up @@ -5069,8 +5070,19 @@ void JSC__VM__throwError(JSC__VM* vm_, JSC__JSGlobalObject* arg1, JSC__JSValue e
JSValue value = JSValue::decode(encodedValue);
scope.assertNoException(); // can't throw an exception when there's already one.
ASSERT(!value.isEmpty()); // can't throw an empty value.
JSC::JSObject* error = value.getObject();
JSC::Exception* exception = JSC::Exception::create(vm, error);

// This case can happen if we did not call .toError() on a JSValue.
if (value.isCell()) {
JSC::JSCell* cell = value.asCell();
if (cell->type() == JSC::CellType && cell->inherits<JSC::Exception>()) {
scope.throwException(arg1, jsCast<JSC::Exception*>(value));
return;
}
}

// Do not call .getObject() on it.
// https://github.com/oven-sh/bun/issues/13311
JSC::Exception* exception = JSC::Exception::create(vm, value);
scope.throwException(arg1, exception);
}

Expand Down
2 changes: 1 addition & 1 deletion src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2909,7 +2909,7 @@ pub const JSGlobalObject = opaque {
) JSValue {
const ty_str = value.jsTypeString(this).toSlice(this, bun.default_allocator);
defer ty_str.deinit();
this.ERR_INVALID_ARG_TYPE("The \"{s}\" argument must be of type {s}. Received {s}", .{ field, typename, ty_str.slice() }).throw();
this.ERR_INVALID_ARG_TYPE("The \"{s}\" argument must be of type {s}. Received {}", .{ field, typename, bun.fmt.quote(ty_str.slice()) }).throw();
return .zero;
}

Expand Down
1 change: 1 addition & 0 deletions src/codegen/class-definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export type Field =
| ({
fn: string;
length?: number;
passThis?: boolean;
DOMJIT?: {
returns: string;
args?: [string, string] | [string, string, string] | [string] | [];
Expand Down
13 changes: 7 additions & 6 deletions src/codegen/generate-classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,8 @@ function renderDecls(symbolName, typeName, proto, supportsObjectCreate = false)
`extern JSC_CALLCONV JSC::EncodedJSValue JSC_HOST_CALL_ATTRIBUTES ${symbolName(
typeName,
proto[name].fn,
)}(void* ptr, JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame);` + "\n";
)}(void* ptr, JSC::JSGlobalObject* lexicalGlobalObject, JSC::CallFrame* callFrame${proto[name].passThis ? ", JSC::EncodedJSValue thisValue" : ""});` +
"\n";
rows.push(
`
JSC_DECLARE_HOST_FUNCTION(${symbolName(typeName, name)}Callback);
Expand Down Expand Up @@ -1103,7 +1104,7 @@ JSC_DEFINE_HOST_FUNCTION(${symbolName(typeName, name)}Callback, (JSGlobalObject
JSC::EnsureStillAliveScope thisArg = JSC::EnsureStillAliveScope(thisObject);
#ifdef BUN_DEBUG
#if defined(BUN_DEBUG) && BUN_DEBUG
/** View the file name of the JS file that called this function
* from a debugger */
SourceOrigin sourceOrigin = callFrame->callerSourceOrigin(vm);
Expand All @@ -1113,14 +1114,14 @@ JSC_DEFINE_HOST_FUNCTION(${symbolName(typeName, name)}Callback, (JSGlobalObject
lastFileName = fileName;
}
JSC::EncodedJSValue result = ${symbolName(typeName, proto[name].fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame);
JSC::EncodedJSValue result = ${symbolName(typeName, proto[name].fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame${proto[name].passThis ? ", JSValue::encode(thisObject)" : ""});
ASSERT_WITH_MESSAGE(!JSValue::decode(result).isEmpty() or DECLARE_CATCH_SCOPE(vm).exception() != 0, \"${typeName}.${proto[name].fn} returned an empty value without an exception\");
return result;
#endif
return ${symbolName(typeName, proto[name].fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame);
return ${symbolName(typeName, proto[name].fn)}(thisObject->wrapped(), lexicalGlobalObject, callFrame${proto[name].passThis ? ", JSValue::encode(thisObject)" : ""});
}
`);
Expand Down Expand Up @@ -1715,9 +1716,9 @@ const JavaScriptCoreBindings = struct {
}

output += `
pub fn ${names.fn}(thisValue: *${typeName}, globalObject: *JSC.JSGlobalObject, callFrame: *JSC.CallFrame) callconv(JSC.conv) JSC.JSValue {
pub fn ${names.fn}(thisValue: *${typeName}, globalObject: *JSC.JSGlobalObject, callFrame: *JSC.CallFrame${proto[name].passThis ? ", js_this_value: JSC.JSValue" : ""}) callconv(JSC.conv) JSC.JSValue {
if (comptime Environment.enable_logs) zig("<d>${typeName}.<r>${name}<d>({})<r>", .{callFrame});
return @call(.always_inline, ${typeName}.${fn}, .{thisValue, globalObject, callFrame});
return @call(.always_inline, ${typeName}.${fn}, .{thisValue, globalObject, callFrame${proto[name].passThis ? ", js_this_value" : ""}});
}
`;
}
Expand Down
9 changes: 9 additions & 0 deletions test/js/bun/websocket/websocket-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,15 @@ describe("ServerWebSocket", () => {
let count = 0;
return {
open(ws) {
expect(() => ws.cork()).toThrow();
expect(() => ws.cork(undefined)).toThrow();
expect(() => ws.cork({})).toThrow();
expect(() =>
ws.cork(() => {
throw new Error("boom");
}),
).toThrow();

setTimeout(() => {
ws.cork(() => {
ws.send("1");
Expand Down
4 changes: 2 additions & 2 deletions test/js/node/crypto/pbkdf2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe("invalid inputs", () => {
for (let input of ["test", {}, [], true, undefined, null]) {
test(`${input} is invalid`, () => {
expect(() => crypto.pbkdf2("pass", "salt", input, 8, "sha256")).toThrow(
`The "iteration count" argument must be of type integer. Received ${typeof input}`,
`The "iteration count" argument must be of type integer. Received "${typeof input}"`,
);
});
}
Expand Down Expand Up @@ -115,7 +115,7 @@ describe("invalid inputs", () => {
[Infinity, -Infinity, NaN].forEach(input => {
test(`${input} keylen`, () => {
expect(() => crypto.pbkdf2("password", "salt", 1, input, "sha256")).toThrow(
`The \"keylen\" argument must be of type integer. Received number`,
`The \"keylen\" argument must be of type integer. Received "number"`,
);
});
});
Expand Down

0 comments on commit 36fc324

Please sign in to comment.