Commit Graph

9 Commits

Author SHA1 Message Date
robobun
344b2c1dfe fix: Response.clone() no longer locks body when body was accessed before clone (#25484)
## Summary
- Fix bug where `Response.clone()` would lock the original response's
body when `response.body` was accessed before cloning
- Apply the same fix to `Request.clone()`

## Root Cause
When `response.body` was accessed before calling `response.clone()`, the
original response's body would become locked after cloning. This
happened because:

1. When the cloned response was wrapped with `toJS()`,
`checkBodyStreamRef()` was called which moved the stream from
`Locked.readable` to `js.gc.stream` and cleared `Locked.readable`
2. The subsequent code tried to get the stream from `Locked.readable`,
which was now empty, so the body cache update was skipped
3. The JavaScript-level body property cache still held the old locked
stream

## Fix
Updated the cache update logic to:
1. For the cloned response: use `js.gc.stream.get()` instead of
`Locked.readable.get()` since `toJS()` already moved the stream
2. For the original response: use `Locked.readable.get()` which still
holds the teed stream since `checkBodyStreamRef` hasn't been called yet

## Reproduction
```javascript
const readableStream = new ReadableStream({
  start(controller) {
    controller.enqueue(new TextEncoder().encode("Hello, world!"));
    controller.close();
  },
});

const response = new Response(readableStream);
console.log(response.body?.locked); // Accessing body before clone
const cloned = response.clone();
console.log(response.body?.locked); // Expected: false, Actual: true 
console.log(cloned.body?.locked);   // Expected: false, Actual: false 
```

## Test plan
- [x] Added regression tests for `Response.clone()` in
`test/js/web/fetch/response.test.ts`
- [x] Added regression test for `Request.clone()` in
`test/js/web/request/request.test.ts`
- [x] Verified tests fail with system bun (before fix) and pass with
debug build (after fix)

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

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2025-12-15 18:46:02 -08:00
SUZUKI Sosuke
f4b6396eac Fix unhandled exception in JSC__JSPromise__wrap when resolving promise (#23961)
### What does this PR do?

Previously, `JSC__JSPromise__wrap` would call
`JSC::JSPromise::resolvedPromise(globalObject, result)` without checking
if an exception was thrown during promise resolution. This
could happen in certain edge cases, such as when the result value is a
thenable that triggers stack overflow, or when the promise resolution
mechanism itself encounters an error.
When such exceptions occurred, they would escape back to the Zig code,
causing the CatchScope assertion to fail with "ASSERTION FAILED:
Unexpected exception observed on thread"
instead of being properly handled.

This PR adds an exception check immediately after calling
`JSC::JSPromise::resolvedPromise()` and before the `RELEASE_AND_RETURN`
macro. If an exception is detected, the function
now clears it and returns a rejected promise with the exception value,
ensuring consistent error handling behavior. This matches the pattern
already used earlier in the function
for the initial function call exception handling.

### How did you verify your code works?

new and existing tests

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2025-10-24 21:36:33 -07:00
Jarred Sumner
5c65c18e72 Delete incorrect assertion in ComptimeStringMap (#21504)
### What does this PR do?

Resolves
```js
Bun v1.2.13 ([64ed68c](64ed68c9e0)) on windows x86_64 [TestCommand]

panic: ComptimeStringMap.fromJS: input is not a string

[comptime_string_map.zig:268](64ed68c9e0/src/comptime_string_map.zig (L268)): getWithEql
[Response.zig:682](64ed68c9e0/src/bun.js/webcore/Response.zig (L682)): init
[Request.zig:679](64ed68c9e0/src/bun.js/webcore/Request.zig (L679)): constructInto
[ZigGeneratedClasses.cpp:37976](64ed68c9e0/C:/buildkite-agent/builds/EC2AMAZ-Q4V5GV4/bun/bun/build/release/codegen/ZigGeneratedClasses.cpp#L37976): WebCore::JSRequestConstructor::construct
2 unknown/js code
llint_entry

Features: tsconfig, Bun.stdout, dotenv, jsc
```

### How did you verify your code works?

There is a test.
2025-07-31 00:56:50 -07:00
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
Meghan Denny
9bc559e09f console: fix printing of Response(Bun.file()) (#20933) 2025-07-11 11:37:44 -07:00
Jarred Sumner
cd6785771e run prettier and add back format action (#13722) 2024-09-03 21:32:52 -07:00
John-David Dalton
769d7a1680 fix: null is not an object at readableStreamCancel (#10091)
* fix: null is not an object at readableStreamCancel

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
2024-04-09 14:02:23 -07:00
Jarred Sumner
c49a5a7d7d When getting properties, treat undefined the same as though the property was not defined (#7048)
It works this way in JS.

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
2023-11-11 05:06:24 -08:00
Chris Toshok
c5354951ba Fix Response.statusText (#6151) 2023-10-16 15:57:16 -07:00