Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
28 changes: 15 additions & 13 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6826,19 +6826,21 @@ pub mod bv2_impl {
.text;
let loader = this.graph.input_files.items_loader()[source_index as usize];
if this.should_add_watcher(source_path) {
// const generic `CLONE_FILE_PATH = isWindows`
// matches `cfg!(windows)` at compile time.
let _ = this
.bun_watcher_mut()
.unwrap()
.add_file::<{ cfg!(windows) }>(
parse_result.watcher_data.fd,
source_path,
bun_wyhash::hash(source_path) as u32,
bun_watcher::Loader(loader as u8),
parse_result.watcher_data.dir_fd,
None,
);
// CLONE_FILE_PATH=true: `source_path` is not guaranteed to
// be process-lifetime storage — `dupe_alloc` allocates the
// disjoint `text\0pretty\0` case (e.g. every `ssr:`-prefixed
// SSR-graph path) from the per-bundle arena, which is freed
// when the bundle completes. The watchlist outlives the
// bundle, so it must own its copy of the path (the watcher
// dedups by hash, so this is one copy per watched file).
let _ = this.bun_watcher_mut().unwrap().add_file::<true>(
parse_result.watcher_data.fd,
source_path,
bun_wyhash::hash(source_path) as u32,
bun_watcher::Loader(loader as u8),
parse_result.watcher_data.dir_fd,
None,
);
}
}
}
Expand Down
14 changes: 11 additions & 3 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4503,13 +4503,21 @@ pub(super) fn finalize_bundle(
dev.incremental_result.html_routes_soft_affected.clear();
ctx.gts.clear();

for index in &dev.incremental_result.client_components_affected {
// `trace_dependencies` appends to `client_components_affected` when it
// visits another client component boundary, so iterate by index against
// the live Vec (never a captured slice). Entries appended mid-loop
// already had their `gts` bit set when pushed, so re-visiting them is a
// no-op and the loop is bounded.
let mut i = 0;
while i < dev.incremental_result.client_components_affected.len() {
let index = dev.incremental_result.client_components_affected[i];
dev.server_graph.trace_dependencies(
*index,
index,
ctx.gts,
incremental_graph::TraceDependencyGoal::NoStop,
*index,
index,
)?;
i += 1;
}

for request in &dev.incremental_result.framework_routes_affected {
Expand Down
35 changes: 31 additions & 4 deletions src/runtime/bake/FrameworkRouter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,10 @@ impl TinyLog {
}

pub fn print(&self, rel_path: &[u8]) {
let cursor_at = self.cursor_at as usize;
// `cursor_at` may still be the `empty()` sentinel (`u32::MAX`) when the
// producer recorded a message without a cursor position. Clamp to the
// path length so the slicing below cannot go out of bounds.
let cursor_at = (self.cursor_at as usize).min(rel_path.len());
let cursor_len = self.cursor_len as usize;
let after = &rel_path[cursor_at..];
Output::err_generic(
Expand Down Expand Up @@ -1639,8 +1642,25 @@ impl FrameworkRouter {
.parse(rel_path, ext, &mut log, t.allow_layouts, arena_state);
let parsed = match parse_result {
Err(_) => {
log.cursor_at +=
u32::try_from(abs_root_len - root_len).expect("int cast");
// Rebase the cursor from `rel_path` coordinates
// (where `parse` recorded it) onto
// `full_rel_path` (what `print` receives).
// `rel_path` starts one byte before the
// `full_rel_path` suffix it mirrors — the
// leading '/' — so the offset is one less than
// the root-length delta. Error paths that never
// called `log.fail()` (e.g. OOM) leave the
// `u32::MAX` sentinel; keep it pinned there so
// `print` clamps to the end of the path.
if log.cursor_at != u32::MAX {
log.cursor_at = log
.cursor_at
.saturating_add(
u32::try_from(abs_root_len - root_len)
.expect("int cast"),
)
.saturating_sub(1);
}
ctx.on_router_syntax_error(full_rel_path, log)?;
arena_state.reset_retain_with_limit(8 * 1024 * 1024);
continue 'outer;
Expand Down Expand Up @@ -1673,7 +1693,14 @@ impl FrameworkRouter {
}

if param_count > 64 {
log.write(format_args!("Pattern cannot have more than 64 param"));
// Use `fail()` so the cursor is set, highlighting
// the whole path: unlike `fail()` callers inside
// `parse`, there is no narrower span to point at.
let _ = log.fail(
format_args!("Pattern cannot have more than 64 param"),
0,
full_rel_path.len(),
);
ctx.on_router_syntax_error(full_rel_path, log)?;
arena_state.reset_retain_with_limit(8 * 1024 * 1024);
continue 'outer;
Expand Down
62 changes: 62 additions & 0 deletions test/bake/dev/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -814,3 +814,65 @@ devTest("barrel optimization: two import statements from the same barrel (#28886
await c.expectMessage("got: ALPHA BETA");
},
});

// Regression: editing a file shared by several client component boundaries
// crashed the dev server. `finalizeBundle` iterates
// `incremental_result.client_components_affected` while `trace_dependencies`
// re-appends every visited boundary to that same list (the trace bits were
// just cleared), so with 3+ boundaries the Vec reallocates mid-iteration and
// the stale iterator reads freed memory — caught as a use-after-free under
// ASAN, and as `index out of bounds: the len is N but the index is N` in
// release once the freed block is reused by `framework_routes_affected`.
devTest("hot update touching multiple client component boundaries does not crash", {
framework: {
...minimalFramework,
serverComponents: {
...minimalFramework.serverComponents!,
separateSSRGraph: true,
},
},
files: {
"routes/index.ts": `
import * as A from '../components/A';
import * as B from '../components/B';
import * as C from '../components/C';
import * as D from '../components/D';
export default function (req, meta) {
// Client component exports are reference proxies on the server; only
// their existence is observable here.
return new Response('page: ' + [typeof A.a, typeof B.b, typeof C.c, typeof D.d].join(','));
}
`,
"components/A.ts": `
"use client";
import { shared } from './shared';
export const a = 'A' + shared;
`,
"components/B.ts": `
"use client";
import { shared } from './shared';
export const b = 'B' + shared;
`,
"components/C.ts": `
"use client";
import { shared } from './shared';
export const c = 'C' + shared;
`,
"components/D.ts": `
"use client";
import { shared } from './shared';
export const d = 'D' + shared;
`,
"components/shared.ts": `export const shared = '1';`,
},
async test(dev) {
await dev.fetch("/").equals("page: object,object,object,object");
// Invalidate every client component boundary in a single hot update.
await dev.write("components/shared.ts", `export const shared = '2';`);
await dev.fetch("/").equals("page: object,object,object,object");
// A second round exercises the watcher entries recorded during the
// previous (SSR-duplicated) bundle.
await dev.write("components/shared.ts", `export const shared = '3';`);
await dev.fetch("/").equals("page: object,object,object,object");
},
});
77 changes: 76 additions & 1 deletion test/bake/framework-router.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { frameworkRouterInternals } from "bun:internal-for-testing";
import { describe, expect, test } from "bun:test";
import { tempDirWithFiles } from "harness";
import { bunEnv, bunExe, isWindows, tempDirWithFiles } from "harness";
import path from "path";

const { parseRoutePattern, FrameworkRouter } = frameworkRouterInternals;
Expand Down Expand Up @@ -133,3 +133,78 @@ test("discovers from filesystem paths", () => {
],
});
});

// Regression: printing a route syntax error crashed the dev server instead of
// reporting it. The 65+ parameter check recorded its message without a cursor
// position, leaving `TinyLog.cursor_at` at the `u32::MAX` sentinel, and
// `TinyLog.print` sliced `rel_path[cursor_at..]` with it.
// The 65-directory fixture exceeds Windows' MAX_PATH; the code under test is
// platform-independent.
test.skipIf(isWindows)("dev server reports route syntax errors without crashing", async () => {
const manyParams = Array.from({ length: 65 }, (_, i) => `[p${i}]`).join("/");
const dir = tempDirWithFiles("fsr-syntax-error", {
"server.ts": `
export function render(req, meta) {
return meta.pageModule.default(req, meta);
}
`,
"main.ts": `
export default {
port: 0,
development: true,
app: {
framework: {
fileSystemRouterTypes: [
{ root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts" },
],
},
},
};
`,
"routes/index.tsx": `export default () => new Response("ok");`,
"routes/[bad.tsx": `export default () => new Response("bad");`,
[`routes/${manyParams}/index.tsx`]: `export default () => new Response("deep");`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "main.ts"],
cwd: dir,
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

// Drain stderr concurrently with the stdout scan below so neither pipe
// can fill up and block the child.
const stderrPromise = proc.stderr.text();

// The route scan runs during server startup, so "Started development
// server" only prints after both syntax errors were reported. Without the
// fix the process dies while printing the 65-param error.
let stdout = "";
let url: string | undefined;
const decoder = new TextDecoder();
for await (const chunk of proc.stdout) {
stdout += decoder.decode(chunk, { stream: true });
url = stdout.match(/Started development server: (http:\/\/\S+)/)?.[1];
if (url) break;
}
expect(url).toBeDefined();

// The server must still respond after reporting the errors.
const res = await fetch(url!);
expect(await res.text()).toBe("ok");

proc.kill();
const [stderr] = await Promise.all([stderrPromise, proc.exited]);
// The caret/underline must line up with the offending "[" — column
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// 8 for the `error: "` prefix plus 7 for `routes/`.
expect(stderr).toContain(
'error: "routes/[bad.tsx" is not a valid route\n' +
" ".repeat(8 + 7) +
"-------\n" +
" ".repeat(8 + 7) +
'Missing "]" to match this route parameter',
);
expect(stderr).toContain("Pattern cannot have more than 64 param");
});
Loading