Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 7 additions & 6 deletions src/http/InternalState.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,12 @@ impl<'a> InternalState<'a> {

// gzip stores the size of the uncompressed data in the last 4 bytes of the stream
// But it's only valid if the stream is less than 4.7 GB, since it's 4 bytes.
// If we know that the stream is going to be larger than our
// pre-allocated buffer, then let's dynamically allocate the exact
// size.
// When the trailer gives us a plausible size, decompress straight
// into the caller's buffer instead of going through the shared
// scratch buffer and copying the whole body a second time. A lying
// trailer is harmless: the reservation is capped at 32 MB, and an
// undersized reservation makes decompress_to_vec fail with
// InsufficientSpace, which falls through to the streaming slow path.
if self.encoding == Encoding::Gzip
&& buffer.len() > 16
&& buffer.len() < 1024 * 1024 * 1024
Expand All @@ -290,9 +293,7 @@ impl<'a> InternalState<'a> {
.expect("infallible: size matches"),
);
// Since this is arbtirary input from the internet, let's set an upper bound of 32 MB for the allocation size.
if (estimated_size as usize) > deflater.shared_buffer.len()
&& estimated_size < 32 * 1024 * 1024
{
if estimated_size > 0 && estimated_size < 32 * 1024 * 1024 {
body_out_str.list.reserve_exact(
(estimated_size as usize).saturating_sub(body_out_str.list.len()),
);
Expand Down
23 changes: 18 additions & 5 deletions src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2275,11 +2275,24 @@ impl FetchTasklet {
}
} else {
if success {
bun_core::handle_oom(
task_ref
.scheduled_response_buffer
.write(task_ref.response_buffer.list.as_slice()),
);
if task_ref.scheduled_response_buffer.list.is_empty() && !task_ref.result.has_more
{
// Final delivery into an empty scheduled buffer — the common
// buffered-response case, where this callback carries the
// complete body. Hand the accumulated bytes over instead of
// copying them. Intermediate streaming chunks keep the copy
// below so `response_buffer` retains its capacity for reuse.
core::mem::swap(
&mut task_ref.scheduled_response_buffer.list,
&mut task_ref.response_buffer.list,
);
} else {
bun_core::handle_oom(
task_ref
.scheduled_response_buffer
.write(task_ref.response_buffer.list.as_slice()),
);
}
}
// reset for reuse
task_ref.response_buffer.reset();
Expand Down
42 changes: 42 additions & 0 deletions test/js/web/fetch/fetch-gzip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,45 @@
server.stop();
done();
});

describe("gzip response edge cases", () => {
// Behavior pins for the libdeflate fast path and its fallbacks: honest
// streams decode byte-exactly; integrity violations (which both libdeflate
// and zlib verify) reject with ZlibError regardless of which decode path
// ran. The corrupted-trailer cases also exercise the
// exact-size-reservation branch falling through to the streaming path.
const payload = Buffer.alloc(300 * 1024);
for (let i = 0; i < payload.length; i++) payload[i] = (i * 13) & 0xff;

function corrupt(data: Buffer, offsetFromEnd: number) {
const gz = Buffer.from(Bun.gzipSync(data));
gz[gz.length - offsetFromEnd] ^= 0xff;
return gz;
}

const cases: Record<string, { body: Uint8Array; expected: Buffer | "error" }> = {
"honest-large": { body: Bun.gzipSync(payload), expected: payload },
"honest-small": { body: Bun.gzipSync(Buffer.from("hello gzip world")), expected: Buffer.from("hello gzip world") },
"empty": { body: Bun.gzipSync(Buffer.alloc(0)), expected: Buffer.alloc(0) },
"isize-corrupt": { body: corrupt(payload, 1), expected: "error" },

Check warning on line 316 in test/js/web/fetch/fetch-gzip.test.ts

View check run for this annotation

Claude / Claude Code Review

isize-corrupt test bypasses the exact-size-reservation branch it claims to exercise

The `isize-corrupt` case flips `gz[gz.length - 1]`, which is the **MSB** of the little-endian ISIZE field — for a 300KB payload (ISIZE = 0x0004B000) this yields 0xFF04B000 ≈ 4.28 GB, which fails the `< 32 MB` cap and skips the exact-size-reservation branch entirely (it falls through to the shared_buffer path instead). So the comment overstates coverage: `crc-corrupt` does enter the reservation branch and fall through on BadData, but neither case exercises the "undersized ISIZE → `InsufficientSpa
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
"crc-corrupt": { body: corrupt(payload, 8), expected: "error" },
"truncated": { body: Buffer.from(Bun.gzipSync(payload)).subarray(0, 1000), expected: "error" },
};

for (const [name, c] of Object.entries(cases)) {
it(`decodes or rejects: ${name}`, async () => {
using server = Bun.serve({
port: 0,
fetch: () => new Response(c.body, { headers: { "Content-Encoding": "gzip" } }),
});
if (c.expected === "error") {
expect(async () => {
await (await fetch(server.url)).arrayBuffer();
}).toThrow();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
} else {
const got = Buffer.from(await (await fetch(server.url)).arrayBuffer());
expect(Buffer.compare(got, c.expected)).toBe(0);
}
});
}
});
Loading