From 625e537f5d2ab90b269855980dfbfb73e1c6ba13 Mon Sep 17 00:00:00 2001 From: Ciro Spaciari Date: Tue, 7 Oct 2025 22:35:08 -0700 Subject: [PATCH] fix(NodeHTTP) remove unneeded code add more safety measures agains raw_response after upgrade/close (#23348) ### What does this PR do? BeforeOpen code is not necessary since we have `setOnSocketUpgraded` callback now,and we should NOT convert websocket to a response, make sure that no closed socket is passed to `JSNodeHTTPServerSocket`, change isIdle to be inside AsyncSocketData to be more reliable (works for websocket and normal sockets) ### How did you verify your code works? CI --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- packages/bun-uws/src/App.h | 6 +- packages/bun-uws/src/AsyncSocketData.h | 1 + packages/bun-uws/src/HttpContext.h | 1 + packages/bun-uws/src/HttpResponseData.h | 3 - packages/bun-uws/src/WebSocketData.h | 2 + src/bun.js/api/server.zig | 29 +-- src/bun.js/api/server/NodeHTTPResponse.zig | 195 +++++++++--------- src/bun.js/api/server/ServerWebSocket.zig | 7 - .../api/server/WebSocketServerContext.zig | 12 -- .../bindings/node/JSNodeHTTPServerSocket.cpp | 10 +- src/js/node/_http_server.ts | 2 +- 11 files changed, 126 insertions(+), 142 deletions(-) diff --git a/packages/bun-uws/src/App.h b/packages/bun-uws/src/App.h index 9c73e64dba..251665b8ea 100644 --- a/packages/bun-uws/src/App.h +++ b/packages/bun-uws/src/App.h @@ -303,10 +303,10 @@ public: auto context = (struct us_socket_context_t *)this->httpContext; struct us_socket_t *s = context->head_sockets; while (s) { - HttpResponseData *httpResponseData = HttpResponse::getHttpResponseDataS(s); - httpResponseData->shouldCloseOnceIdle = true; + // no matter the type of socket will always contain the AsyncSocketData + auto *data = ((AsyncSocket *) s)->getAsyncSocketData(); struct us_socket_t *next = s->next; - if (httpResponseData->isIdle) { + if (data->isIdle) { us_socket_close(SSL, s, LIBUS_SOCKET_CLOSE_CODE_CLEAN_SHUTDOWN, 0); } s = next; diff --git a/packages/bun-uws/src/AsyncSocketData.h b/packages/bun-uws/src/AsyncSocketData.h index ad0d13ca5e..72ac4e5004 100644 --- a/packages/bun-uws/src/AsyncSocketData.h +++ b/packages/bun-uws/src/AsyncSocketData.h @@ -83,6 +83,7 @@ struct AsyncSocketData { /* Or empty */ AsyncSocketData() = default; + bool isIdle = false; }; } diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 6fc803295d..8f001bf0da 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -253,6 +253,7 @@ private: /* Mark that we are inside the parser now */ httpContextData->flags.isParsingHttp = true; httpResponseData->isIdle = false; + // clients need to know the cursor after http parse, not servers! // how far did we read then? we need to know to continue with websocket parsing data? or? diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 8fb572d900..43a09558cb 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -109,9 +109,6 @@ struct HttpResponseData : AsyncSocketData, HttpParser { uint8_t idleTimeout = 10; // default HTTP_TIMEOUT 10 seconds bool fromAncientRequest = false; bool isConnectRequest = false; - bool isIdle = true; - bool shouldCloseOnceIdle = false; - #ifdef UWS_WITH_PROXY ProxyParser proxyParser; diff --git a/packages/bun-uws/src/WebSocketData.h b/packages/bun-uws/src/WebSocketData.h index f9139341d1..25bd545084 100644 --- a/packages/bun-uws/src/WebSocketData.h +++ b/packages/bun-uws/src/WebSocketData.h @@ -70,6 +70,8 @@ public: inflationStream = new InflationStream(compressOptions); } } + // never close websocket sockets when closing idle connections + this->isIdle = false; this->socketData = socketData; this->onSocketClosed = onSocketClosed; } diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index 19ce635155..54c4ed1a30 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -812,10 +812,11 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d if (fetch_headers_to_use.fastGet(.SecWebSocketExtensions)) |protocol| { sec_websocket_extensions = protocol; } - - // we must write the status first so that 200 OK isn't written - nodeHttpResponse.raw_response.writeStatus("101 Switching Protocols"); - fetch_headers_to_use.toUWSResponse(comptime ssl_enabled, nodeHttpResponse.raw_response.socket()); + if (nodeHttpResponse.raw_response) |raw_response| { + // we must write the status first so that 200 OK isn't written + raw_response.writeStatus("101 Switching Protocols"); + fetch_headers_to_use.toUWSResponse(comptime ssl_enabled, raw_response.socket()); + } } if (globalThis.hasException()) { @@ -1936,12 +1937,15 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d _ = vm.uncaughtException(globalThis, err, http_result == .rejection); if (node_http_response) |node_response| { - if (!node_response.flags.request_has_completed and node_response.raw_response.state().isResponsePending()) { - if (node_response.raw_response.state().isHttpStatusCalled()) { - node_response.raw_response.writeStatus("500 Internal Server Error"); - node_response.raw_response.endWithoutBody(true); - } else { - node_response.raw_response.endStream(true); + if (!node_response.flags.upgraded and node_response.raw_response != null) { + const raw_response = node_response.raw_response.?; + if (!node_response.flags.request_has_completed and raw_response.state().isResponsePending()) { + if (raw_response.state().isHttpStatusCalled()) { + raw_response.writeStatus("500 Internal Server Error"); + raw_response.endWithoutBody(true); + } else { + raw_response.endStream(true); + } } } node_response.onRequestComplete(); @@ -1952,8 +1956,9 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d } if (node_http_response) |node_response| { - if (!node_response.flags.upgraded) { - if (!node_response.flags.request_has_completed and node_response.raw_response.state().isResponsePending()) { + if (!node_response.flags.upgraded and node_response.raw_response != null) { + const raw_response = node_response.raw_response.?; + if (!node_response.flags.request_has_completed and raw_response.state().isResponsePending()) { node_response.setOnAbortedHandler(); } // If we ended the response without attaching an ondata handler, we discard the body read stream diff --git a/src/bun.js/api/server/NodeHTTPResponse.zig b/src/bun.js/api/server/NodeHTTPResponse.zig index 94f9dc44e9..4baba2f863 100644 --- a/src/bun.js/api/server/NodeHTTPResponse.zig +++ b/src/bun.js/api/server/NodeHTTPResponse.zig @@ -13,7 +13,7 @@ pub const deref = RefCount.deref; ref_count: RefCount, -raw_response: uws.AnyResponse, +raw_response: ?uws.AnyResponse, flags: Flags = .{}, @@ -104,57 +104,39 @@ pub const BodyReadState = enum(u8) { extern "C" fn Bun__getNodeHTTPResponseThisValue(bool, *anyopaque) jsc.JSValue; pub fn getThisValue(this: *NodeHTTPResponse) jsc.JSValue { - if (this.flags.socket_closed) { + if (this.flags.socket_closed or this.flags.upgraded or this.raw_response == null) { return .zero; } - return Bun__getNodeHTTPResponseThisValue(this.raw_response == .SSL, this.raw_response.socket()); + return Bun__getNodeHTTPResponseThisValue(this.raw_response.? == .SSL, this.raw_response.?.socket()); } extern "C" fn Bun__getNodeHTTPServerSocketThisValue(bool, *anyopaque) jsc.JSValue; pub fn getServerSocketValue(this: *NodeHTTPResponse) jsc.JSValue { - if (this.flags.socket_closed) { + if (this.flags.socket_closed or this.flags.upgraded or this.raw_response == null) { return .zero; } - - return Bun__getNodeHTTPServerSocketThisValue(this.raw_response == .SSL, this.raw_response.socket()); + return Bun__getNodeHTTPServerSocketThisValue(this.raw_response.? == .SSL, this.raw_response.?.socket()); } pub fn pauseSocket(this: *NodeHTTPResponse) void { log("pauseSocket", .{}); - if (this.flags.socket_closed or this.flags.upgraded or this.raw_response.isConnectRequest()) { + if (this.flags.socket_closed or this.flags.upgraded or this.raw_response == null or this.raw_response.?.isConnectRequest()) { return; } - this.raw_response.pause(); + this.raw_response.?.pause(); } pub fn resumeSocket(this: *NodeHTTPResponse) void { log("resumeSocket", .{}); - if (this.flags.socket_closed or this.flags.upgraded or this.raw_response.isConnectRequest()) { + if (this.flags.socket_closed or this.flags.upgraded or this.raw_response == null or this.raw_response.?.isConnectRequest()) { return; } - this.raw_response.@"resume"(); + this.raw_response.?.@"resume"(); } -const OnBeforeOpen = struct { - this: *NodeHTTPResponse, - socketValue: jsc.JSValue, - globalObject: *jsc.JSGlobalObject, - - pub fn onBeforeOpen(ctx: *OnBeforeOpen, js_websocket: JSValue, socket: *uws.RawWebSocket) void { - Bun__setNodeHTTPServerSocketUsSocketValue(ctx.socketValue, socket.asSocket()); - ServerWebSocket.js.gc.socket.set(js_websocket, ctx.globalObject, ctx.socketValue); - ctx.this.flags.upgraded = true; - defer ctx.this.poll_ref.unref(ctx.globalObject.bunVM()); - switch (ctx.this.raw_response) { - .SSL => ctx.this.raw_response = uws.AnyResponse.init(uws.NewApp(true).Response.castRes(@alignCast(@ptrCast(socket)))), - .TCP => ctx.this.raw_response = uws.AnyResponse.init(uws.NewApp(false).Response.castRes(@alignCast(@ptrCast(socket)))), - } - } -}; - pub fn upgrade(this: *NodeHTTPResponse, data_value: JSValue, sec_websocket_protocol: ZigString, sec_websocket_extensions: ZigString) bool { const upgrade_ctx = this.upgrade_context.context orelse return false; const ws_handler = this.server.webSocketHandler() orelse return false; @@ -206,19 +188,11 @@ pub fn upgrade(this: *NodeHTTPResponse, data_value: JSValue, sec_websocket_proto else this.upgrade_context.sec_websocket_key; - var on_before_open = OnBeforeOpen{ - .this = this, - .socketValue = socketValue, - .globalObject = this.server.globalThis(), - }; - var on_before_open_ptr = WebSocketServerContext.Handler.OnBeforeOpen{ - .ctx = &on_before_open, - .callback = @ptrCast(&OnBeforeOpen.onBeforeOpen), - }; - - this.server.webSocketHandler().?.onBeforeOpen = &on_before_open_ptr; - _ = this.raw_response.upgrade(*ServerWebSocket, ws, websocket_key, sec_websocket_protocol_value, sec_websocket_extensions_value, upgrade_ctx); - + if (this.raw_response) |raw_response| { + this.raw_response = null; + this.flags.upgraded = true; + _ = raw_response.upgrade(*ServerWebSocket, ws, websocket_key, sec_websocket_protocol_value, sec_websocket_extensions_value, upgrade_ctx); + } return true; } pub fn maybeStopReadingBody(this: *NodeHTTPResponse, vm: *jsc.VirtualMachine, thisValue: jsc.JSValue) void { @@ -231,7 +205,9 @@ pub fn maybeStopReadingBody(this: *NodeHTTPResponse, vm: *jsc.VirtualMachine, th const had_ref = this.body_read_ref.has; if (!this.flags.upgraded and !this.flags.socket_closed) { log("clearOnData", .{}); - this.raw_response.clearOnData(); + if (this.raw_response) |raw_response| { + raw_response.clearOnData(); + } } this.body_read_ref.unref(vm); @@ -351,7 +327,9 @@ pub fn setOnAbortedHandler(this: *NodeHTTPResponse) void { } // Don't overwrite WebSocket user data if (!this.flags.upgraded) { - this.raw_response.onTimeout(*NodeHTTPResponse, onTimeout, this); + if (this.raw_response) |raw_response| { + raw_response.onTimeout(*NodeHTTPResponse, onTimeout, this); + } } // detach and this.upgrade_context.preserveWebSocketHeadersIfNeeded(); @@ -394,8 +372,10 @@ pub fn getBufferedAmount(this: *const NodeHTTPResponse, _: *jsc.JSGlobalObject) if (this.flags.request_has_completed or this.flags.socket_closed) { return jsc.JSValue.jsNumber(0); } - - return jsc.JSValue.jsNumber(this.raw_response.getBufferedAmount()); + if (this.raw_response) |raw_response| { + return jsc.JSValue.jsNumber(raw_response.getBufferedAmount()); + } + return jsc.JSValue.jsNumber(0); } pub fn jsRef(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { @@ -441,12 +421,12 @@ pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, cal return globalObject.ERR(.STREAM_ALREADY_FINISHED, "Stream is already ended", .{}).throw(); } - if (this.flags.socket_closed) { + if (this.flags.socket_closed or this.flags.upgraded or this.raw_response == null) { // We haven't emitted the "close" event yet. return .js_undefined; } - const state = this.raw_response.state(); + const state = this.raw_response.?.state(); try handleEndedIfNecessary(state, globalObject); const status_code_value: JSValue = if (arguments.len > 0) arguments[0] else .js_undefined; @@ -484,7 +464,7 @@ pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, cal do_it: { if (status_message_slice.len == 0) { if (HTTPStatusText.get(@intCast(status_code))) |status_message| { - writeHeadInternal(this.raw_response, globalObject, status_message, headers_object_value); + writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value); break :do_it; } } @@ -492,7 +472,7 @@ pub fn writeHead(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, cal const message = if (status_message_slice.len > 0) status_message_slice.slice() else "HM"; const status_message = bun.handleOom(std.fmt.allocPrint(allocator, "{d} {s}", .{ status_code, message })); defer allocator.free(status_message); - writeHeadInternal(this.raw_response, globalObject, status_message, headers_object_value); + writeHeadInternal(this.raw_response.?, globalObject, status_message, headers_object_value); break :do_it; } @@ -511,11 +491,11 @@ pub fn writeContinue(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, if (this.isDone()) { return .js_undefined; } - - const state = this.raw_response.state(); + const raw_response = this.raw_response orelse return .js_undefined; + const state = raw_response.state(); try handleEndedIfNecessary(state, globalObject); - this.raw_response.writeContinue(); + raw_response.writeContinue(); return .js_undefined; } @@ -526,6 +506,9 @@ pub const AbortEvent = enum(u8) { }; fn handleAbortOrTimeout(this: *NodeHTTPResponse, comptime event: AbortEvent, js_value: jsc.JSValue) void { + defer { + if (event == .abort) this.raw_response = null; + } if (this.flags.request_has_completed) { return; } @@ -572,11 +555,11 @@ pub fn onTimeout(this: *NodeHTTPResponse, _: uws.AnyResponse) void { pub fn doPause(this: *NodeHTTPResponse, _: *jsc.JSGlobalObject, _: *jsc.CallFrame, _: jsc.JSValue) bun.JSError!jsc.JSValue { log("doPause", .{}); - if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.ended or this.flags.upgraded) { + if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.ended or this.flags.upgraded or this.raw_response == null) { return .false; } this.flags.is_data_buffered_during_pause = true; - this.raw_response.onData(*NodeHTTPResponse, onBufferRequestBodyWhilePaused, this); + this.raw_response.?.onData(*NodeHTTPResponse, onBufferRequestBodyWhilePaused, this); // TODO: figure out why windows is not emitting EOF with UV_DISCONNECT if (!Environment.isWindows) { @@ -601,11 +584,11 @@ fn drainBufferedRequestBodyFromPause(this: *NodeHTTPResponse, globalObject: *jsc pub fn doResume(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, _: *jsc.CallFrame) jsc.JSValue { log("doResume", .{}); - if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.ended or this.flags.upgraded) { + if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.ended or this.flags.upgraded or this.raw_response == null) { return .false; } this.setOnAbortedHandler(); - this.raw_response.onData(*NodeHTTPResponse, onData, this); + this.raw_response.?.onData(*NodeHTTPResponse, onData, this); this.flags.is_data_buffered_during_pause = false; var result: jsc.JSValue = .true; @@ -642,11 +625,13 @@ pub export fn Bun__NodeHTTPRequest__onResolve(globalObject: *jsc.JSGlobalObject, js.onAbortedSetCached(this_value, globalObject, .zero); } log("clearOnData", .{}); - this.raw_response.clearOnData(); - this.raw_response.clearOnWritable(); - this.raw_response.clearTimeout(); - if (this.raw_response.state().isResponsePending()) { - this.raw_response.endWithoutBody(this.raw_response.state().isHttpConnectionClose()); + if (this.raw_response) |raw_response| { + raw_response.clearOnData(); + raw_response.clearOnWritable(); + raw_response.clearTimeout(); + if (raw_response.state().isResponsePending()) { + raw_response.endWithoutBody(raw_response.state().isHttpConnectionClose()); + } } this.onRequestComplete(); } @@ -669,13 +654,16 @@ pub export fn Bun__NodeHTTPRequest__onReject(globalObject: *jsc.JSGlobalObject, js.onAbortedSetCached(this_value, globalObject, .zero); } log("clearOnData", .{}); - this.raw_response.clearOnData(); - this.raw_response.clearOnWritable(); - this.raw_response.clearTimeout(); - if (!this.raw_response.state().isHttpStatusCalled()) { - this.raw_response.writeStatus("500 Internal Server Error"); + if (this.raw_response) |raw_response| { + raw_response.clearOnData(); + raw_response.clearOnWritable(); + raw_response.clearTimeout(); + if (!raw_response.state().isHttpStatusCalled()) { + raw_response.writeStatus("500 Internal Server Error"); + } + raw_response.endStream(raw_response.state().isHttpConnectionClose()); } - this.raw_response.endStream(this.raw_response.state().isHttpConnectionClose()); + this.onRequestComplete(); } @@ -689,16 +677,18 @@ pub fn abort(this: *NodeHTTPResponse, _: *jsc.JSGlobalObject, _: *jsc.CallFrame) } this.flags.socket_closed = true; - const state = this.raw_response.state(); - if (state.isHttpEndCalled()) { - return .js_undefined; + if (this.raw_response) |raw_response| { + const state = raw_response.state(); + if (state.isHttpEndCalled()) { + return .js_undefined; + } + resumeSocket(this); + log("clearOnData", .{}); + raw_response.clearOnData(); + raw_response.clearOnWritable(); + raw_response.clearTimeout(); + raw_response.endWithoutBody(true); } - resumeSocket(this); - log("clearOnData", .{}); - this.raw_response.clearOnData(); - this.raw_response.clearOnWritable(); - this.raw_response.clearTimeout(); - this.raw_response.endWithoutBody(true); this.onRequestComplete(); return .js_undefined; } @@ -841,11 +831,13 @@ fn writeOrEnd( // // then we haven't gotten the 'close' event yet. // return false; // } - if (this.flags.socket_closed) { + if (this.flags.socket_closed or this.raw_response == null) { return if (is_end) .js_undefined else jsc.JSValue.jsNumber(0); } - const state = this.raw_response.state(); + const raw_response = this.raw_response.?; + + const state = raw_response.state(); if (!state.isResponsePending()) { return globalObject.ERR(.STREAM_WRITE_AFTER_END, "Stream already ended", .{}).throw(); } @@ -937,30 +929,30 @@ fn writeOrEnd( js.onAbortedSetCached(this_value, globalObject, .zero); } - this.raw_response.clearAborted(); - this.raw_response.clearOnWritable(); - this.raw_response.clearTimeout(); + raw_response.clearAborted(); + raw_response.clearOnWritable(); + raw_response.clearTimeout(); this.flags.ended = true; if (!state.isHttpWriteCalled() or bytes.len > 0) { - this.raw_response.end(bytes, state.isHttpConnectionClose()); + raw_response.end(bytes, state.isHttpConnectionClose()); } else { - this.raw_response.endStream(state.isHttpConnectionClose()); + raw_response.endStream(state.isHttpConnectionClose()); } this.onRequestComplete(); return jsc.JSValue.jsNumberFromUint64(bytes.len); } else { const js_this = if (this_value != .zero) this_value else this.getThisValue(); - switch (this.raw_response.write(bytes)) { + switch (raw_response.write(bytes)) { .want_more => |written| { - this.raw_response.clearOnWritable(); + raw_response.clearOnWritable(); js.onWritableSetCached(js_this, globalObject, .js_undefined); return jsc.JSValue.jsNumberFromUint64(written); }, .backpressure => |written| { if (!callback_value.isUndefined()) { js.onWritableSetCached(js_this, globalObject, callback_value.withAsyncContextIfNeeded(globalObject)); - this.raw_response.onWritable(*NodeHTTPResponse, onDrain, this); + raw_response.onWritable(*NodeHTTPResponse, onDrain, this); } return jsc.JSValue.jsNumberFromInt64(-@as(i64, @intCast(@min(written, std.math.maxInt(i64))))); @@ -1024,7 +1016,9 @@ fn clearOnDataCallback(this: *NodeHTTPResponse, thisValue: jsc.JSValue, globalOb } if (!this.flags.socket_closed and !this.flags.upgraded) { log("clearOnData", .{}); - this.raw_response.clearOnData(); + if (this.raw_response) |raw_response| { + raw_response.clearOnData(); + } } if (this.body_read_state != .done) { this.body_read_state = .done; @@ -1044,7 +1038,9 @@ pub fn setOnData(this: *NodeHTTPResponse, thisValue: jsc.JSValue, globalObject: .pending, .done => { if (!this.flags.request_has_completed and !this.flags.socket_closed and !this.flags.upgraded) { log("clearOnData", .{}); - this.raw_response.clearOnData(); + if (this.raw_response) |raw_response| { + raw_response.clearOnData(); + } } this.body_read_state = .done; }, @@ -1055,7 +1051,9 @@ pub fn setOnData(this: *NodeHTTPResponse, thisValue: jsc.JSValue, globalObject: js.onDataSetCached(thisValue, globalObject, value.withAsyncContextIfNeeded(globalObject)); this.flags.hasCustomOnData = true; - this.raw_response.onData(*NodeHTTPResponse, onData, this); + if (this.raw_response) |raw_response| { + raw_response.onData(*NodeHTTPResponse, onData, this); + } this.flags.is_data_buffered_during_pause = false; if (!this.body_read_ref.has) { @@ -1071,8 +1069,8 @@ pub fn write(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, callfra } pub fn flushHeaders(this: *NodeHTTPResponse, _: *jsc.JSGlobalObject, _: *jsc.CallFrame) bun.JSError!jsc.JSValue { - if (!this.flags.socket_closed and !this.flags.upgraded) - this.raw_response.flushHeaders(); + if (!this.flags.socket_closed and !this.flags.upgraded and this.raw_response != null) + this.raw_response.?.flushHeaders(); return .js_undefined; } @@ -1097,11 +1095,11 @@ fn handleCorked(globalObject: *jsc.JSGlobalObject, function: jsc.JSValue, result } pub fn setTimeout(this: *NodeHTTPResponse, seconds: u8) void { - if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.upgraded) { + if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.upgraded or this.raw_response == null) { return; } - this.raw_response.timeout(seconds); + this.raw_response.?.timeout(seconds); } export fn NodeHTTPResponse__setTimeout(this: *NodeHTTPResponse, seconds: jsc.JSValue, globalThis: *jsc.JSGlobalObject) bool { @@ -1110,11 +1108,11 @@ export fn NodeHTTPResponse__setTimeout(this: *NodeHTTPResponse, seconds: jsc.JSV return false; } - if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.upgraded) { + if (this.flags.request_has_completed or this.flags.socket_closed or this.flags.upgraded or this.raw_response == null) { return false; } - this.raw_response.timeout(@intCast(@min(seconds.to(c_uint), 255))); + this.raw_response.?.timeout(@intCast(@min(seconds.to(c_uint), 255))); return true; } @@ -1137,8 +1135,11 @@ pub fn cork(this: *NodeHTTPResponse, globalObject: *jsc.JSGlobalObject, callfram this.ref(); defer this.deref(); - this.raw_response.corked(handleCorked, .{ globalObject, arguments[0], &result, &is_exception }); - + if (this.raw_response) |raw_response| { + raw_response.corked(handleCorked, .{ globalObject, arguments[0], &result, &is_exception }); + } else { + handleCorked(globalObject, arguments[0], &result, &is_exception); + } if (is_exception) { if (result != .zero) { return globalObject.throwValue(result); @@ -1174,7 +1175,6 @@ fn deinit(this: *NodeHTTPResponse) void { comptime { @export(&create, .{ .name = "NodeHTTPResponse__createForJS" }); } -extern "c" fn Bun__setNodeHTTPServerSocketUsSocketValue(jsc.JSValue, ?*anyopaque) void; pub export fn Bun__NodeHTTPResponse_onClose(response: *NodeHTTPResponse, js_value: jsc.JSValue) void { response.onAbort(js_value); @@ -1186,7 +1186,6 @@ pub export fn Bun__NodeHTTPResponse_setClosed(response: *NodeHTTPResponse) void const string = []const u8; -const WebSocketServerContext = @import("./WebSocketServerContext.zig"); const std = @import("std"); const bun = @import("bun"); diff --git a/src/bun.js/api/server/ServerWebSocket.zig b/src/bun.js/api/server/ServerWebSocket.zig index a244b2dc29..a47a4795ce 100644 --- a/src/bun.js/api/server/ServerWebSocket.zig +++ b/src/bun.js/api/server/ServerWebSocket.zig @@ -81,18 +81,11 @@ pub fn onOpen(this: *ServerWebSocket, ws: uws.AnyWebSocket) void { this.#flags.opened = false; if (onOpenHandler.isEmptyOrUndefinedOrNull()) { - if (bun.take(&this.#handler.onBeforeOpen)) |on_before_open| { - // Only create the "this" value if needed. - on_before_open.callback(on_before_open.ctx, this.#this_value.tryGet() orelse .js_undefined, ws.raw()); - } return; } const this_value = this.#this_value.tryGet() orelse .js_undefined; var args = [_]JSValue{this_value}; - if (bun.take(&this.#handler.onBeforeOpen)) |on_before_open| { - on_before_open.callback(on_before_open.ctx, this_value, ws.raw()); - } const loop = vm.eventLoop(); loop.enter(); diff --git a/src/bun.js/api/server/WebSocketServerContext.zig b/src/bun.js/api/server/WebSocketServerContext.zig index 694715916f..e98ead2c11 100644 --- a/src/bun.js/api/server/WebSocketServerContext.zig +++ b/src/bun.js/api/server/WebSocketServerContext.zig @@ -28,13 +28,6 @@ pub const Handler = struct { globalObject: *jsc.JSGlobalObject = undefined, active_connections: usize = 0, - /// Only used by NodeHTTPResponse. - /// - /// Before we call into JavaScript and after the WebSocket is upgraded, we need to call a function in NodeHTTPResponse. - /// - /// This is per-ServerWebSocket data, so it needs to be null'd on usage. - onBeforeOpen: ?*OnBeforeOpen = null, - /// used by publish() flags: packed struct(u8) { ssl: bool = false, @@ -42,11 +35,6 @@ pub const Handler = struct { _: u6 = 0, } = .{}, - pub const OnBeforeOpen = struct { - ctx: *anyopaque, - callback: *const fn (*anyopaque, this_value: jsc.JSValue, socket: *uws.RawWebSocket) void, - }; - pub fn runErrorCallback(this: *const Handler, vm: *jsc.VirtualMachine, globalObject: *jsc.JSGlobalObject, error_value: jsc.JSValue) void { const onError = this.onError; if (!onError.isEmptyOrUndefinedOrNull()) { diff --git a/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp b/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp index 9006ddd1c0..1e5700262b 100644 --- a/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/bun.js/bindings/node/JSNodeHTTPServerSocket.cpp @@ -28,6 +28,10 @@ const JSC::ClassInfo JSNodeHTTPServerSocket::s_info = { "NodeHTTPServerSocket"_s JSNodeHTTPServerSocket* JSNodeHTTPServerSocket::create(JSC::VM& vm, JSC::Structure* structure, us_socket_t* socket, bool is_ssl, WebCore::JSNodeHTTPResponse* response) { + if (socket && us_socket_is_closed(is_ssl, socket)) { + // dont attach closed socket because the callback will never be called + socket = nullptr; + } auto* object = new (JSC::allocateCell(vm)) JSNodeHTTPServerSocket(vm, structure, socket, is_ssl, response); object->finishCreation(vm); return object; @@ -319,12 +323,6 @@ extern "C" JSC::EncodedJSValue Bun__getNodeHTTPServerSocketThisValue(bool is_ssl return JSValue::encode(getNodeHTTPServerSocket(socket)); } -extern "C" void Bun__setNodeHTTPServerSocketUsSocketValue(JSC::EncodedJSValue thisValue, us_socket_t* socket) -{ - auto* response = jsCast(JSValue::decode(thisValue)); - response->socket = socket; -} - extern "C" JSC::EncodedJSValue Bun__createNodeHTTPServerSocketForClientError(bool isSSL, us_socket_t* us_socket, Zig::GlobalObject* globalObject) { auto& vm = globalObject->vm(); diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 61cc987bc5..81d627e5f5 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -303,7 +303,7 @@ Server.prototype.closeAllConnections = function () { Server.prototype.closeIdleConnections = function () { const server = this[serverSymbol]; - server.closeIdleConnections(); + server?.closeIdleConnections(); }; Server.prototype.close = function (optionalCallback?) {