Files
bun.sh/test/js/web/fetch/response.test.ts
robobun 066a25ac40 Fix crash in Response.redirect with invalid arguments (#21440)
## Summary
- Fix crash in `Response.redirect()` when called with invalid arguments
like `Response.redirect(400, "a")`
- Add proper status code validation per Web API specification (301, 302,
303, 307, 308)
- Add comprehensive tests to prevent regression and ensure spec
compliance

## Issue
When `Response.redirect()` is called with invalid arguments (e.g.,
`Response.redirect(400, "a")`), the code crashes with a panic due to an
assertion failure in `fastGet()`. The second argument is passed to
`Response.Init.init()` which attempts to call `fastGet()` on non-object
values, triggering `bun.assert(this.isObject())` to fail.

Additionally, the original implementation didn't properly validate
redirect status codes according to the Web API specification.

## Fix
Enhanced the `constructRedirect()` function with:

1. **Proper status code validation**: Only allows valid redirect status
codes (301, 302, 303, 307, 308) as specified by the MDN Web API
documentation
2. **Crash prevention**: Only processes object init values to prevent
`fastGet()` crashes with non-object values
3. **Consistent behavior**: Throws `RangeError` for invalid status codes
in both number and object forms

## Changes
- **`src/bun.js/webcore/Response.zig`**: Enhanced `constructRedirect()`
with validation logic
- **`test/js/web/fetch/response.test.ts`**: Added comprehensive tests
for crash prevention and status validation
- **`test/js/web/fetch/fetch.test.ts`**: Updated existing test to use
valid redirect status (307 instead of 408)

## Test Plan
- [x] Added test that reproduces the original crash scenario - now
passes without crashing
- [x] Added tests for proper status code validation (valid codes pass,
invalid codes throw RangeError)
- [x] Verified existing Response.redirect tests still pass
- [x] Confirmed Web API compliance with MDN specification
- [x] Tested various edge cases: `Response.redirect(400, "a")`,
`Response.redirect("url", 400)`, etc.

## Behavior Changes
- **Invalid status codes now throw RangeError** (spec compliant
behavior)
- **Non-object init values are safely ignored** (no more crashes)
- **Maintains backward compatibility** for valid use cases

Per [MDN Web API
specification](https://developer.mozilla.org/en-US/docs/Web/API/Response/redirect_static),
Response.redirect() should only accept status codes 301, 302, 303, 307,
or 308.

Fixes https://github.com/oven-sh/bun/issues/18414

🤖 Generated with [Claude Code](https://claude.ai/code)

---------

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2025-07-28 21:16:47 -07:00

102 lines
3.4 KiB
TypeScript

import { describe, expect, test } from "bun:test";
import { normalizeBunSnapshot } from "harness";
test("zero args returns an otherwise empty 200 response", () => {
const response = new Response();
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
test("calling cancel() on response body doesn't throw", () => {
expect(() => new Response("").body?.cancel()).not.toThrow();
});
test("undefined args don't throw", () => {
const response = new Response("", {
status: undefined,
statusText: undefined,
headers: undefined,
});
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
test("1-arg form returns a 200 response", () => {
const response = new Response("body text");
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
describe("2-arg form", () => {
test("can fill in status/statusText, and it works", () => {
const response = new Response("body text", {
status: 202,
statusText: "Accepted.",
});
expect(response.status).toBe(202);
expect(response.statusText).toBe("Accepted.");
});
test('empty object continues to return 200/""', () => {
const response = new Response("body text", {});
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
});
test("print size", () => {
expect(normalizeBunSnapshot(Bun.inspect(new Response(Bun.file(import.meta.filename)))), import.meta.dir)
.toMatchInlineSnapshot(`
"Response (3.48 KB) {
ok: true,
url: "",
status: 200,
statusText: "",
headers: Headers {
"content-type": "text/javascript;charset=utf-8",
},
redirected: false,
bodyUsed: false,
FileRef ("<cwd>/test/js/web/fetch/response.test.ts") {
type: "text/javascript;charset=utf-8"
}
}"
`);
});
test("Response.redirect with invalid arguments should not crash", () => {
// This should not crash - issue #18414
// Passing a number as URL and string as init should handle gracefully
expect(() => Response.redirect(400, "a")).not.toThrow();
// Test various invalid argument combinations - should not crash
expect(() => Response.redirect(42, "test")).not.toThrow();
expect(() => Response.redirect(true, "string")).not.toThrow();
expect(() => Response.redirect(null, "init")).not.toThrow();
expect(() => Response.redirect(undefined, "value")).not.toThrow();
});
test("Response.redirect status code validation", () => {
// Valid redirect status codes should work
expect(() => Response.redirect("url", 301)).not.toThrow();
expect(() => Response.redirect("url", 302)).not.toThrow();
expect(() => Response.redirect("url", 303)).not.toThrow();
expect(() => Response.redirect("url", 307)).not.toThrow();
expect(() => Response.redirect("url", 308)).not.toThrow();
// Invalid status codes should throw RangeError
expect(() => Response.redirect("url", 200)).toThrow(RangeError);
expect(() => Response.redirect("url", 400)).toThrow(RangeError);
expect(() => Response.redirect("url", 500)).toThrow(RangeError);
// Status in object should also be validated
expect(() => Response.redirect("url", { status: 307 })).not.toThrow();
expect(() => Response.redirect("url", { status: 400 })).toThrow(RangeError);
// Check that the correct status is set
expect(Response.redirect("url", 301).status).toBe(301);
expect(Response.redirect("url", { status: 308 }).status).toBe(308);
});