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 2 commits
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,8 +860,10 @@
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)
),
Comment thread
robobun marked this conversation as resolved.
age_gate_ms / MS_PER_S,
Expand All @@ -877,20 +883,22 @@
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 {
this.log_mut().add_error_fmt(
None,
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)
),
),
);
}

Check warning on line 901 in src/install/PackageManager/PackageManagerEnqueue.rs

View check run for this annotation

Claude / Claude Code Review

Verbose resolve trace still prints version.literal and resolution.fmt() unescaped 

The `verbose_install()` resolve trace here (and its byte-identical copy at lines 1436-1448) still prints `bstr::BStr::new(label)` (= `version.literal`) and `result.package.resolution.fmt(...)` raw - the same two fields this PR wraps in `escape_control_chars` / `EscapeControlChars` in the error branches directly above and at the `--verbose` retry site fixed in b0c69d39. A tarball/git specifier carrying a C1 byte prints raw here under `bun install --verbose`. Same fix: wrap `label` and the resolut
Comment thread
claude[bot] marked this conversation as resolved.
}
return Ok(());
} else {
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
36 changes: 21 additions & 15 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::isolated_install::store::{EntryColumns as _, NodeColumns as _};
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 @@ -676,10 +676,12 @@ pub fn run_tasks<C: RunTasksCallbacks>(
bun_ast::Loc::EMPTY,
"<r><yellow>warn:<r> {} downloading tarball <b>{}@{}<r>. Retrying {}/{}...",
bstr::BStr::new(err.name().as_bytes()),
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),
),
task.retried,
manager.options.max_retry_count,
);
Expand Down Expand Up @@ -746,10 +748,12 @@ pub fn run_tasks<C: RunTasksCallbacks>(
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),
),
Comment thread
robobun marked this conversation as resolved.
);
} else {
bun_ast::add_warning_pretty!(
Expand All @@ -758,10 +762,12 @@ pub fn run_tasks<C: RunTasksCallbacks>(
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 +840,7 @@ pub fn run_tasks<C: RunTasksCallbacks>(
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 +849,7 @@ pub fn run_tasks<C: RunTasksCallbacks>(
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