diff --git a/src/bundler/defines.rs b/src/bundler/defines.rs index 8897c2649eb2..fb0ae068822c 100644 --- a/src/bundler/defines.rs +++ b/src/bundler/defines.rs @@ -51,12 +51,8 @@ fn env_string_store_put( key: &[u8], value: &[u8], ) -> Result<(), crate::Error> { - // The `E.String` slab must NOT live in the thread-local - // `Expr.Data.Store` — `configureDefines` resets that store on return, so - // the env-define payloads must outlive it. Allocate from `bump` (the - // transpiler arena) so the slab is bulk-freed with the `Define` table - // instead of leaking a `Box` per env var. Value bytes alias the long-lived - // env-map storage. + // Copied because `Bun__setEnvValue` frees the env-map entry `value` points into. + let value: &[u8] = bump.alloc_slice_copy(value); let value: ExprData = ExprData::EString(bun_ast::StoreRef::from_bump( bump.alloc(bun_ast::E::EString::init(value)), )); diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index 57b4464602a8..ab2858f0663f 100644 --- a/src/dotenv/env_loader.rs +++ b/src/dotenv/env_loader.rs @@ -336,6 +336,7 @@ impl Loader { /// Get proxy URL for HTTP/HTTPS requests, respecting NO_PROXY. /// `hostname` is the host without port (e.g., "localhost") /// `host` is the host with port if present (e.g., "localhost:3000") + /// The returned URL borrows the map entry, which `Map::put` frees. pub fn get_http_proxy( &self, is_http: bool, @@ -1394,6 +1395,7 @@ impl Map { } } + /// Frees the old value; JS hits this via `Bun__setEnvValue`, so don't hold borrows across JS. #[inline] pub fn put(&mut self, key: &[u8], value: &[u8]) -> Result<(), AllocError> { #[cfg(all(windows, debug_assertions))] diff --git a/src/jsc/rare_data.rs b/src/jsc/rare_data.rs index 8476ef60cb1b..8ca9cbf26261 100644 --- a/src/jsc/rare_data.rs +++ b/src/jsc/rare_data.rs @@ -537,8 +537,7 @@ impl ProxyEnvSlots { // RefCountedEnvValue // ────────────────────────────────────────────────────────────────────────── -/// A ref-counted heap-allocated byte slice. The env map stores borrowed -/// `.bytes` slices; as long as any VM holds a ref, the bytes stay valid. +/// Last JS-assigned proxy value, shared with later Workers; the env map keeps its own copy. /// /// Holders are `Arc` (per LIFETIMES.tsv): the refcount /// lives in the `Arc` header, so ref/deref are `Arc::clone`/`drop`. diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index 6c6d3657029c..8832a384ab79 100644 --- a/test/bake/dev/bundle.test.ts +++ b/test/bake/dev/bundle.test.ts @@ -1,6 +1,6 @@ // Bundle tests are tests concerning bundling bugs that only occur in DevServer. import { expect } from "bun:test"; -import { devTest, emptyHtmlFile, minimalFramework } from "../bake-harness"; +import { Dev, devTest, emptyHtmlFile, minimalFramework } from "../bake-harness"; devTest("import identifier doesnt get renamed", { framework: minimalFramework, @@ -865,3 +865,57 @@ devTest("barrel optimization: namespace re-export cycle through a star-exported await c.expectMessage("result: object Y KEEP DEEP OTHER"); }, }); + +// With `[serve.static] env = "inline"`, DevServer builds its define table from +// the VM's env map once at startup and keeps it for the life of the server. +// Assigning a proxy variable on process.env replaces that variable's entry in +// the env map, so the define must own a copy of the value instead of pointing +// into the map; otherwise every rebuild of a module that reads the variable +// inlines freed memory. +const startupProxy = "http://proxy-at-startup.example:8080/" + Buffer.alloc(120, "a").toString(); +async function clientBundle(dev: Dev) { + const html = await dev.fetch("/").text(); + const scripts = [...html.matchAll(/src="([^"]+\.js)"/g)]; + expect(scripts).toHaveLength(1); + return dev.fetch(scripts[0][1]).text(); +} +devTest("inlined env var survives a runtime process.env write to a proxy variable", { + files: { + "bunfig.toml": ` + [serve.static] + env = "inline" + `, + "index.html": emptyHtmlFile({ scripts: ["index.ts"] }), + "index.ts": `console.log("v1", process.env.HTTPS_PROXY);`, + "bun.app.ts": ` + import html from "./index.html"; + export default { + static: { "/": html }, + fetch(req) { + if (new URL(req.url).pathname === "/set-proxy") { + process.env.HTTPS_PROXY = "http://changed-at-runtime.example:1/"; + return new Response("ok"); + } + return new Response("Not Found", { status: 404 }); + }, + }; + `, + }, + htmlFiles: [], + env: { HTTPS_PROXY: startupProxy }, + async test(dev) { + const before = await clientBundle(dev); + expect(before).toContain('"v1"'); + expect(before).toContain(startupProxy); + + await dev.fetch("/set-proxy").equals("ok"); + + // Rebuilding index.ts prints the define again, now that the env map entry + // it was created from is gone. + await dev.write("index.ts", `console.log("v2", process.env.HTTPS_PROXY);`); + const after = await clientBundle(dev); + expect(after).toContain('"v2"'); + expect(after).toContain(startupProxy); + expect(after).not.toContain("changed-at-runtime"); + }, +}); diff --git a/test/bundler/bundler_env.test.ts b/test/bundler/bundler_env.test.ts index b87ab47ab605..1f33d41316b3 100644 --- a/test/bundler/bundler_env.test.ts +++ b/test/bundler/bundler_env.test.ts @@ -1,4 +1,5 @@ -import { describe } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; import { itBundled } from "./expectBundled"; for (let backend of ["api", "cli"] as const) { @@ -118,3 +119,78 @@ for (let backend of ["api", "cli"] as const) { }); }); } + +// The define table is built from the env map when the build is configured. +// Assigning one of the proxy variables on process.env (the only process.env +// writes that reach the native env map) replaces the map entry that define was +// built from, so the define has to own its value: here that write happens +// while the build is still running, from a macro and from a plugin. +const proxyAtStart = "http://proxy-at-start.example:8080/" + Buffer.alloc(120, "a").toString(); + +describe("bundler/cli", () => { + itBundled("env/inline survives macro proxy write", { + backend: "cli", + dotenv: "inline", + env: { HTTPS_PROXY: proxyAtStart }, + files: { + "/a.ts": /* ts */ ` + import { setProxy } from "./macro.ts" with { type: "macro" }; + setProxy(); + console.log(process.env.HTTPS_PROXY); + `, + "/macro.ts": /* ts */ ` + export function setProxy() { + process.env.HTTPS_PROXY = "http://changed-by-macro.example:1/"; + return 0; + } + `, + }, + onAfterBundle(api) { + api.expectFile("/out.js").toContain(proxyAtStart); + api.expectFile("/out.js").not.toContain("changed-by-macro"); + }, + run: { + env: { HTTPS_PROXY: "http://not-inlined.example:1/" }, + stdout: proxyAtStart + "\n", + }, + }); +}); + +describe("bundler/api", () => { + test.concurrent("env: inline survives a plugin assigning a proxy variable during the build", async () => { + using dir = tempDir("bundler-env-inline-plugin-proxy", { + "entry.ts": `export const replaced = "by the plugin";`, + "build.ts": /* ts */ ` + const result = await Bun.build({ + entrypoints: ["./entry.ts"], + env: "inline", + plugins: [{ + name: "assign-proxy", + setup(build) { + build.onLoad({ filter: /entry\\.ts$/ }, () => { + process.env.HTTPS_PROXY = "http://changed-by-plugin.example:1/"; + return { loader: "ts", contents: "export const proxy = process.env.HTTPS_PROXY;" }; + }); + }, + }], + }); + if (!result.success) throw new AggregateError(result.logs, "build failed"); + process.stdout.write(await result.outputs[0].text()); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build.ts"], + cwd: String(dir), + env: { ...bunEnv, HTTPS_PROXY: proxyAtStart }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout).toContain(`var proxy = ${JSON.stringify(proxyAtStart)};`); + expect(stdout).not.toContain("changed-by-plugin"); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +}); diff --git a/test/js/web/workers/worker.test.ts b/test/js/web/workers/worker.test.ts index 49e7a197ed19..cff37fbe1bac 100644 --- a/test/js/web/workers/worker.test.ts +++ b/test/js/web/workers/worker.test.ts @@ -168,6 +168,44 @@ describe("web worker", () => { expect(exitCode).toBe(0); }); + // A worker's transpiler builds its process.env defines from the worker's env + // map when the worker starts. Assigning a proxy variable replaces that map + // entry, so a module transpiled afterwards must not read the old entry's + // memory. Spawned so the launch value is set without touching this + // process's proxy settings. + test("worker-env: a module transpiled after the worker assigns HTTPS_PROXY sees a real value", async () => { + const launchValue = "http://proxy-at-launch.example:8080/" + Buffer.alloc(120, "a").toString(); + const workerValue = "http://assigned-in-worker.example:1/" + Buffer.alloc(120, "b").toString(); + using dir = tempDir("worker-env-proxy-define", { + "main.ts": ` + const worker = new Worker(new URL("./worker.ts", import.meta.url).href); + worker.onerror = e => { console.error(e.message); process.exit(1); }; + worker.onmessage = e => { + console.log(JSON.stringify(e.data)); + worker.terminate(); + }; + `, + "worker.ts": ` + process.env.HTTPS_PROXY = ${JSON.stringify(workerValue)}; + const { proxy } = await import("./mod.ts"); + postMessage(proxy); + `, + "mod.ts": `export const proxy = process.env.HTTPS_PROXY;`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], + cwd: String(dir), + env: { ...bunEnv, HTTPS_PROXY: launchValue }, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + // Either value is a correct answer (it depends on whether worker + // transpiles inline process.env reads); bytes of a freed map entry are not. + expect(JSON.parse(stdout)).toBeOneOf([launchValue, workerValue]); + expect(exitCode).toBe(0); + }); + test("worker-env with a lot of properties", done => { const obj: any = {};