diff --git a/src/bun_core/output.rs b/src/bun_core/output.rs index 3274e5c074cb..e0f1aa363edc 100644 --- a/src/bun_core/output.rs +++ b/src/bun_core/output.rs @@ -1797,7 +1797,7 @@ impl fmt::Display for PrettyBuf { /// Positional-argument bundle for runtime template substitution. pub trait FmtTuple { /// Write the `idx`-th positional into `f`. Returns `false` if `idx` is out - /// of range (caller emits the literal `{}` then). + /// of range. fn write_nth(&self, idx: usize, f: &mut dyn fmt::Write) -> Result; fn len(&self) -> usize; } @@ -1877,7 +1877,7 @@ impl_fmt_tuple!(0 A, 1 B, 2 C, 3 D, 4 E, 5 F, 6 G, 7 H); /// Substitute `{}` / `{s}` / `{d}` / `{any}` / `{f}` placeholders in `template` /// with successive entries from `args`. `{{` / `}}` are emitted as literal -/// braces. Unrecognised specs are passed through verbatim. +/// braces. The spec inside any other `{...}` is ignored. fn substitute_template( template: &[u8], args: &impl FmtTuple, @@ -1901,7 +1901,14 @@ fn substitute_template( } if j < t.len() { // consume placeholder - if args.write_nth(argi, f)? { + let filled = args.write_nth(argi, f)?; + debug_assert!( + filled, + "template has more placeholders than the {} arg(s) passed with it (a format_args! counts as one; pass a tuple): {:?}", + args.len(), + bstr::BStr::new(t), + ); + if filled { argi += 1; } i = j + 1; diff --git a/src/runtime/cli/pack_command.rs b/src/runtime/cli/pack_command.rs index a1ce7dd2c0e1..4b767be83280 100644 --- a/src/runtime/cli/pack_command.rs +++ b/src/runtime/cli/pack_command.rs @@ -2713,7 +2713,7 @@ pub(crate) fn pack( Output::err( err, "failed to stat file: \"{}\"", - format_args!("{}", file.handle), + format_args!("{}", bstr::BStr::new(item.path.as_bytes())), ); Global::crash(); } @@ -3000,8 +3000,7 @@ fn tarball_destination<'a>( if !pack_filename.is_empty() && !pack_destination.is_empty() { Output::err_generic( "cannot use both filename and destination at the same time with tarball: filename \"{}\" and destination \"{}\"", - format_args!( - "{} {}", + ( bstr::BStr::new(strings::without_trailing_slash(pack_filename)), bstr::BStr::new(strings::without_trailing_slash(pack_destination)), ), @@ -3045,13 +3044,12 @@ fn tarball_destination<'a>( if res.is_err() { Output::err_generic( "archive destination name too long: \"{}/{}\"", - format_args!( - "{}/{}", + ( bstr::BStr::new(strings::without_trailing_slash(&dest_buf[..dir_len_full])), fmt_tarball_filename( package_name, package_version, - TarballNameStyle::Normalize + TarballNameStyle::Normalize, ), ), ); @@ -3654,8 +3652,7 @@ impl IgnorePatterns { Output::err( err, "failed to {} {} at: \"{}{}{}\"", - format_args!( - "{} {} {}{}{}", + ( <&str>::from(reason), <&str>::from(ignore_kind), bstr::BStr::new(strings::without_trailing_slash(dir_path)), diff --git a/test/cli/install/bun-pack.test.ts b/test/cli/install/bun-pack.test.ts index 77fe7a55685a..b89765712514 100644 --- a/test/cli/install/bun-pack.test.ts +++ b/test/cli/install/bun-pack.test.ts @@ -2,7 +2,7 @@ import { file, spawn, write } from "bun"; import { readTarball } from "bun:internal-for-testing"; import { beforeEach, describe, expect, test } from "bun:test"; import { exists, mkdir, rm } from "fs/promises"; -import { bunEnv, bunExe, pack, runBunInstall, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, pack, runBunInstall, tempDir, tmpdirSync } from "harness"; import fs from "node:fs/promises"; import { join } from "path"; @@ -331,6 +331,31 @@ describe("flags", () => { }); } + // The destination directory ends `headroom` bytes short of the path buffer, so it still fits, + // but the longer "/-.tgz" appended to it does not. Windows command lines cannot + // carry an argument anywhere near its path buffer size. + test.skipIf(isWindows)("--destination with no room left for the tarball name", async () => { + const pathMax = isLinux ? 4096 : 1024; + const headroom = 100; + const name = `pack-dest-too-long-${Buffer.alloc(2 * headroom, "n").toString()}`; + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name, + version: "1.0.0", + }), + ), + write(join(packageDir, "index.js"), "console.log('hello ./index.js')"), + ]); + + // packageDir + "/" + filler is `headroom` bytes short of the buffer + const filler = Buffer.alloc(pathMax - headroom - Buffer.byteLength(packageDir) - 1, "d").toString(); + const dest = join(packageDir, filler); + const { err } = await packExpectError(packageDir, bunEnv, `--destination=${dest}`); + expect(err).toContain(`error: archive destination name too long: "${dest}/${name}-1.0.0.tgz"\n`); + }); + const filenameTests = [ { filename: "test.tgz", @@ -394,7 +419,12 @@ describe("flags", () => { write(join(packageDir, "index.js"), "console.log('hello ./index.js')"), ]); - expect(async () => await pack(packageDir, bunEnv, "--filename=test.tgz", "--destination=packed")).toThrowError(); + const { err } = await packExpectError(packageDir, bunEnv, "--filename=test.tgz", "--destination=packed"); + expect(err).toContain( + 'error: cannot use both filename and destination at the same time with tarball: filename "test.tgz" and destination "packed"', + ); + expect(await exists(join(packageDir, "test.tgz"))).toBeFalse(); + expect(await exists(join(packageDir, "packed"))).toBeFalse(); }); test("--ignore-scripts", async () => { @@ -1405,6 +1435,26 @@ describe(".gitignore/.npmignore", () => { }); } + for (const ignoreFile of [".gitignore", ".npmignore"]) { + test(`reports which ${ignoreFile} could not be read`, async () => { + await Promise.all([ + write( + join(packageDir, "package.json"), + JSON.stringify({ + name: "pack-ignore-unreadable", + version: "1.0.0", + }), + ), + write(join(packageDir, "subdir", "index.js"), "console.log('hello ./subdir/index.js')"), + // a directory where the ignore file is expected: opening it succeeds, reading it fails + mkdir(join(packageDir, "subdir", ignoreFile), { recursive: true }), + ]); + + const { err } = await packExpectError(packageDir, bunEnv); + expect(err).toContain(`EISDIR: failed to read ${ignoreFile} at: "${join(packageDir, "subdir", ignoreFile)}"\n`); + }); + } + test("excludes files recursively", async () => { await Promise.all([ write(