Bun.serve: validate server.timeout() seconds argument

[robobun]

Problem

• server.timeout(req, seconds) only checked is_number() and converted with to_u32(). NaN, negatives and fractions below 1 became 0, which clears the timeout. Anything above 255 was clamped by RequestContext::set_timeout (seconds.min(255) as u8), so server.timeout(req, 3600) is a 4 min 16 s cutoff with no diagnostic.
• The idleTimeout option validates the same quantity and throws for all of these. The method accepted them.
• Cause: timeout in src/runtime/server/server_body.rs. The NodeHTTPResponse arm also used (value % 255) as u8, which maps 255 to 0 (disabled).

Fix

• Require an integer (is_any_int(), else TypeError: timeout() expects an integer number of seconds) in 0..=255 (else RangeError [ERR_OUT_OF_RANGE]: The value of "seconds" is out of range. It must be >= 0 and <= 255. Received N, the same shape options.port uses). The validated u8 goes to both arms, so the % 255 is gone.
• Negatives are rejected, not clamped to 0: 0 is the documented way to disable the timeout.
• Docs and types now state the 0..=255 range for server.timeout().
• Verified: test/js/bun/http/serve.test.ts (server.timeout() validates its seconds argument, ... does not silently disable the idle timeout on invalid input; both fail on 1.4.3). Also every other timeout test in that file and serve-http2-lifecycle.test.ts.

Background

• The per-socket idle timer in uSockets is a u8 count of seconds on a 4 s granularity wheel, which is why idleTimeout caps at 255. server.timeout(req, n) re-arms that same timer for one request's socket, so it has the same range.
• set_timeout(0) calls clear_timeout(): zero means "never time out", not "time out now".

Notes

• Rebased onto current main on 2026-09-06. The earlier revision threw TypeError for out-of-range values too; the range failure is now a RangeError with code: "ERR_OUT_OF_RANGE", and the test pins the error class with each message.
• Observed on 1.4.3 with a raw TCP client against a hung handler and idleTimeout: 30: timeout(req, 256 | 1000 | 3600 | Infinity) all close the connection after ~256 s, timeout(req, -1 | NaN) never close it, timeout(req, 0.4) never closes it.
• Behavior change: values that used to truncate or clamp (1.9 to 1 s, 300 to 255 s) now throw. That is the point of the change, and it matches what the idleTimeout option already does for the same numbers.
• server.timeout() on a unix-socket server still returns early without validating further (unchanged).

[robobun]

^{Updated 4:52 PM PT - Sep 6th, 2026}

❌ @robobun, your commit 7bc1713 has 3 failures in Build #111801 (All Failures):

• test/js/web/streams/native-source-onclose-leak.test.ts - 1 failing in the parallel batch on 🍎 any aarch64
• test/js/bun/http/serve-body-leak.test.ts - code 1 on 🍎 any aarch64
• test/js/node/test/parallel/test-crypto-dh-leak.js - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 34958

That installs a local version of the PR into your bun-34958 executable, so you can run:

bun-34958 --bun

[robobun]

Reproduced with a raw-socket probe against a hung handler: with idleTimeout set, server.timeout(req, NaN | -1 | 0.4) cleared the per-request timeout and the connection was never reaped. Both new tests in test/js/bun/http/serve.test.ts fail on stock 1.4.x and pass with this change. PR: #34958.

CI (build 111801): serve.test.ts and every other file that calls server.timeout() pass on all lanes. The build is red on three files that do not call server.timeout() and that are red on other open PR builds from the same base:

• test/js/node/test/parallel/test-crypto-dh-leak.js (x64-asan, RSS threshold)
• test/js/bun/http/serve-body-leak.test.ts (darwin aarch64, hook timeout; also red on builds 111836, 111820, 111805, 111784)
• test/js/web/streams/native-source-onclose-leak.test.ts (darwin aarch64, GC count; also red on builds 111820, 111784, 111744)

Ready for maintainer review.

[coderabbitai]

Walkthrough

Changes

Server timeout behavior

Layer / File(s)	Summary
Timeout argument validation src/runtime/server/server_body.rs	NewServer::timeout now requires integer values in the inclusive range 0..=255 and applies the validated value without modulo normalization.
Timeout regression coverage test/js/bun/http/serve.test.ts	Concurrent tests cover invalid argument messages and verify that invalid input does not disable idle-timeout connection reaping.

