Skip to content

Commit

Permalink
Fix memory leak in certain cases when long URLs are passed to req.url (
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Jan 27, 2025
1 parent 843cb38 commit cd53d32
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/bun.js/webcore/request.zig
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ pub const Request = struct {
const href = bun.JSC.URL.hrefFromString(this.url);
// TODO: what is the right thing to do for invalid URLS?
if (!href.isEmpty()) {
this.url.deref();
this.url = href;
}

Expand Down
26 changes: 26 additions & 0 deletions test/js/bun/http/req-url-leak-fixture.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 47 additions & 0 deletions test/js/bun/http/req-url-leak.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { test, expect } from "bun:test";
import { bunExe, bunEnv } from "harness";
import path from "path";
test("req.url doesn't leak memory", async () => {
const { promise, resolve } = Promise.withResolvers();
await using process = Bun.spawn({
cmd: [bunExe(), path.join(import.meta.dir, "req-url-leak-fixture.js")],
env: bunEnv,
stdout: "inherit",
stderr: "inherit",
stdin: "inherit",
ipc(message, child) {
if (message.url) {
resolve(message.url);
}
},
});

const baseURL = await promise;

const url = new URL(Buffer.alloc(1024 * 15, "Z").toString(), baseURL);

let maxRSS = 0;

for (let i = 0; i < 512; i++) {
const batchSize = 64;
const promises = [];
for (let j = 0; j < batchSize; j++) {
promises.push(
fetch(url)
.then(r => r.text())
.then(rssText => {
const rss = parseFloat(rssText);
if (Number.isSafeInteger(rss)) {
maxRSS = Math.max(maxRSS, rss);
}
}),
);
}
await Promise.all(promises);
}

console.log("RSS", maxRSS);

// 557 MB on Bun 1.2
expect(maxRSS).toBeLessThan(1024 * 1024 * 256);
}, 10_000);

0 comments on commit cd53d32

Please sign in to comment.