fix(sanitizeStatusCode): return default for non-numeric input instead of NaN by chatman-media · Pull Request #1420 · h3js/h3

chatman-media · 2026-06-22T23:51:00Z

Coercing a non-numeric string status to a number yields NaN, and NaN silently passes the range check, so the sanitizer returns NaN — defeating its own purpose ("Make sure the status code is a valid HTTP status code").

Bug

sanitizeStatusCode("abc"); // NaN  (expected: 200)
sanitizeStatusCode("12x"); // NaN
sanitizeStatusCode(Number.NaN); // NaN

Inside sanitizeStatusCode, +statusCode is NaN for non-numeric strings. Because every comparison with NaN is false, the statusCode < 100 || statusCode > 599 guard does not catch it and NaN is returned unchanged.

This is reachable through HTTPError (src/error.ts reads status/statusCode from arbitrary error/cause objects) and the deprecated setResponseStatus(event, code) (which accepts a string). A NaN status then throws when the response is built:

RangeError: init["status"] must be in the range of 200 to 599, inclusive.

So a value meant to be sanitized into a safe code instead crashes response construction.

Fix

Reject NaN alongside the existing out-of-range check so an invalid code always falls back to defaultStatusCode:

if (Number.isNaN(statusCode) || statusCode < 100 || statusCode > 599) {
  return defaultStatusCode;
}

Valid numbers and numeric strings are unaffected. The bug has been present since the original implementation (2023).

Tests

Added test/unit/sanitize.test.ts. The non-numeric strings (never NaN) case fails on main (expected NaN to be 200) and passes with the fix; the other cases pin existing behavior (valid codes, out-of-range, falsy input).

vitest run test/unit/sanitize.test.ts → 5 passed
Full suite: 48/49 files, 1152 tests pass (the only red is test/bench/bundle.test.ts, which needs the native esbuild binary and fails identically on a clean checkout — unrelated to this change)
oxfmt --check clean on both files

Summary by CodeRabbit

Bug Fixes
- Improved HTTP status code validation to properly reject invalid values including NaN, preventing potential response construction failures.
Tests
- Added comprehensive unit tests for status code and message sanitization utilities, ensuring proper handling of valid codes, out-of-range values, and edge cases.

… of NaN `+statusCode` coerces non-numeric strings to `NaN`, and every comparison with `NaN` is `false`, so `NaN` slipped past the `< 100 || > 599` range check and was returned as-is. A `NaN` status then throws `RangeError: init["status"] must be in the range of 200 to 599` when used to build a `Response`, defeating the purpose of the sanitizer. Guard against `NaN` so an invalid code always falls back to the default.

coderabbitai · 2026-06-22T23:51:13Z

📝 Walkthrough

Walkthrough

sanitizeStatusCode gains an explicit Number.isNaN check after coercing its string input to a number, preventing non-numeric strings from leaking NaN through the range comparisons. A new unit test file covers valid, out-of-range, missing, and non-numeric inputs for sanitizeStatusCode, plus a character-stripping test for sanitizeStatusMessage.

Changes

NaN guard in sanitizeStatusCode

Layer / File(s)	Summary
NaN guard implementation and unit tests `src/utils/sanitize.ts`, `test/unit/sanitize.test.ts`	`sanitizeStatusCode` adds `Number.isNaN` check after `+statusCode` coercion to prevent NaN bypassing range guards. Tests verify pass-through for valid numeric and string codes, fallback for out-of-range or undefined inputs, the NaN regression path, and newline removal in `sanitizeStatusMessage`.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A hop, a check, a NaN in disguise,
🐇 The rabbit sniffed out its trickery and lies.
+statusCode could yield something wrong,
But now isNaN keeps the range check strong.
No RangeError shall ruin our day —
The sanitizer hops safely away! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing sanitizeStatusCode to return a default value for non-numeric input instead of NaN, which is the core issue addressed in the PR.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

coderabbitai

🧹 Nitpick comments (1)

test/unit/sanitize.test.ts (1)
4-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider using describeMatrix for cross-runtime test coverage (optional).

The coding guidelines specify: "Use describeMatrix for writing cross-runtime tests that execute in both web and node modes" for files in test/**/*.test.ts. These utility functions have no runtime-specific behavior (no DOM APIs, no Node-specific features), so they behave identically in web and node environments. Using describeMatrix would run the same tests twice without discovering new behavior differences. However, if the project's convention is to apply describeMatrix uniformly across all test files for consistency, consider wrapping the test suites in describeMatrix("sanitizeStatusCode", ({ it }) => { ... }) and adapting assertions to use the matrix it function. Given that this is a utility test with pure functions, this refactor is optional and low-priority.
🤖 Prompt for 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.

In `@test/unit/sanitize.test.ts` around lines 4 - 34, The test file for the
sanitizeStatusCode utility is not using the describeMatrix function for
cross-runtime test coverage. Wrap the entire describe block for
sanitizeStatusCode in describeMatrix("sanitizeStatusCode", ({ it }) => { ... })
and replace all instances of the describe it function with the matrix it
function provided by describeMatrix to enable the tests to run in both web and
node runtime environments. Note that this refactor is optional and low-priority
since sanitizeStatusCode has no runtime-specific dependencies.
Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@test/unit/sanitize.test.ts`:
- Around line 4-34: The test file for the sanitizeStatusCode utility is not
using the describeMatrix function for cross-runtime test coverage. Wrap the
entire describe block for sanitizeStatusCode in
describeMatrix("sanitizeStatusCode", ({ it }) => { ... }) and replace all
instances of the describe it function with the matrix it function provided by
describeMatrix to enable the tests to run in both web and node runtime
environments. Note that this refactor is optional and low-priority since
sanitizeStatusCode has no runtime-specific dependencies.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47febfb2-019d-460e-9c9a-09ab09355c48

📥 Commits

Reviewing files that changed from the base of the PR and between edb53fe and 6fc26fb.

📒 Files selected for processing (2)

src/utils/sanitize.ts
test/unit/sanitize.test.ts

pi0

thnx!

codecov · 2026-06-23T05:35:36Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

… of NaN (h3js#1420)

chatman-media requested a review from pi0 as a code owner June 22, 2026 23:51

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

pi0 approved these changes Jun 23, 2026

View reviewed changes

pi0 merged commit f1155fc into h3js:main Jun 23, 2026
8 checks passed

mahirhir mentioned this pull request Jun 30, 2026

fix(sanitizeStatusCode): return default for non-numeric input instead of NaN #1422

Open

pi0x mentioned this pull request Jul 2, 2026

fix(sanitize): reject NaN status code from non-numeric strings #1397

Closed

pi0x pushed a commit to mixelburg/h3 that referenced this pull request Jul 2, 2026

fix(sanitizeStatusCode): return default for non-numeric input instead…

5a519d8

… of NaN (h3js#1420)

coderabbitai Bot mentioned this pull request Jul 18, 2026

fix(response): do not render non-Error throws as successful responses #1485

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sanitizeStatusCode): return default for non-numeric input instead of NaN#1420

fix(sanitizeStatusCode): return default for non-numeric input instead of NaN#1420
pi0 merged 1 commit into
h3js:mainfrom
chatman-media:fix/sanitize-status-code-nan

chatman-media commented Jun 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

pi0 left a comment

Uh oh!

codecov Bot commented Jun 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chatman-media commented Jun 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug

Fix

Tests

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

pi0 left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jun 23, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatman-media commented Jun 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading