Skip to content

install: escape control characters in resolutions, specifiers, bin names and registry error text #38631

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 8 commits into
base: main
Choose a base branch
from
Open

install: escape control characters in resolutions, specifiers, bin names and registry error text #38631

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ff5f463
install: escape control characters in resolutions, specifiers, bin na…
robobun Aug 14, 2026
b0c69d3
install: escape the resolution in the verbose tarball retry warning too
robobun Aug 14, 2026
58a48bf
install: escape the remaining resolution and specifier prints
robobun Aug 14, 2026
62f351a
test: the resolve trace prints the workspace path with the platform s…
robobun Aug 14, 2026
e83afa4
install: escape resolutions in bun patch, git and package.json error …
robobun Aug 14, 2026
fe22663
ci: retrigger
robobun Aug 14, 2026
2fcf0d2
fmt: scan for escapable bytes with the SIMD string helpers
robobun Aug 15, 2026
1e3a599
fmt: shorten the scanner comment
robobun Aug 15, 2026
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
44 changes: 44 additions & 0 deletions src/bun_core/fmt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3325,6 +3325,50 @@ fn escape_powershell_impl(str: &[u8], writer: &mut impl fmt::Write) -> fmt::Resu
write_bytes(writer, remain)
}

// ───────────────────────────────────────────────────────────────────────────
// escapeControlChars
// ───────────────────────────────────────────────────────────────────────────
Comment thread
robobun marked this conversation as resolved.

/// `Display` adapter that spells out C0 controls, DEL and C1 controls
/// (`\n`, `\x1b`, `\x7f`, `\u009b`, ...) so text authored by a dependency
/// cannot erase, repaint or forge lines of terminal output when printed.
Comment thread
robobun marked this conversation as resolved.
pub struct EscapeControlChars<T>(pub T);

/// [`EscapeControlChars`] over raw bytes; invalid UTF-8 renders as U+FFFD.
pub fn escape_control_chars(text: &[u8]) -> EscapeControlChars<&bstr::BStr> {
EscapeControlChars(bstr::BStr::new(text))
}

impl<T: Display> Display for EscapeControlChars<T> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let mut writer = EscapeControlCharsWriter(f);
write!(writer, "{}", self.0)
}
}

struct EscapeControlCharsWriter<'a, 'f>(&'a mut Formatter<'f>);

impl fmt::Write for EscapeControlCharsWriter<'_, '_> {
fn write_str(&mut self, s: &str) -> fmt::Result {
let mut start = 0;
for (i, c) in s.char_indices() {
Comment thread
robobun marked this conversation as resolved.
Outdated
if !matches!(c, '\0'..='\x1f' | '\x7f' | '\u{80}'..='\u{9f}') {
continue;
}
self.0.write_str(&s[start..i])?;
match c {
'\n' => self.0.write_str("\\n")?,
'\r' => self.0.write_str("\\r")?,
'\t' => self.0.write_str("\\t")?,
c if c.is_ascii() => write!(self.0, "\\x{:02x}", c as u32)?,
c => write!(self.0, "\\u{:04x}", c as u32)?,
}
start = i + c.len_utf8();
}
self.0.write_str(&s[start..])
}
}

// js_bindings (fmtString for highlighter.test.ts) lives in src/jsc/fmt_jsc.rs
// alongside fmt_jsc.bind.ts; bun_core/ stays JSC-free.

Expand Down
30 changes: 19 additions & 11 deletions src/install/PackageManager/PackageManagerEnqueue.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,8 +806,8 @@
bun_ast::Loc::EMPTY,
format_args!(
"Package \"{}\" with tag \"{}\" not found, but package exists",
bstr::BStr::new(this.lockfile.str(&name)),
bstr::BStr::new(
bun_fmt::escape_control_chars(this.lockfile.str(&name)),
bun_fmt::escape_control_chars(
this.lockfile.str(&version.dist_tag().tag)
),
),
Expand All @@ -825,8 +825,10 @@
None,
bun_ast::Loc::EMPTY,
"No version matching \"{}\" found for specifier \"{}\"<r> <d>(but package exists)<r>",
bstr::BStr::new(this.lockfile.str(&version.literal)),
bstr::BStr::new(this.lockfile.str(&name)),
bun_fmt::escape_control_chars(
this.lockfile.str(&version.literal)
),
bun_fmt::escape_control_chars(this.lockfile.str(&name)),
);
}
}
Expand All @@ -844,8 +846,10 @@
None,
bun_ast::Loc::EMPTY,
"Package \"{}\" with tag \"{}\" not found<r> <d>(all versions blocked by minimum-release-age: {} seconds)<r>",
bstr::BStr::new(this.lockfile.str(&name)),
bstr::BStr::new(
bun_fmt::escape_control_chars(
this.lockfile.str(&name)
),
bun_fmt::escape_control_chars(
this.lockfile.str(&version.dist_tag().tag)
),
age_gate_ms / MS_PER_S,
Expand All @@ -856,10 +860,12 @@
None,
bun_ast::Loc::EMPTY,
"No version matching \"{}\" found for specifier \"{}\"<r> <d>(blocked by minimum-release-age: {} seconds)<r>",
bstr::BStr::new(this.lockfile.str(&name)),
bstr::BStr::new(
bun_fmt::escape_control_chars(
this.lockfile.str(&name)
),
bun_fmt::escape_control_chars(
this.lockfile.str(&version.literal)
),

Check notice on line 868 in src/install/PackageManager/PackageManagerEnqueue.rs

View check run for this annotation

Claude / Claude Code Review

Swapped format args in age-gated 'No version matching' error 

Pre-existing: the age-gated `No version matching` branch has its format arguments swapped — it passes `(name, version.literal)` where the sibling branch at line 827 correctly passes `(version.literal, name)`, so this prints e.g. `No version matching "lodash" found for specifier "^1.2.3"`. Since this PR rewrites both arguments anyway, worth swapping them to match the sibling.
Comment thread
robobun marked this conversation as resolved.
age_gate_ms / MS_PER_S,
);
}
Expand All @@ -877,8 +883,8 @@
bun_ast::Loc::EMPTY,
format_args!(
"Could not find package.json for \"file:{}\" dependency \"{}\"",
bstr::BStr::new(this.lockfile.str(version.folder())),
bstr::BStr::new(this.lockfile.str(&name)),
bun_fmt::escape_control_chars(this.lockfile.str(version.folder())),
bun_fmt::escape_control_chars(this.lockfile.str(&name)),
),
);
} else {
Expand All @@ -887,7 +893,9 @@
bun_ast::Loc::EMPTY,
format_args!(
"Could not find package.json for dependency \"{}\"",
bstr::BStr::new(this.lockfile.str(&name)),
bun_fmt::escape_control_chars(
this.lockfile.str(&name)
),
),
);
}
Comment thread
claude[bot] marked this conversation as resolved.
Expand Down
12 changes: 9 additions & 3 deletions src/install/PackageManager/PackageManagerResolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,20 @@ impl PackageManager {
{
Output::err_generic(
"<b>{}<r><d> failed to resolve<r>",
(failed_dep.version.literal.fmt(string_buf),),
(bun_core::fmt::escape_control_chars(
failed_dep.version.literal.slice(string_buf),
),),
);
} else {
Output::err_generic(
"<b>{}<r><d>@<b>{}<r><d> failed to resolve<r>",
(
bstr::BStr::new(failed_dep.name.slice(string_buf)),
failed_dep.version.literal.fmt(string_buf),
bun_core::fmt::escape_control_chars(
failed_dep.name.slice(string_buf),
),
bun_core::fmt::escape_control_chars(
failed_dep.version.literal.slice(string_buf),
),
),
);
}
Expand Down
26 changes: 15 additions & 11 deletions src/install/PackageManager/runTasks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
use crate::lifecycle_script_runner::InstallCtx;
use crate::network_task::{Authorization, ForTarballError};
use crate::package_manifest_map::Value as ManifestEntry;
use bun_core::fmt::PathSep;
use bun_core::fmt::{EscapeControlChars, PathSep, escape_control_chars};
use bun_install::lockfile::Package;
use bun_install::package_manager_task as Task;
// Import the *module* under the `Options` name so `Options::LogLevel` resolves as a path
Expand Down Expand Up @@ -746,10 +746,12 @@
bun_ast::Loc::EMPTY,
"{} downloading tarball <b>{}@{}<r>",
err.name(),
bstr::BStr::new(extract.name.slice()),
extract
.resolution
.fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,),
escape_control_chars(extract.name.slice()),
EscapeControlChars(
extract
.resolution
.fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto),
),

Check warning on line 754 in src/install/PackageManager/runTasks.rs

View check run for this annotation

Claude / Claude Code Review

Verbose tarball retry warning still prints resolution unescaped 

