Skip to content

Commit 8ca0eb8

Browse files
Jarred-Sumnerdylan-conway
Jarred-Sumner
and
dylan-conway
authored
Clean up some error handling code (#15368)
Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
1 parent b19f13f commit 8ca0eb8

File tree

11 files changed

+977
-163
lines changed

11 files changed

+977
-163
lines changed

‎src/bun.js/api/bun/h2_frame_parser.zig‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,24 +3353,27 @@ pub const H2FrameParser = struct {
33533353

33543354
if (this.isServer) {
33553355
if (!ValidPseudoHeaders.has(name)) {
3356-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_PSEUDOHEADER, "\"{s}\" is an invalid pseudoheader or is used incorrectly", .{name}, globalObject);
3357-
globalObject.throwValue(exception);
3356+
if (!globalObject.hasException()) {
3357+
globalObject.ERR_HTTP2_INVALID_PSEUDOHEADER("\"{s}\" is an invalid pseudoheader or is used incorrectly", .{name}).throw();
3358+
}
33583359
return .zero;
33593360
}
33603361
} else {
33613362
if (!ValidRequestPseudoHeaders.has(name)) {
3362-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_PSEUDOHEADER, "\"{s}\" is an invalid pseudoheader or is used incorrectly", .{name}, globalObject);
3363-
globalObject.throwValue(exception);
3363+
if (!globalObject.hasException()) {
3364+
globalObject.ERR_HTTP2_INVALID_PSEUDOHEADER("\"{s}\" is an invalid pseudoheader or is used incorrectly", .{name}).throw();
3365+
}
33643366
return .zero;
33653367
}
33663368
}
33673369
} else if (ignore_pseudo_headers == 0) {
33683370
continue;
33693371
}
33703372

3371-
var js_value = try headers_arg.getTruthy(globalObject, name) orelse {
3372-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_HEADER_VALUE, "Invalid value for header \"{s}\"", .{name}, globalObject);
3373-
globalObject.throwValue(exception);
3373+
const js_value: JSC.JSValue = try headers_arg.get(globalObject, name) orelse {
3374+
if (!globalObject.hasException()) {
3375+
globalObject.ERR_HTTP2_INVALID_HEADER_VALUE("Invalid value for header \"{s}\"", .{name}).throw();
3376+
}
33743377
return .zero;
33753378
};
33763379

@@ -3380,21 +3383,24 @@ pub const H2FrameParser = struct {
33803383
var value_iter = js_value.arrayIterator(globalObject);
33813384

33823385
if (SingleValueHeaders.has(name) and value_iter.len > 1) {
3383-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_HEADER_VALUE, "Header field \"{s}\" must only have a single value", .{name}, globalObject);
3384-
globalObject.throwValue(exception);
3386+
if (!globalObject.hasException()) {
3387+
globalObject.ERR_HTTP2_INVALID_HEADER_VALUE("Header field \"{s}\" must only have a single value", .{name}).throw();
3388+
}
33853389
return .zero;
33863390
}
33873391

33883392
while (value_iter.next()) |item| {
33893393
if (item.isEmptyOrUndefinedOrNull()) {
3390-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_HEADER_VALUE, "Invalid value for header \"{s}\"", .{name}, globalObject);
3391-
globalObject.throwValue(exception);
3394+
if (!globalObject.hasException()) {
3395+
globalObject.ERR_HTTP2_INVALID_HEADER_VALUE("Invalid value for header \"{s}\"", .{name}).throw();
3396+
}
33923397
return .zero;
33933398
}
33943399

33953400
const value_str = item.toStringOrNull(globalObject) orelse {
3396-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_HEADER_VALUE, "Invalid value for header \"{s}\"", .{name}, globalObject);
3397-
globalObject.throwValue(exception);
3401+
if (!globalObject.hasException()) {
3402+
globalObject.ERR_HTTP2_INVALID_HEADER_VALUE("Invalid value for header \"{s}\"", .{name}).throw();
3403+
}
33983404
return .zero;
33993405
};
34003406

