Skip to content

Fix server stability issue with oversized requests #22701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 26, 2025
Merged

Fix server stability issue with oversized requests #22701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7a94b5a
Fix server stability issue with oversized requests
Sep 15, 2025
f4e77c2
[autofix.ci] apply automated fixes
autofix-ci[bot] Sep 16, 2025
87b955c
Merge branch 'main' into claude/fix-oversized-request-stability
Jarred-Sumner Sep 26, 2025
888272b
Update server.zig
Jarred-Sumner Sep 26, 2025
4a4ece9
Cleanup
Jarred-Sumner Sep 26, 2025
d2f8c33
[autofix.ci] apply automated fixes
autofix-ci[bot] Sep 26, 2025
e193c80
Update 22353.test.ts
Jarred-Sumner Sep 26, 2025
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
49 changes: 30 additions & 19 deletions src/bun.js/api/server.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2143,7 +2143,36 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d

pub fn prepareJsRequestContext(this: *ThisServer, req: *uws.Request, resp: *App.Response, should_deinit_context: ?*bool, create_js_request: bool, method: ?bun.http.Method) ?PreparedRequest {
jsc.markBinding(@src());

// We need to register the handler immediately since uSockets will not buffer.
//
// We first validate the self-reported request body length so that
// we avoid needing to worry as much about what memory to free.
const request_body_length: ?usize = request_body_length: {
if ((HTTP.Method.which(req.method()) orelse HTTP.Method.OPTIONS).hasRequestBody()) {
const len: usize = brk: {
if (req.header("content-length")) |content_length| {
break :brk std.fmt.parseInt(usize, content_length, 10) catch 0;
}

break :brk 0;
};

// Abort the request very early.
if (len > this.config.max_request_body_size) {
resp.writeStatus("413 Request Entity Too Large");
resp.endWithoutBody(true);
return null;
}

break :request_body_length len;
}

break :request_body_length null;
};

this.onPendingRequest();

if (comptime Environment.isDebug) {
this.vm.eventLoop().debug.enter();
}
Expand Down Expand Up @@ -2193,25 +2222,7 @@ pub fn NewServer(protocol_enum: enum { http, https }, development_kind: enum { d
};
}

// we need to do this very early unfortunately
// it seems to work fine for synchronous requests but anything async will take too long to register the handler
// we do this only for HTTP methods that support request bodies, so not GET, HEAD, OPTIONS, or CONNECT.
if ((HTTP.Method.which(req.method()) orelse HTTP.Method.OPTIONS).hasRequestBody()) {
const req_len: usize = brk: {
if (req.header("content-length")) |content_length| {
break :brk std.fmt.parseInt(usize, content_length, 10) catch 0;
}

break :brk 0;
};

if (req_len > this.config.max_request_body_size) {
resp.writeStatus("413 Request Entity Too Large");
resp.endWithoutBody(true);
this.finalize();
return null;
}

if (request_body_length) |req_len| {
ctx.request_body_content_len = req_len;
ctx.flags.is_transfer_encoding = req.header("transfer-encoding") != null;
if (req_len > 0 or ctx.flags.is_transfer_encoding) {
Expand Down
37 changes: 37 additions & 0 deletions test/regression/issue/22353.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { expect, test } from "bun:test";

test("issue #22353 - server should handle oversized request without crashing", async () => {
using server = Bun.serve({
port: 0,
maxRequestBodySize: 1024, // 1KB limit
async fetch(req) {
const body = await req.text();
return new Response(
JSON.stringify({
received: true,
size: body.length,
}),
{
headers: { "Content-Type": "application/json" },
},
);
},
});

const resp = await fetch(server.url, {
method: "POST",
body: "A".repeat(1025),
});
expect(resp.status).toBe(413);
expect(await resp.text()).toBeEmpty();
for (let i = 0; i < 100; i++) {
const resp2 = await fetch(server.url, {
method: "POST",
});
expect(resp2.status).toBe(200);
expect(await resp2.json()).toEqual({
received: true,
size: 0,
});
}
}, 10000);
Comment on lines +3 to +37
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Drop the explicit timeout argument

Line 37 forces a 10 s cap on the test, which our testing guidelines forbid because it introduces avoidable flakiness. Let Bun’s default timeout handle this instead—nothing in the body requires a custom duration.

-}, 10000);
+});

As per coding guidelines

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test("issue #22353 - server should handle oversized request without crashing", async () => {
using server = Bun.serve({
port: 0,
maxRequestBodySize: 1024, // 1KB limit
async fetch(req) {
const body = await req.text();
return new Response(
JSON.stringify({
received: true,
size: body.length,
}),
{
headers: { "Content-Type": "application/json" },
},
);
},
});
const resp = await fetch(server.url, {
method: "POST",
body: "A".repeat(1025),
});
expect(resp.status).toBe(413);
expect(await resp.text()).toBeEmpty();
for (let i = 0; i < 100; i++) {
const resp2 = await fetch(server.url, {
method: "POST",
});
expect(resp2.status).toBe(200);
expect(await resp2.json()).toEqual({
received: true,
size: 0,
});
}
}, 10000);
test("issue #22353 - server should handle oversized request without crashing", async () => {
using server = Bun.serve({
port: 0,
maxRequestBodySize: 1024, // 1KB limit
async fetch(req) {
const body = await req.text();
return new Response(
JSON.stringify({
received: true,
size: body.length,
}),
{
headers: { "Content-Type": "application/json" },
},
);
},
});
const resp = await fetch(server.url, {
method: "POST",
body: "A".repeat(1025),
});
expect(resp.status).toBe(413);
expect(await resp.text()).toBeEmpty();
for (let i = 0; i < 100; i++) {
const resp2 = await fetch(server.url, {
method: "POST",
});
expect(resp2.status).toBe(200);
expect(await resp2.json()).toEqual({
received: true,
size: 0,
});
}
});
🤖 Prompt for AI Agents 
In test/regression/issue/22353.test.ts around lines 3 to 37, the test call
includes an explicit timeout argument (10000) which should be removed per
guidelines; delete the final timeout parameter and the trailing comma so the
test uses the default timeout (i.e., change the test invocation to omit the ",
10000" portion and ensure the closing parentheses/braces remain balanced).

Loading