Skip to content
Closed
Show file tree
Hide file tree
Changes from 6 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
9 changes: 9 additions & 0 deletions src/js/internal/test/binding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ function internalBinding(name: string) {
return { TCP: TestTCPWrap, constants: { SOCKET: 0, SERVER: 1 } };
case "util":
return { isInsideNodeModules };
// Vendored tls engine tests construct binding.SecureContext (or replace it)
// before requiring node:tls; Bun's SecureContext class is the equivalent
// native surface.
Comment thread
robobun marked this conversation as resolved.
Resolved
case "crypto":
return { SecureContext: require("node:tls").SecureContext };
// BoringSSL does not compile in OpenSSL's SSL_trace(), so a Node built
// against it reports HAVE_SSL_TRACE = false; --trace-tls tests skip.
Comment thread
robobun marked this conversation as resolved.
Resolved
case "tls_wrap":
return { HAVE_SSL_TRACE: false };
case "cares_wrap":
// Only the pure IP-normalizer the vendored tls/dns tests reach for; the
// resolver surface lives in node:dns.
Expand Down
1 change: 1 addition & 0 deletions src/js/internal/timers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function getTimerDuration(msecs, name) {
}

export default {
TIMEOUT_MAX,
// For hiding Timeouts on other internals. A registered symbol so the node
// test harness's --expose-internals shim ("internal/timers" virtual module
// in test/js/node/test/common/index.js) can hand the same symbol to ported
Expand Down
38 changes: 24 additions & 14 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,13 @@ const kSetTOS = Symbol("kSetTOS");
const kSetKeepAlive = Symbol("kSetKeepAlive");
const kSyncWriteFd = Symbol("kSyncWriteFd");
const kSetKeepAliveInitialDelay = Symbol("kSetKeepAliveInitialDelay");

// The keepalive delay is stored in whole seconds, as node does for libuv. Bun's
// native setKeepAlive takes an int32 of milliseconds, so convert at the call
// and clamp: node never rejects a large delay, it passes the seconds on.
Comment thread
robobun marked this conversation as resolved.
Resolved
function keepAliveDelayMs(seconds) {
return MathMin(seconds * 1000, 2147483647);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
const kConnectOptions = Symbol("connect-options");
const kAttach = Symbol("kAttach");
const kCloseRawConnection = Symbol("kCloseRawConnection");
Expand Down Expand Up @@ -503,7 +510,7 @@ const SocketHandlers: SocketHandler = {
}

if (self[kSetKeepAlive]) {
socket.setKeepAlive(true, self[kSetKeepAliveInitialDelay]);
socket.setKeepAlive(true, keepAliveDelayMs(self[kSetKeepAliveInitialDelay]));
}

// A TOS value set before the connection existed (setTypeOfService before
Expand Down Expand Up @@ -1216,7 +1223,7 @@ function onconnection(err, clientHandle) {
if (self.keepAlive && clientHandle.setKeepAlive) {
_socket[kSetKeepAlive] = true;
_socket[kSetKeepAliveInitialDelay] = self.keepAliveInitialDelay;
clientHandle.setKeepAlive(true, self.keepAliveInitialDelay);
clientHandle.setKeepAlive(true, keepAliveDelayMs(self.keepAliveInitialDelay));
}

self._connections++;
Expand Down Expand Up @@ -1585,9 +1592,9 @@ function Socket(options?) {

this[kSetNoDelay] = Boolean(noDelay);
this[kSetKeepAlive] = Boolean(keepAlive);
// Bun's native _handle.setKeepAlive takes milliseconds (it is the public
// Bun.Socket), so store ms here. Node stores seconds because libuv does.
this[kSetKeepAliveInitialDelay] = MathMax(0, ~~keepAliveInitialDelay);
// Node stores whole seconds here (libuv's unit); keepAliveDelayMs converts
// for Bun's native _handle.setKeepAlive.
Comment thread
robobun marked this conversation as resolved.
Resolved
this[kSetKeepAliveInitialDelay] = MathMax(0, ~~(keepAliveInitialDelay / 1000));

this[khandlers] = SocketHandlers2;
this.bytesRead = 0;
Expand Down Expand Up @@ -1873,7 +1880,7 @@ Socket.prototype[kAttach] = function (port, socket) {
}

if (this[kSetKeepAlive]) {
socket.setKeepAlive(true, this[kSetKeepAliveInitialDelay]);
socket.setKeepAlive(true, keepAliveDelayMs(this[kSetKeepAliveInitialDelay]));
}

if (!this[kupgraded]) {
Expand Down Expand Up @@ -2149,7 +2156,10 @@ Socket.prototype.connect = function connect(...args) {
Socket.prototype[kReinitializeHandle] = function reinitializeHandle(handle) {
this._handle?.close();

this._handle = handle;
// Node's TLSSocket override creates the replacement handle itself when none
// is passed (net.Socket's version requires one); Bun's TLS sockets share
// this method, so default it here for both flavors.
Comment thread
robobun marked this conversation as resolved.
Resolved
this._handle = handle ?? newDetachedSocket(typeof this[bunTlsSymbol] === "function");
this._handle[owner_symbol] = this;

initSocketHandle(this);
Expand Down Expand Up @@ -2592,10 +2602,10 @@ Socket.prototype.resetAndDestroy = function resetAndDestroy() {

Socket.prototype.setKeepAlive = function setKeepAlive(enable = false, initialDelayMsecs = 0) {
enable = Boolean(enable);
// Bun's native _handle.setKeepAlive takes milliseconds; the ms→seconds
// conversion for TCP_KEEPIDLE lives in the native binding. Clamp to 0 so
// negatives and ~~ overflow match Node's no-validate behavior.
const initialDelay = MathMax(0, ~~initialDelayMsecs);
// Node truncates to whole seconds (libuv's TCP_KEEPIDLE unit) and stores
// seconds in kSetKeepAliveInitialDelay; keepAliveDelayMs converts for the
// native call.
Comment thread
robobun marked this conversation as resolved.
Resolved
const initialDelay = MathMax(0, ~~(initialDelayMsecs / 1000));

if (!this._handle) {
this[kSetKeepAlive] = enable;
Expand All @@ -2608,7 +2618,7 @@ Socket.prototype.setKeepAlive = function setKeepAlive(enable = false, initialDel
if (enable !== this[kSetKeepAlive] || (enable && this[kSetKeepAliveInitialDelay] !== initialDelay)) {
this[kSetKeepAlive] = enable;
this[kSetKeepAliveInitialDelay] = initialDelay;
this._handle.setKeepAlive(enable, initialDelay);
this._handle.setKeepAlive(enable, keepAliveDelayMs(initialDelay));
}
return this;
};
Expand Down Expand Up @@ -3379,7 +3389,7 @@ function afterConnect(status, handle, req, readable, writable) {
}

if (self[kSetKeepAlive] && self._handle.setKeepAlive) {
self._handle.setKeepAlive(true, self[kSetKeepAliveInitialDelay]);
self._handle.setKeepAlive(true, keepAliveDelayMs(self[kSetKeepAliveInitialDelay]));
}

self.emit("connect");
Expand Down Expand Up @@ -3522,7 +3532,7 @@ function Server(options?, connectionListener?) {
// https://github.com/nodejs/node/blob/843dc5f0d5ad/lib/net.js#L1880
this.allowHalfOpen = allowHalfOpen;
this.keepAlive = Boolean(keepAlive);
this.keepAliveInitialDelay = MathMax(0, ~~keepAliveInitialDelay);
this.keepAliveInitialDelay = MathMax(0, ~~(keepAliveInitialDelay / 1000));
this.highWaterMark = highWaterMark;
this.pauseOnConnect = Boolean(pauseOnConnect);
this.noDelay = Boolean(noDelay);
Expand Down
41 changes: 27 additions & 14 deletions src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,13 @@ const SUPPORTED_ECDH_GROUPS = new Set([
function validateSecureContextOptions(options) {
const {
ciphers,
key,
passphrase,
ecdhCurve,
minVersion,
maxVersion,
privateKeyIdentifier,
privateKeyEngine,
sessionTimeout,
sigalgs,
ticketKeys,
Expand All @@ -218,6 +221,30 @@ function validateSecureContextOptions(options) {
validateString(sigalgs, "options.sigalgs");
if (sigalgs === "") throw $ERR_INVALID_ARG_VALUE("options.sigalgs", sigalgs);
}
// BoringSSL has no OpenSSL ENGINE support, so engine-backed keys fail like Node's
// no-engine builds; validation ordering matches Node:
// https://github.com/nodejs/node/blob/614050b657e9757c1097aa85f92f2cb51149dc0d/lib/internal/tls/secure-context.js#L221
Comment thread
robobun marked this conversation as resolved.
Resolved
if (privateKeyIdentifier !== undefined && privateKeyIdentifier !== null) {
if (privateKeyEngine === undefined || privateKeyEngine === null) {
// Engine is required when privateKeyIdentifier is present
throw $ERR_INVALID_ARG_VALUE("options.privateKeyEngine", privateKeyEngine);
}
if (key) {
// Both data key and engine key can't be set at the same time
throw $ERR_INVALID_ARG_VALUE("options.privateKeyIdentifier", privateKeyIdentifier);
}
if (typeof privateKeyIdentifier === "string" && typeof privateKeyEngine === "string") {
throw $ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED("Custom engines not supported by this OpenSSL");
} else if (typeof privateKeyIdentifier !== "string") {
throw $ERR_INVALID_ARG_TYPE(
"options.privateKeyIdentifier",
["string", "null", "undefined"],
privateKeyIdentifier,
);
} else {
throw $ERR_INVALID_ARG_TYPE("options.privateKeyEngine", ["string", "null", "undefined"], privateKeyEngine);
}
}
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
if (ecdhCurve !== undefined) {
validateString(ecdhCurve, "options.ecdhCurve");
if (ecdhCurve !== "auto") {
Expand Down Expand Up @@ -638,20 +665,6 @@ var InternalSecureContext = class SecureContext {
throw new TypeError("servername argument must be an string");
if (options.secureOptions != null && typeof options.secureOptions !== "number")
throw new TypeError("secureOptions argument must be an number");
const privateKeyIdentifier = options.privateKeyIdentifier;
if (!$isUndefinedOrNull(privateKeyIdentifier)) {
const privateKeyEngine = options.privateKeyEngine;
if ($isUndefinedOrNull(privateKeyEngine))
throw $ERR_INVALID_ARG_VALUE("options.privateKeyEngine", privateKeyEngine);
if (typeof privateKeyEngine !== "string")
throw $ERR_INVALID_ARG_TYPE("options.privateKeyEngine", ["string", "null", "undefined"], privateKeyEngine);
if (typeof privateKeyIdentifier !== "string")
throw $ERR_INVALID_ARG_TYPE(
"options.privateKeyIdentifier",
["string", "null", "undefined"],
privateKeyIdentifier,
);
}
}
const requestedCiphers = options?.ciphers;
if (requestedCiphers && StringPrototypeIncludes.$call(requestedCiphers, "TLS_")) {
Expand Down
5 changes: 4 additions & 1 deletion src/runtime/socket/tls_socket_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,13 +423,16 @@ pub(super) fn set_max_send_fragment(
return Err(global.throw(format_args!("Expected size to be a number")));
}
let size = arg.coerce_to_int64(global)?;
if !(512..=16384).contains(&size) {
if size < 0 {
return Ok(JSValue::FALSE);
}

let Some(ssl_ptr) = this.socket.get().ssl() else {
return Ok(JSValue::FALSE);
};
// No range gate here: Node returns SSL_set_max_send_fragment's own verdict,
// and BoringSSL clamps out-of-range sizes into [512, 16384] and reports
// success (vendor/boringssl/ssl/ssl_lib.cc, SSL_set_max_send_fragment).
Comment thread
robobun marked this conversation as resolved.
Resolved
Ok(JSValue::from(
ffi::SSL_set_max_send_fragment(
boringssl::SSL::opaque_ref(ssl_ptr),
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/timer/ImmediateObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ use super::{Kind, TimerObjectInternals};
super::impl_timer_object!(ImmediateObject, ImmediateObject, "Immediate");

impl ImmediateObject {
/// Hook called by the `dispose` host function `impl_timer_object!` generates.
/// An Immediate has no `_idleTimeout`, so there is nothing to record.
Comment thread
robobun marked this conversation as resolved.
Resolved
#[inline]
pub(crate) fn before_explicit_cancel(&self, _global: &JSGlobalObject, _this_value: JSValue) {}

pub(crate) fn init(
global: &JSGlobalObject,
id: i32,
Expand Down
13 changes: 13 additions & 0 deletions src/runtime/timer/TimeoutObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,23 @@ impl TimeoutObject {

#[bun_jsc::host_fn(method)]
pub fn close(this: &Self, global: &JSGlobalObject, frame: &CallFrame) -> JsResult<JSValue> {
this.before_explicit_cancel(global, frame.this());
this.internals.cancel(global.bun_vm_ptr());
Ok(frame.this())
}

/// Node's `unenroll`: a timer cancelled by `clearTimeout`/`clearInterval`,
/// `close()` or `[Symbol.dispose]` reads back `_idleTimeout === -1`. Not for
/// the natural-fire path, where a fired timer keeps its duration.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub(crate) fn mark_unenrolled(this_value: JSValue, global: &JSGlobalObject) {
js::idle_timeout_set_cached(this_value, global, JSValue::js_number(-1.0));
}

/// Hook called by the `dispose` host function `impl_timer_object!` generates.
pub(crate) fn before_explicit_cancel(&self, global: &JSGlobalObject, this_value: JSValue) {
Self::mark_unenrolled(this_value, global);
}

// Cached-property getters/setters — codegen passes `this_value` (the JS
// wrapper) so the cached `WriteBarrier` slot on the C++ side can be read/written.
// Signature does not match the standard `host_fn(getter/setter)` shape; the
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/timer/Timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,12 @@ impl All {
};

let Some(timer) = timer else { return Ok(()) };
if kind != Kind::SetImmediate
// SAFETY: timer points to a live TimerObjectInternals
&& let Some(js_timer) = unsafe { (*timer).this_value.get().try_get() }
{
TimeoutObject::mark_unenrolled(js_timer, global_this);
}
// SAFETY: timer points to a live TimerObjectInternals
unsafe { (*timer).cancel(vm) };
Ok(())
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,9 @@ macro_rules! impl_timer_object {
pub fn dispose(
this: &Self,
global: &::bun_jsc::JSGlobalObject,
_frame: &::bun_jsc::CallFrame,
frame: &::bun_jsc::CallFrame,
) -> ::bun_jsc::JsResult<::bun_jsc::JSValue> {
this.before_explicit_cancel(global, frame.this());
this.internals.cancel(global.bun_vm_ptr());
Ok(::bun_jsc::JSValue::UNDEFINED)
}
Expand Down
29 changes: 28 additions & 1 deletion test/js/node/net/node-net-server.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { realpathSync } from "fs";
import { bunEnv, bunExe, tempDir } from "harness";
import { AddressInfo, createServer, Server, Socket } from "net";
import { AddressInfo, connect, createServer, Server, Socket } from "net";
import { createTest } from "node-harness";
import { once } from "node:events";
import { tmpdir } from "os";
Expand Down Expand Up @@ -662,3 +662,30 @@ it("server.unref() on a pipe/unix-socket listener lets the process exit", async
expect(exitCode === 0 ? "" : stderr).toBe("");
expect(exitCode).toBe(0);
});

describe("keepAlive initial delay", () => {
// node stores the delay in whole seconds and passes it on to libuv without a
// ceiling; the native handle takes int32 milliseconds, so the conversion
// back must clamp instead of throwing inside the connection callbacks.
it("a delay above int32 milliseconds is stored in seconds and does not throw", async () => {
const delay = 3_000_000_000;
await using server = createServer({ keepAlive: true, keepAliveInitialDelay: delay });
expect(server.keepAliveInitialDelay).toBe(delay / 1000);
server.on("connection", socket => socket.end("ok"));
server.listen(0, "127.0.0.1");
await once(server, "listening");

const client = connect({ port: (server.address() as AddressInfo).port, host: "127.0.0.1" });
await once(client, "connect");
expect(client.setKeepAlive(true, delay)).toBe(client);
const [data] = await once(client, "data");
expect(String(data)).toBe("ok");
await once(client, "close");
});

it("sub-second delays truncate to whole seconds like node", async () => {
await using server = createServer({ keepAlive: true, keepAliveInitialDelay: 1999 });
expect(server.keepAliveInitialDelay).toBe(1);
expect(createServer({ keepAliveInitialDelay: 999 }).keepAliveInitialDelay).toBe(0);
});
});
18 changes: 17 additions & 1 deletion test/js/node/test/common/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,23 @@ function installBunExposeInternalsShim() {
const normalizedArgsSymbol = Object.getOwnPropertySymbols(probe).find(
s => s.description === "normalizedArgs",
);
return { loader: "object", exports: { normalizedArgsSymbol } };
// The per-socket option symbols are module-private in Bun's node:net,
// like normalizedArgsSymbol; recover the real ones from a probe
// socket (the constructor always sets them) and the prototype.
const socketProbe = new net.Socket();
const bySymbolName = name =>
Object.getOwnPropertySymbols(socketProbe).find(s => s.description === name) ??
Object.getOwnPropertySymbols(net.Socket.prototype).find(s => s.description === name);
return {
loader: "object",
exports: {
normalizedArgsSymbol,
kSetNoDelay: bySymbolName("kSetNoDelay"),
kSetKeepAlive: bySymbolName("kSetKeepAlive"),
kSetKeepAliveInitialDelay: bySymbolName("kSetKeepAliveInitialDelay"),
kReinitializeHandle: bySymbolName("kReinitializeHandle"),
},
};
});
// node's internal/options: map the few CLI options vendored http tests ask
// about onto the equivalent runtime values. Unknown options return undefined.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Flags: --expose-internals
'use strict';
const common = require('../common');

if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
// Monkey-patch SecureContext
const { internalBinding } = require('internal/test/binding');
const binding = internalBinding('crypto');
const NativeSecureContext = binding.SecureContext;

binding.SecureContext = function() {
const rv = new NativeSecureContext();
rv.setClientCertEngine = undefined;
return rv;
};

const tls = require('tls');

{
assert.throws(
() => { tls.createSecureContext({ clientCertEngine: 'Cannonmouth' }); },
{
code: 'ERR_CRYPTO_CUSTOM_ENGINE_NOT_SUPPORTED',
message: 'Custom engines not supported by this OpenSSL'
}
);
}
Loading
Loading