pm licenses: escape control characters in text output (adds bun_core::fmt::escape_control_chars)
Closed
RO
1mo ago+123+123+012345678901234567890123456789-36-36-01234567890123456789
Problem
bun pm licenses(new in #38333, not in any release yet) prints text it did not write: thelicense,author,descriptionandhomepagefields of every installed package.json, and each package's name and resolution.- Its sanitizer,
printable()insrc/runtime/cli/pm_licenses_command.rs, only dropped bytes matchingu8::is_ascii_control. C1 controls (U+0080..U+009F; U+009B is the one-character form ofESC [, and xterm-style terminals execute it) went through raw in the license header and in the--longlines. - The
name@versionline was not passed throughprintable()at all. The version column is the resolution rendered to text (resolution.fmt(...)at the top ofexec): for a tarball, folder or git dependency that is a path or URL taken from some package.json in the tree, and the name of such a package is whatever its own package.json says. A tarball nameddep-<U+009B>.tgzwhose manifest is nameddep-<U+009B>installs today and was printed as-is on both sides of the@. - Dropping the bytes also misrepresented the data: a license of
MIT<ESC>[31m<LF>EVILwas printed asMIT[31mEVIL. - Found while reviewing the command's output; there is no user report. Since the command is unreleased, changing what it prints for these fields costs nothing now.
Fix
- Adds
bun_core::fmt::escape_control_chars/EscapeControlChars<T: Display>: aDisplayadapter that writes C0 controls, DEL and C1 controls as\n,\r,\t,\xNN(other C0 and DEL) and\uNNNN(C1). Everything else passes through unchanged, backslashes included: the output is for reading,--jsonis the machine-readable form. pm licensesprints the license header, the name, the version and the--longfields through it;printable()is deleted.- Escaping at the print site is the right layer: the raw strings are still what groups and sorts entries, what is compared with the installed version, and what
--jsonemits.--jsonis unchanged; its string escaping already makes the output parse back to the original bytes, which the tests check. - The helper is the one several open package manager PRs need, and this PR is meant to be the one that lands it; the others rebase onto it and drop their copy (they cannot all auto-merge against each other, because the copies are not all the same):
- #38631 carries this exact hunk (it is where the SIMD scan below was reviewed), so its copy disappears on rebase.
- #38525, #38615, #38673 and #38536 carry an earlier
char_indicesbody with the same output (#38536 also adds a multi-line variant). They take this copy; #38536 rebuilds its variant on it. - #38557 carries a byte-slice variant that spells ESC as
\^[; it switches to this one and respells its assertions. - #38977 wraps the same version column in
redacted(..); with this helper generic overDisplaythat becomesEscapeControlChars(redacted(..)).
- The escape spelling is pinned by
test/cli/install/bun-pm-licenses.test.tsrather than by a Rust unit test:cargo test -p bun_coredoes not link (the crate's SIMD entry points such ashighway_memmemlive in the C++ build), and CI runs no unit tests for it. - Verified with
test/cli/install/bun-pm-licenses.test.ts: three existing control-character assertions move from the dropped to the escaped spelling, and a new case installs a tarball with U+009B in its file name and in its package name, and U+009B, DEL, a backslash and U+00A9 (same UTF-8 lead byte as the C1 range) in the manifest fields, then checks every line of the text output and the--jsonround trip. Against a build of main these four tests fail (raw U+009B in the output,MIT[31mEVILinstead ofMIT\x1b[31m\nEVIL); with the fix all 81 tests in the file pass. test/internal/source-lintspasses;cargo clippy -p bun_coreis clean.
Background
- C0 controls are bytes 0x00..0x1F (ESC, BEL, CR, ...), DEL is 0x7F. C1 controls are the code points U+0080..U+009F, encoded in UTF-8 as
C2 80..C2 9F; terminals treat several of them as one-byte equivalents of ESC sequences (U+009B = CSI, U+009D = OSC), so a byte-levelis_ascii_controlfilter does not cover them. - The version column is the lockfile
Resolutionformatted as text: the version number for registry packages, otherwise the tarball path or URL, the git URL plus commit, or the folder path. The name column is the lockfile package name, which for tarball, folder and git packages is copied from the package's own package.json (#38615 and #38633 are about rejecting such names at install time; this PR escapes whatever reaches the printer, and if they land the new test's package name moves to its tarball file name only). escape_control_charsfinds candidates withstrings::index_of_needs_escape_for_java_script_string, a SIMD scan whose stop set is everything outside 0x20..0x7E plus the quote character (\is passed as the quote so nothing extra is added), and only inspects those positions, stepping over ordinary multi-byte characters withwtf8_byte_sequence_length.
Notes
Rebased onto main after #38952 (banner and summary line for bun pm licenses) and #40074 landed. One conflict, in the test "control characters from package.json are escaped in text output but preserved in --json": #40074 runs it with --no-summary so the banner cannot match the group-header filter, and this PR changes the expected spelling from dropped to escaped. The resolution keeps both (--no-summary plus the escaped assertions). The five print sites in print_text merged without conflict.
[review] gate passed · iteration 2 · 3 files touched
fails on main (without fix)
ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-pm-licenses.test.ts
bun test v1.4.1 (4448a2e21)
test/cli/install/bun-pm-licenses.test.ts:
(pass) bun pm licenses > text output groups packages by license, Unknown last, dev-only packages marked [155.64ms]
(pass) bun pm licenses > --json shape [178.14ms]
(pass) bun pm licenses > legacy license shapes [388.27ms]
(pass) bun pm licenses > `license` array and legacy `name` key [394.00ms]
(pass) bun pm licenses > empty `license` falls through to `licenses`; `license` wins when both are present [329.96ms]
(pass) bun pm licenses > non-string license shapes are Unknown; entries with non-string type are skipped [349.89ms]
(pass) bun pm licenses > groups are ordered case-insensitively, ignoring a leading parenthesis, Unknown last [520.69ms]
(pass) bun pm licenses > repeated legacy entries are not deduplicated [366.27ms]
(pass) bun pm licenses > --json `license` follows each version's group; an empty description is omitted [374.08ms]
(pass) bun pm licenses > one package with two versions: per-license grouping, metadata from t
... (truncated)
release without fix: 26 FAILED
bun test v1.4.0-canary.1 (4448a2e21)
test/cli/install/bun-pm-licenses.test.ts:
271 | [hoistedDir, monoDir] = await Promise.all([setup(), setup("hoisted", monorepoFiles)]);
272 | });
273 |
274 | test.concurrent("text output groups packages by license, Unknown last, dev-only packages marked", async () => {
275 | const [stdout, stderr, exitCode] = await licenses(hoistedDir);
276 | expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`
^
error: expect(received).toMatchInlineSnapshot(expected)
- "bun pm licenses <version> (<revision>)
-
- MIT (2)
+ "MIT (2)
├── path-parse@1.0.6
└── resolve@1.9.0
Unknown (4)
├── a-dep@1.0.1 (dev)
├── no-deps@1.0.0
├── no-deps@1.0.1
- └── one-dep@1.0.0
-
- 6 packages across 2 licenses (checked 6 packages in bun.lock)"
-
+ └── one-dep@1.0.0"
+
- Expected - 7
+ Received + 3
at <anonymous> (/workspace/bun/test/cli/install/bun-pm-licenses.test.ts:276:42)
(fail) bun pm licenses > text output groups packages by license, Unknown last, dev-only packages marked [14.33ms]
(pass) bun pm licenses > --json sh
... (truncated)
passes on PR (with fix)
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/cli/install/bun-pm-licenses.test.ts
bun test v1.4.1 (4448a2e21)
test/cli/install/bun-pm-licenses.test.ts:
(pass) bun pm licenses > text output groups packages by license, Unknown last, dev-only packages marked [189.34ms]
(pass) bun pm licenses > --json shape [192.76ms]
(pass) bun pm licenses > legacy license shapes [428.93ms]
(pass) bun pm licenses > `license` array and legacy `name` key [419.79ms]
(pass) bun pm licenses > empty `license` falls through to `licenses`; `license` wins when both are present [369.27ms]
(pass) bun pm licenses > non-string license shapes are Unknown; entries with non-string type are skipped [345.02ms]
(pass) bun pm licenses > groups are ordered case-insensitively, ignoring a leading parenthesis, Unknown last [566.77ms]
(pass) bun pm licenses > repeated legacy entries are not deduplicated [370.18ms]
(pass) bun pm licenses > --json `license` follows each version's group; an empty description is omitted [381.22ms]
(pass) bun pm licenses > one package with two versions: per-license grouping, metadata from t
... (truncated)
release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
target linux-x64-gnu
build type Release
build dir ./build/release
revision 496a637b60
features baseline
23 deps, 129 codegen, 1172 objects in 716ms
ninja: Entering directory `/workspace/bun/build/release'
[1/1244] install /workspace/bun
bun install v1.4.0-canary.1 (4448a2e21)
Checked 26 installs across 63 packages (no changes) [7.00ms]
[2/1244] gen bindgenv2
[3/1244] fetch zlib
[zlib] up to date
[4/1244] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (4448a2e21)
Checked 1 install across 2 packages (no changes) [1.00ms]
[5/1244] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cpp
[6/1244] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[7/1217] fetch tinycc
[tinycc] up to date
[8/1216] gen .bind.ts → GeneratedBindings.cpp
[9/1216] gen JSBuffer.lut.h
Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp
[10/1216] install /workspace/bun/src/node-fal
... (truncated)
diff hotspot
src/bun_core/fmt.rs | 57 ++++++++++++++++++++++++
src/runtime/cli/pm_licenses_command.rs | 26 +++--------
test/cli/install/bun-pm-licenses.test.ts | 76 +++++++++++++++++++++++++-------
3 files changed, 123 insertions(+), 36 deletions(-)
gate history · 2 passed · 1 rejected · iteration 2
evidence per changed file
file reads edits tests
src/bun_core/fmt.rs 0 0 0
src/runtime/cli/pm_licenses_command.rs 4 8 0
test/cli/install/bun-pm-licenses.test.ts 9 9 0