Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
2 changes: 1 addition & 1 deletion scripts/build/deps/webkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* for local mode. Override via `--webkit-version=<hash>` to test a branch.
* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "34c01d13391e00c06862a3d2c5b7fff350ac87e0";
export const WEBKIT_VERSION = "autobuild-preview-pr-384-0cd81acb";
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
* WebKit (JavaScriptCore) — the JS engine.
Expand Down
142 changes: 110 additions & 32 deletions test/cli/inspect/inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,49 +300,51 @@
});
});

describe("http metadata endpoint", () => {
let metadataInspectee: Subprocess | undefined;

async function spawnInspectee(): Promise<URL> {
metadataInspectee = spawn({
cwd: import.meta.dir,
cmd: [bunExe(), "--inspect=127.0.0.1:0", "inspectee.js"],
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});
async function spawnInspectee(): Promise<{ child: Subprocess; url: URL }> {
const child = spawn({
cwd: import.meta.dir,
cmd: [bunExe(), "--inspect=127.0.0.1:0", "inspectee.js"],
env: bunEnv,
stdout: "ignore",
stderr: "pipe",
});

let url: URL | undefined;
let stderr = "";
const decoder = new TextDecoder();
for await (const chunk of metadataInspectee.stderr as ReadableStream) {
stderr += decoder.decode(chunk);
for (const line of stderr.split("\n")) {
try {
url = new URL(line);
} catch {}
if (url?.protocol.includes("ws")) {
break;
}
}
if (stderr.includes("Listening:")) {
let url: URL | undefined;
let stderr = "";
const decoder = new TextDecoder();
for await (const chunk of child.stderr as ReadableStream) {
stderr += decoder.decode(chunk);
for (const line of stderr.split("\n")) {
try {
url = new URL(line);
} catch {}
if (url?.protocol.includes("ws")) {
break;
}
}

if (!url) {
process.stderr.write(stderr);
throw new Error("Unable to find listening URL");
if (stderr.includes("Listening:")) {
break;
}
return url;
}

if (!url) {
process.stderr.write(stderr);
throw new Error("Unable to find listening URL");
}
return { child, url };

Check warning on line 334 in test/cli/inspect/inspect.test.ts

View check run for this annotation

Claude / Claude Code Review

spawnInspectee() leaks child process if URL discovery throws

The hoist changed cleanup ordering: previously `metadataInspectee = spawn({...})` ran before the URL-discovery loop, so `afterEach` would kill the child even if this throw fired; now the child is a local that only escapes on success, so a throw here leaks the subprocess. Consider `child.kill()` before throwing (inspectee.js self-exits after 30s and the banner is written atomically, so this is low-probability — but it's a regression from ca97e16f and REVIEW.md asks for cleanup registered before t
Comment thread
robobun marked this conversation as resolved.
}

describe("http metadata endpoint", () => {
let metadataInspectee: Subprocess | undefined;

afterEach(() => {
metadataInspectee?.kill();
});

test("serves /json/version only for a Host of the bound hostname, localhost, or an IP literal", async () => {
const { port } = await spawnInspectee();
const { child, url } = await spawnInspectee();
metadataInspectee = child;
const { port } = url;
const endpoint = `http://127.0.0.1:${port}/json/version`;

const allowed = await fetch(endpoint);
Expand All @@ -365,7 +367,9 @@
});

test("serves /json/version only to allowed web origins", async () => {
const { port } = await spawnInspectee();
const { child, url } = await spawnInspectee();
metadataInspectee = child;
const { port } = url;
const endpoint = `http://127.0.0.1:${port}/json/version`;

const loopback = await fetch(endpoint, { headers: { "Origin": "http://127.0.0.1:8080" } });
Expand All @@ -377,6 +381,80 @@
});
});

// Runtime.evaluate with returnByValue:true on a value holding a BigInt or
// Symbol used to hit ASSERT_NOT_REACHED in Inspector::jsToInspectorValue
// (aborting an assertions build) and return the misleading "Object has too
// long reference chain" error on release. It should now report the value as
// unserializable with V8's wording and leave the debuggee running.
describe("Runtime.evaluate returnByValue with BigInt/Symbol", () => {
let child: Subprocess | undefined;

afterEach(() => {
child?.kill();
child = undefined;
});

async function evaluateByValue(expression: string) {
const spawned = await spawnInspectee();
child = spawned.child;

const ws = new WebSocket(spawned.url);
await new Promise<void>((resolve, reject) => {
ws.addEventListener("open", () => resolve(), { once: true });
ws.addEventListener("error", cause => reject(new Error("WebSocket error", { cause })), { once: true });
ws.addEventListener("close", () => reject(new Error("WebSocket closed before open")), { once: true });
});
const reply = new Promise<any>((resolve, reject) => {
ws.addEventListener(
"message",
({ data }) => {
try {
resolve(JSON.parse(data.toString()));
} catch (cause) {
reject(new Error(`non-JSON inspector reply: ${data}`, { cause }));
}
},
{ once: true },
);
ws.addEventListener("error", cause => reject(new Error("WebSocket error", { cause })), { once: true });
ws.addEventListener("close", () => reject(new Error("WebSocket closed before reply")), { once: true });
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
ws.send(JSON.stringify({ id: 1, method: "Runtime.evaluate", params: { expression, returnByValue: true } }));
const result = await reply;
ws.close();
return result;
}

for (const expression of ["[1n]", "({a: 1n})", 'Symbol("s")', "({b: Symbol()})"]) {
test(expression, async () => {
const reply = await evaluateByValue(expression);
expect(reply).toEqual({
id: 1,
error: {
code: -32000,
message: "Object couldn't be returned by value",
data: expect.anything(),
},
});
// The debuggee must survive the request (an assertions build used to
// SIGABRT here before the reply arrived).
expect(child!.exitCode).toBeNull();
expect(child!.signalCode).toBeNull();
});
}

test("bare 1n", async () => {
// A bare top-level BigInt is represented via description without a JSON
// value and has always worked; keep it covered so the unserializable path
// above does not regress it.
const reply = await evaluateByValue("1n");
expect(reply).toEqual({
id: 1,
result: { result: { type: "bigint", description: "1n" }, wasThrown: false },
});
});
});

describe("unix domain socket without websocket", () => {
let tempdir: string;
let randomSocketPath: () => string;
Expand Down
Loading