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
55 changes: 46 additions & 9 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,17 +494,29 @@ Server.prototype.unref = function () {
return this;
};

// Node.js's ConnectionsList (what closeAllConnections/closeIdleConnections walk)
// is parser-keyed, and freeParser() removes a connection once its 'connect' or
// 'upgrade' request has been received in full: at the handoff for CONNECT and
// body-less upgrades, after the body for an upgrade that carries one.
// releaseServerParserShim() nulls socket.parser at the handoff in every case,
// so a handed-off socket still counts as listed while that body is arriving
// (hasIncompleteRequest stays set until the message completes).
Comment thread
robobun marked this conversation as resolved.
function isOutsideConnectionsList(socket) {
return socket.parser == null && !socket[kHandle]?.hasIncompleteRequest;
}

Server.prototype.closeAllConnections = function () {
closeAllHttp1Connections(this);
const server = this[serverSymbol];
if (!server) {
const connections = this[kTrackedConnections];
if (!connections) {
return;
}
this[serverSymbol] = undefined;
clearInterval(this[kConnectionsCheckingInterval]);
this.listening = false;

server.stop(true);
for (const socket of connections) {
if (isOutsideConnectionsList(socket)) {
continue;
}
socket.destroy();
}
Comment thread
robobun marked this conversation as resolved.
};

Server.prototype.getConnections = function (callback) {
Expand All @@ -519,8 +531,33 @@ Server.prototype.getConnections = function (callback) {

Server.prototype.closeIdleConnections = function () {
closeIdleHttp1Connections(this);
const server = this[serverSymbol];
server?.closeIdleConnections();
const connections = this[kTrackedConnections];
if (!connections) {
return;
}
for (const socket of connections) {
if (isOutsideConnectionsList(socket)) {
continue;
}
const message = socket._httpMessage;
if (message && !message.finished) {
continue;
}
// Deliberately unlike Node.js, which destroys the connection here and
// drops the queued responses: a pipelined queue counts as in flight, as it
// does for the native idle sweep that close() runs.
Comment thread
robobun marked this conversation as resolved.
if (socket[kPipelinedResponses]?.length) {
continue;
}
// Node.js's ConnectionsList.idle() also skips parsers whose
// last_message_start_ is set, i.e. a request head or body is still being
// received (a connection that has not sent its first request yet counts
// too); hasIncompleteRequest is the native handle's view of the same state.
Comment thread
robobun marked this conversation as resolved.
if (socket[kHandle]?.hasIncompleteRequest) {
continue;
}
socket.destroy();
}
Comment thread
claude[bot] marked this conversation as resolved.
};

Server.prototype.close = function (optionalCallback?) {
Expand Down
21 changes: 21 additions & 0 deletions src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,27 @@ bool JSNodeHTTPServerSocket::isRequestTimedOut(uint64_t headersTimeoutMs, uint64
return isRequestTimedOutImpl<false>(socket, headersTimeoutMs, requestTimeoutMs);
}

template<bool SSL>
static bool hasIncompleteRequestImpl(us_socket_t* socket)
{
auto* httpResponseData = reinterpret_cast<uWS::NodeHttpResponseData<SSL>*>(us_socket_ext(socket));
if (httpResponseData->isConnectRequest) {
return false;
}
return httpResponseData->lastMessageStartMs != 0;
}

bool JSNodeHTTPServerSocket::hasIncompleteRequest() const
{
if (!socket || upgraded || us_socket_is_closed(socket)) {
return false;
}
if (is_ssl) {
return hasIncompleteRequestImpl<true>(socket);
}
return hasIncompleteRequestImpl<false>(socket);
}

bool JSNodeHTTPServerSocket::isAuthorized() const
{
// is secure means that tls was established successfully
Expand Down
4 changes: 4 additions & 0 deletions src/jsc/bindings/node/JSNodeHTTPServerSocket.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject {
* (both in milliseconds; 0 disables the respective check). */
bool isRequestTimedOut(uint64_t headersTimeoutMs, uint64_t requestTimeoutMs) const;

/* node:http server compat: whether a request message is currently being
* received on this connection (Node's parser.last_message_start_ != 0). */
Comment thread
robobun marked this conversation as resolved.
bool hasIncompleteRequest() const;

/* node:http server compat - HTTP/1.1 pipelining. Responses for requests
* that were parsed while an earlier response on this connection was still
* in flight are queued here (in arrival order) and become the connection's
Expand Down
11 changes: 11 additions & 0 deletions src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ using namespace WebCore;
JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterOnClose);
JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterOnDrain);
JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterClosed);
JSC_DECLARE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterHasIncompleteRequest);
JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnClose);
JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnDrain);
JSC_DECLARE_CUSTOM_SETTER(jsNodeHttpServerSocketSetterOnData);
Expand Down Expand Up @@ -60,6 +61,7 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] =
{ "ondata"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterOnData, jsNodeHttpServerSocketSetterOnData } },
{ "bytesWritten"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterBytesWritten, noOpSetter } },
{ "closed"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterClosed, noOpSetter } },
{ "hasIncompleteRequest"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterHasIncompleteRequest, noOpSetter } },
{ "response"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterResponse, noOpSetter } },
{ "duplex"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterDuplex, jsNodeHttpServerSocketSetterDuplex } },
{ "remoteAddress"_s, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | JSC::PropertyAttribute::ReadOnly), JSC::NoIntrinsic, { JSC::HashTableValue::GetterSetterType, jsNodeHttpServerSocketGetterRemoteAddress, noOpSetter } },
Expand Down Expand Up @@ -513,6 +515,15 @@ JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterClosed, (JSC::JSGlobalObjec
return JSValue::encode(JSC::jsBoolean(thisObject->isClosed()));
}

JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterHasIncompleteRequest, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName propertyName))
{
auto* thisObject = dynamicDowncast<JSNodeHTTPServerSocket>(JSC::JSValue::decode(thisValue));
if (!thisObject) [[unlikely]] {
return JSValue::encode(JSC::jsUndefined());
}
return JSValue::encode(JSC::jsBoolean(thisObject->hasIncompleteRequest()));
}

JSC_DEFINE_CUSTOM_GETTER(jsNodeHttpServerSocketGetterBytesWritten, (JSC::JSGlobalObject * globalObject, JSC::EncodedJSValue thisValue, JSC::PropertyName propertyName))
{
auto* thisObject = dynamicDowncast<JSNodeHTTPServerSocket>(JSC::JSValue::decode(thisValue));
Expand Down
2 changes: 0 additions & 2 deletions test/js/bun/http/node-http-halfclose-midupload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ async function runTeardownStages(bind: string | undefined, url: (port: number) =
try {
expect(await withTimeout(writeResult.promise, "write-after-end")).toBeTrue();
await withTimeout(fetchSettled, "fetch-settled");
// Not server.closeAllConnections(): bun's implementation also stops the
// server, which makes the disposal/close below reject.
socket?.destroy();
await withTimeout(connectionClosed.promise, "connection-closed");
const serverClosed = new Promise<void>((resolve, reject) => server.close(err => (err ? reject(err) : resolve())));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,9 @@ const { expect } = createTest(import.meta.path);
const server = http.createServer();
await once(server.listen(0), "listening");
expect(server.listening).toBe(true);
// closeAllConnections() destroys the connections; it does not stop listening.
server.closeAllConnections();
expect(server.listening).toBe(true);
server.close();
expect(server.listening).toBe(false);
await once(server, "close");
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,10 @@ const { expect } = createTest(import.meta.path);
const { kConnectionsCheckingInterval } = require("_http_server");
const server = http.createServer();
await once(server.listen(0), "listening");
expect(server[kConnectionsCheckingInterval]._destroyed).toBe(false);
// Only close() tears the interval down; closeAllConnections() keeps listening.
server.closeAllConnections();
expect(server[kConnectionsCheckingInterval]._destroyed).toBe(false);
server.close();
expect(server[kConnectionsCheckingInterval]._destroyed).toBe(true);
await once(server, "close");
1 change: 1 addition & 0 deletions test/js/first_party/ws/ws.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -771,6 +771,7 @@ it("Server should be able to send empty pings", async () => {
return await promise;
} finally {
httpServer.closeAllConnections();
httpServer.close();
}
}
{
Expand Down
Loading