@@ -3417,11 +3423,12 @@ pub const H2FrameParser = struct {
34173423
return .undefined;
34183424
};
34193425
}
3420-
} else {
3426+
} else if (!js_value.isEmptyOrUndefinedOrNull()) {
34213427
log("single header {s}", .{name});
34223428
const value_str = js_value.toStringOrNull(globalObject) orelse {
3423-
const exception = JSC.toTypeError(.ERR_HTTP2_INVALID_HEADER_VALUE, "Invalid value for header \"{s}\"", .{name}, globalObject);
3424-
globalObject.throwValue(exception);
3429+
if (!globalObject.hasException()) {
3430+
globalObject.ERR_HTTP2_INVALID_HEADER_VALUE("Invalid value for header \"{s}\"", .{name}).throw();
3431+
}
34253432
return .zero;
34263433
};
34273434

‎src/bun.js/api/bun/socket.zig‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ pub const SocketConfig = struct {
313313

314314
pub fn fromJS(vm: *JSC.VirtualMachine, opts: JSC.JSValue, globalObject: *JSC.JSGlobalObject) bun.JSError!SocketConfig {
315315
var hostname_or_unix: JSC.ZigString.Slice = JSC.ZigString.Slice.empty;
316+
errdefer hostname_or_unix.deinit();
316317
var port: ?u16 = null;
317318
var exclusive = false;
318319
var allowHalfOpen = false;
@@ -332,23 +333,31 @@ pub const SocketConfig = struct {
332333
}
333334
}
334335

336+
errdefer {
337+
if (ssl != null) {
338+
ssl.?.deinit();
339+
}
340+
}
341+
335342
hostname_or_unix: {
336343
if (try opts.getTruthy(globalObject, "fd")) |fd_| {
337344
if (fd_.isNumber()) {
338345
break :hostname_or_unix;
339346
}
340347
}
341348

342-
if (try opts.getTruthy(globalObject, "unix")) |unix_socket| {
343-
if (!unix_socket.isString()) {
344-
return globalObject.throwInvalidArguments("Expected \"unix\" to be a string", .{});
345-
}
349+
if (try opts.getStringish(globalObject, "unix")) |unix_socket| {
350+
defer unix_socket.deref();
346351

347-
hostname_or_unix = unix_socket.getZigString(globalObject).toSlice(bun.default_allocator);
352+
hostname_or_unix = try unix_socket.toUTF8WithoutRef(bun.default_allocator).cloneIfNeeded(bun.default_allocator);
348353

349354
if (strings.hasPrefixComptime(hostname_or_unix.slice(), "file://") or strings.hasPrefixComptime(hostname_or_unix.slice(), "unix://") or strings.hasPrefixComptime(hostname_or_unix.slice(), "sock://")) {
350-
hostname_or_unix.ptr += 7;
351-
hostname_or_unix.len -|= 7;
355+
// The memory allocator relies on the pointer address to
356+
// free it, so if we simply moved the pointer up it would
357+
// cause an issue when freeing it later.
358+
const moved_bytes = try bun.default_allocator.dupeZ(u8, hostname_or_unix.slice()[7..]);
359+
hostname_or_unix.deinit();
360+
hostname_or_unix = ZigString.Slice.init(bun.default_allocator, moved_bytes);
352361
}
353362

354363
if (hostname_or_unix.len > 0) {
@@ -363,20 +372,21 @@ pub const SocketConfig = struct {
363372
allowHalfOpen = true;
364373
}
365374

366-
if (try opts.getTruthy(globalObject, "hostname") orelse try opts.getTruthy(globalObject, "host")) |hostname| {
367-
if (!hostname.isString()) {
368-
return globalObject.throwInvalidArguments("Expected \"hostname\" to be a string", .{});
369-
}
375+
if (try opts.getStringish(globalObject, "hostname") orelse try opts.getStringish(globalObject, "host")) |hostname| {
376+
defer hostname.deref();
370377

371378
var port_value = try opts.get(globalObject, "port") orelse JSValue.zero;
372-
hostname_or_unix = hostname.getZigString(globalObject).toSlice(bun.default_allocator);
379+
hostname_or_unix = try hostname.toUTF8WithoutRef(bun.default_allocator).cloneIfNeeded(bun.default_allocator);
373380

374381
if (port_value.isEmptyOrUndefinedOrNull() and hostname_or_unix.len > 0) {
375382
const parsed_url = bun.URL.parse(hostname_or_unix.slice());
376383
if (parsed_url.getPort()) |port_num| {
377384
port_value = JSValue.jsNumber(port_num);
378-
hostname_or_unix.ptr = parsed_url.hostname.ptr;
379-
hostname_or_unix.len = @as(u32, @truncate(parsed_url.hostname.len));
385+
if (parsed_url.hostname.len > 0) {
386+
const moved_bytes = try bun.default_allocator.dupeZ(u8, parsed_url.hostname);
387+
hostname_or_unix.deinit();
388+
hostname_or_unix = ZigString.Slice.init(bun.default_allocator, moved_bytes);
389+
}
380390
}
381391
}
382392

@@ -410,7 +420,6 @@ pub const SocketConfig = struct {
410420

411421
return globalObject.throwInvalidArguments("Expected either \"hostname\" or \"unix\"", .{});
412422
}
413-
errdefer hostname_or_unix.deinit();
414423

415424
var handlers = try Handlers.fromJS(globalObject, try opts.get(globalObject, "socket") orelse JSValue.zero);
416425

‎src/bun.js/api/server.zig‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,11 +1304,9 @@ pub const ServerConfig = struct {
13041304
}
13051305
if (global.hasException()) return error.JSError;
13061306

1307-
if (try arg.getTruthy(global, "hostname") orelse try arg.getTruthy(global, "host")) |host| {
1308-
const host_str = host.toSlice(
1309-
global,
1310-
bun.default_allocator,
1311-
);
1307+
if (try arg.getStringish(global, "hostname") orelse try arg.getStringish(global, "host")) |host| {
1308+
defer host.deref();
1309+
const host_str = host.toUTF8(bun.default_allocator);
13121310
defer host_str.deinit();
13131311

13141312
if (host_str.len > 0) {
@@ -1318,11 +1316,9 @@ pub const ServerConfig = struct {
13181316
}
13191317
if (global.hasException()) return error.JSError;
13201318

1321-
if (try arg.getTruthy(global, "unix")) |unix| {
1322-
const unix_str = unix.toSlice(
1323-
global,
1324-
bun.default_allocator,
1325-
);
1319+
if (try arg.getStringish(global, "unix")) |unix| {
1320+
defer unix.deref();
1321+
const unix_str = unix.toUTF8(bun.default_allocator);
13261322
defer unix_str.deinit();
13271323
if (unix_str.len > 0) {
13281324
if (has_hostname) {

‎src/bun.js/bindings/ExposeNodeModuleGlobals.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,4 +123,4 @@ extern "C" void Bun__ExposeNodeModuleGlobals(Zig::GlobalObject* globalObject)
123123
0 | JSC::PropertyAttribute::CustomValue
124124
);
125125
}
126-
}
126+
}

‎src/bun.js/bindings/bindings.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3793,8 +3793,9 @@ JSC__JSValue JSC__JSValue__getIfPropertyExistsImpl(JSC__JSValue JSValue0,
37933793

37943794
JSC::VM& vm = globalObject->vm();
37953795
JSC::JSObject* object = value.getObject();
3796-
if (UNLIKELY(!object))
3797-
return JSValue::encode({});
3796+
if (UNLIKELY(!object)) {
3797+
return JSValue::encode(JSValue::decode(JSC::JSValue::ValueDeleted));
3798+
}
37983799

37993800
// Since Identifier might not ref' the string, we need to ensure it doesn't get deref'd until this function returns
38003801
const auto propertyString = String(StringImpl::createWithoutCopying({ arg1, arg2 }));

0 commit comments

Comments
 (0)