The verbose-mode `Retrying {}/{}...` warning ~30 lines above still passes `extract.resolution.fmt(...)` raw — the same field this hunk wraps in `EscapeControlChars` at the final-failure sites. A registry-controlled `dist.tarball` URL carrying a C1 byte that answers 5xx under `--verbose` still emits the raw byte on each retry line. Same one-line fix: wrap it (and `extract.name.slice()`) as done here.
Comment thread
robobun marked this conversation as resolved.
);
} else {
bun_ast::add_warning_pretty!(
Expand All @@ -758,10 +760,12 @@
bun_ast::Loc::EMPTY,
"{} downloading tarball <b>{}@{}<r>",
err.name(),
bstr::BStr::new(extract.name.slice()),
extract
.resolution
.fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto,),
escape_control_chars(extract.name.slice()),
EscapeControlChars(
extract
.resolution
.fmt(&manager.lockfile.buffers.string_bytes, PathSep::Auto),
),
);
}
if manager.subcommand != Subcommand::Remove {
Expand Down Expand Up @@ -834,7 +838,7 @@
None,
bun_ast::Loc::EMPTY,
"<r><red><b>GET<r><red> {}<d> - {}<r>",
bstr::BStr::new(metadata.url.slice()),
escape_control_chars(metadata.url.slice()),
response.status_code,
);
} else {
Expand All @@ -843,7 +847,7 @@
None,
bun_ast::Loc::EMPTY,
"<r><yellow><b>GET<r><yellow> {}<d> - {}<r>",
bstr::BStr::new(metadata.url.slice()),
escape_control_chars(metadata.url.slice()),
response.status_code,
);
}
Expand Down
29 changes: 15 additions & 14 deletions src/install/isolated_install/Installer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::io::Write as _;

use bun_ast::Log;
use bun_collections::{ArrayHashMap, DynamicBitSet, StringHashMap};
use bun_core::fmt::{EscapeControlChars, escape_control_chars};
use bun_core::{Environment, Global, Output};
use bun_core::{ZStr, strings};
use bun_paths::{self as paths, AbsPath, AutoAbsPath, AutoRelPath};
Expand Down Expand Up @@ -255,10 +256,10 @@ impl<'a> Installer<'a> {
Output::err_generic(
"failed to download <b>{}@{}<r>: {}\n <d>{}<r>",
(
bstr::BStr::new(name),
resolution.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(name),
EscapeControlChars(resolution.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
bstr::BStr::new(download_error_reason(err)),
bstr::BStr::new(url),
escape_control_chars(url),
),
);
Output::flush();
Expand Down Expand Up @@ -327,8 +328,8 @@ impl<'a> Installer<'a> {
link_err.clone(),
"failed to link package: {}@{}",
(
bstr::BStr::new(pkg_name.slice(string_buf)),
pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(pkg_name.slice(string_buf)),
EscapeControlChars(pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
),
);
}
Expand All @@ -337,17 +338,17 @@ impl<'a> Installer<'a> {
symlink_err.clone(),
"failed to symlink dependencies for package: {}@{}",
(
bstr::BStr::new(pkg_name.slice(string_buf)),
pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(pkg_name.slice(string_buf)),
EscapeControlChars(pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
),
);
}
TaskError::Patching(patch_log) => {
Output::err_generic(
"failed to patch package: {}@{}",
(
bstr::BStr::new(pkg_name.slice(string_buf)),
pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(pkg_name.slice(string_buf)),
EscapeControlChars(pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
),
);
let _ = patch_log.print(std::ptr::from_mut(Output::error_writer()));
Expand All @@ -357,19 +358,19 @@ impl<'a> Installer<'a> {
*bin_err,
"failed to link binaries for package: {}@{}",
(
bstr::BStr::new(pkg_name.slice(string_buf)),
pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(pkg_name.slice(string_buf)),
EscapeControlChars(pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
),
);
}
TaskError::Download(dl) => {
Output::err_generic(
"failed to download <b>{}@{}<r>: {}\n <d>{}<r>",
(
bstr::BStr::new(pkg_name.slice(string_buf)),
pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto),
escape_control_chars(pkg_name.slice(string_buf)),
EscapeControlChars(pkg_res.fmt(string_buf, bun_core::fmt::PathSep::Auto)),
bstr::BStr::new(download_error_reason(dl.err)),
bstr::BStr::new(&dl.url),
escape_control_chars(&dl.url),
),
);
}
Expand Down
Loading
Loading