Suggested reviewers: jarred-sumner, cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description clearly explains the problem, the validation fix, behavior changes, background, and verification steps. It does not use the template headings exactly, but it provides the required info…
Title check	✅ Passed	The title is concise, specific, and accurately identifies the main change: validation of the seconds argument for Bun.serve server.timeout().

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/http/serve.test.ts`:
- Around line 2779-2829: Extend the inputs matrix in the “server.timeout()
validates its seconds argument” test with an extreme finite integer such as 2 **
64, probe it, and assert the same out-of-range error as other values above 255,
preserving the existing cases and expected messages.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 02912e1a-6594-4951-bd0c-278a944a3c1e

📥 Commits

Reviewing files that changed from the base of the PR and between e550f2c and a8ba610.

📒 Files selected for processing (2)

• src/runtime/server/server_body.rs
• test/js/bun/http/serve.test.ts

[claude]

No bugs found; the validation matches idleTimeout's pattern and the tests are solid. Deferring to a maintainer only because this tightens a public Bun API — inputs that previously truncated/clamped (e.g. 1.9 → 1s, 300 → 255s) will now throw, which is a user-visible behavior change worth a human sign-off.

What was reviewed:

• Confirmed the new check mirrors idleTimeout in ServerConfig.rs (is_any_int() + to_int64() + range) and uses the same throw_invalid_arguments helper.
• Verified dropping % 255 is correct (and fixes 255 % 255 = 0 on the NodeHTTPResponse branch); RequestContext::set_timeout already .min(255)-clamps internally so nothing >255 was ever meaningfully honored.
• Checked that moving the Unix-socket early-return after validation only means invalid args now throw on Unix servers too — consistent, no side effect ordering concern.
• Prior nit (redundant connection.end() alongside await using) was addressed in e7b0c19.

Extended reasoning...

Overview

Two files: src/runtime/server/server_body.rs (~15 net lines in NewServer::timeout) and two new concurrent tests in test/js/bun/http/serve.test.ts. The Rust change replaces is_number() + to_u32() with is_any_int() + to_int64() + an explicit 0..=255 range check, moves the Unix-socket short-circuit after validation, and drops the value % 255 on the NodeHTTPResponse branch now that the range is enforced.

Security risks

None. This is input validation on a per-request idle-timeout knob; no auth/crypto/permissions surface. Tightening validation strictly reduces the input space that reaches set_timeout.

Level of scrutiny

Medium. The implementation is mechanical and mirrors the existing idleTimeout validation in ServerConfig.rs:1126-1143 almost line-for-line. What warrants a human look is the API-design decision, not the code: this changes server.timeout() from accepting any number (with silent truncation/clamping) to rejecting non-integers and out-of-range values. Previously server.timeout(req, 1.9) set a ~1s timeout and server.timeout(req, 300) clamped to 255s on the Request path (or bizarrely became 45s via % 255 on the node:http path); both now throw. The clearly-buggy cases (NaN/negative/0.4 silently disabling the timeout, 255 % 255 → 0) are unambiguous fixes, but rejecting fractional seconds and >255 rather than truncating/clamping is a choice a maintainer should confirm for a Bun-native public API.

Other factors

• All prior review threads (CodeRabbit's 2 ** 64 case, my connection.end() nit) are resolved with follow-up commits.
• Tests cover the full input matrix including boundaries (0, 255) and adversarial values (NaN, ±Infinity, negative, fractional, 300, 2**64), plus a behavioral test proving the idle timeout stays armed after an invalid timeout() call. The behavioral test uses a bounded Promise.race against a 12s sleep with a 20s test timeout, matching the neighboring allow custom timeout per request test's structure.
• The .d.ts signature (timeout(request: Request, seconds: number): void) is unchanged; it might merit a doc note that seconds must be an integer 0–255, but that's non-blocking.

[robobun]

Rebased onto current main (the branch had gone stale and conflicted in server_body.rs). One change on top of the rebase: a value outside 0..=255 now throws a RangeError with code: "ERR_OUT_OF_RANGE" (the same shape options.port uses) instead of a TypeError, and the docs and types state the range. Re-verified: both tests fail on 1.4.3 and pass with the debug build, along with the other timeout tests in serve.test.ts.

[claude]

Code review found no issues

No high-confidence issues detected in this change.