Skip to content
Merged
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
8 changes: 8 additions & 0 deletions src/runtime/bake/config/bundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ impl Default for BuildConfigSubset {
}

impl SplitBundlerOptions {
/// `PluginJscExt::destroy` is the paired destructor of `Plugin::create`
/// in `parse_plugin_array`: it unprotects the JSCell.
pub(super) fn destroy_plugin(&mut self) {
if let Some(p) = self.plugin.take() {
Plugin::destroy(p.as_ptr());
}
}
Comment thread
alii marked this conversation as resolved.
Resolved

pub(super) fn parse_plugin_array(
&mut self,
plugin_array: JSValue,
Expand Down
10 changes: 5 additions & 5 deletions src/runtime/bake/config/framework.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,19 +497,19 @@ impl Framework {
])
};

let ignore_dirs = if let Some(exts_js) = fsr_opts.get(global, "ignoreDirs")? {
let ignore_dirs = if let Some(dirs_js) = fsr_opts.get(global, "ignoreDirs")? {
'exts: {
if exts_js.is_array() {
let mut it_2 = array.array_iterator(global)?;
let mut dirs = Vec::with_capacity(len as usize);
if dirs_js.is_array() {
let mut it_2 = dirs_js.array_iterator(global)?;
let mut dirs = Vec::with_capacity(dirs_js.get_length(global)? as usize);
while let Some(array_item) = it_2.next()? {
dirs.push(array_item.to_utf8(global)?);
Comment thread
alii marked this conversation as resolved.
Resolved
}
break 'exts dirs;
}

return Err(global.throw_invalid_arguments(format_args!(
"'ignoreDirs' must be an array of strings or \"*\" for all extensions"
"'ignoreDirs' must be an array of strings"
)));
}
} else {
Expand Down
39 changes: 23 additions & 16 deletions src/runtime/bake/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//! The Bake config: one type for both `bun index.html` (no config) and `bun ./app.ts` (an `app` export).

use crate::api::js_bundler::Plugin;
use crate::api::js_bundler::js_bundler::PluginJscExt as _;
use bun_alloc::Arena;
use bun_core::{ZBox, strings};
use bun_jsc::{JSGlobalObject, JSValue, JsResult};
Expand Down Expand Up @@ -29,13 +27,7 @@ pub(crate) struct UserOptions {

impl Drop for UserOptions {
fn drop(&mut self) {
if let Some(p) = self.bundler_options.plugin {
// `p` is the FFI handle returned by `Plugin::create` in
// `parse_plugin_array`; `PluginJscExt::destroy` is its paired
// (safe) destructor — it null-checks via `opaque_ref` and
// unprotect()s the JSCell / tombstones the C++ object.
Plugin::destroy(p.as_ptr());
}
self.bundler_options.destroy_plugin();
}
}

Expand Down Expand Up @@ -87,6 +79,26 @@ impl UserOptions {
}
}

match Self::parse_framework_root_and_plugins(config, global, &mut bundler_options) {
Ok((framework, root)) => Ok(UserOptions {
root,
framework,
bundler_options,
arena,
}),
Err(err) => {
// `UserOptions` was never built, so its `Drop` will not free the plugin.
bundler_options.destroy_plugin();
Err(err)
}
}
}

fn parse_framework_root_and_plugins(
config: JSValue,
global: &JSGlobalObject,
bundler_options: &mut SplitBundlerOptions,
) -> JsResult<(Framework, ZBox)> {
let framework = Framework::from_js(
match config.get(global, "framework")? {
Some(v) => v,
Expand All @@ -98,7 +110,7 @@ impl UserOptions {
}
},
global,
&mut bundler_options,
bundler_options,
)?;

let root = if let Some(slice) = config.get_optional_slice(global, "root")? {
Expand All @@ -117,11 +129,6 @@ impl UserOptions {
bundler_options.parse_plugin_array(plugin_array, global)?;
}

Ok(UserOptions {
root,
framework,
bundler_options,
arena,
})
Ok((framework, root))
}
}
2 changes: 2 additions & 0 deletions src/runtime/bake/router/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,8 @@ impl FrameworkRouter {

if param_count > 64 {
log.write(format_args!("Pattern cannot have more than 64 param"));
log.cursor_at = 0;
log.cursor_len = u32::try_from(full_rel_path.len()).expect("int cast");
ctx.on_router_syntax_error(full_rel_path, log)?;
arena_state.reset_retain_with_limit(8 * 1024 * 1024);
continue 'outer;
Expand Down
86 changes: 86 additions & 0 deletions test/bake/app-options.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,89 @@ test('Bun.serve({ app }) reports a framework entry point that a "browser" map di
expect(stdout).toBe("threw: Framework is missing required files!\n");
expect(exitCode).toBe(0);
});

test.concurrent("Bun.serve({ app }) frees the bundler plugin when the config is rejected", async () => {
using dir = tempDir("bake-app-plugin-leak", {
"fixture.ts": `
import { heapStats } from "bun:jsc";
const fsr = { root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts" };
// The framework's plugins are parsed first, then root fails the type check.
const app = { framework: { fileSystemRouterTypes: [fsr], plugins: [] }, root: 123 };
const count = () => heapStats().protectedObjectTypeCounts.BundlerPlugin ?? 0;
const before = count();
for (let i = 0; i < 10; i++) {
try {
Bun.serve({ port: 0, development: true, app, fetch: () => new Response() }).stop(true);
} catch {}
}
Comment thread
alii marked this conversation as resolved.
Resolved
console.log("leaked plugins: " + (count() - before));
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.ts"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stdout).toBe("leaked plugins: 0\n");
expect(exitCode).toBe(0);
});

test.concurrent("Bun.serve({ app }) skips the directories in fileSystemRouterTypes[].ignoreDirs", async () => {
using dir = tempDir("bake-app-ignore-dirs", {
"server.ts": `export function render() { return new Response("ok"); }`,
"routes/index.ts": `export default 1;`,
// Both routes are invalid. Only the one outside ignoreDirs is scanned and reported.
"routes/skipped/[].ts": `export default 1;`,
"routes/scanned/[].ts": `export default 1;`,
"fixture.ts": `
const fsr = { root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts", ignoreDirs: ["skipped"] };
Bun.serve({ port: 0, development: true, app: { framework: { fileSystemRouterTypes: [fsr] } } }).stop(true);
console.log("started");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.ts"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toContain("scanned/[].ts");
expect(stderr).not.toContain("skipped/[].ts");
expect(stdout).toBe("started\n");
expect(exitCode).toBe(0);
});

test.concurrent("Bun.serve({ app }) reports a route with more than 64 params", async () => {
const route = Array.from({ length: 65 }, (_, i) => `[p${i}]`).join("/");
using dir = tempDir("bake-app-too-many-params", {
"server.ts": `export function render() { return new Response("ok"); }`,
[`routes/${route}.ts`]: `export default 1;`,
"fixture.ts": `
const fsr = { root: "routes", style: "nextjs-pages", serverEntryPoint: "./server.ts" };
Bun.serve({ port: 0, development: true, app: { framework: { fileSystemRouterTypes: [fsr] } } }).stop(true);
console.log("started");
`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "fixture.ts"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toContain("Pattern cannot have more than 64 param");
expect(stdout).toBe("started\n");
expect(exitCode).toBe(0);
Comment thread
robobun marked this conversation as resolved.
Resolved
});
Loading