pm licenses: escape control characters in text output (adds bun_core::fmt::escape_control_chars)

Closed
RO
1mo ago
+123-36

Problem

  • bun pm licenses (new in #38333, not in any release yet) prints text it did not write: the license, author, description and homepage fields of every installed package.json, and each package's name and resolution.
  • Its sanitizer, printable() in src/runtime/cli/pm_licenses_command.rs, only dropped bytes matching u8::is_ascii_control. C1 controls (U+0080..U+009F; U+009B is the one-character form of ESC [, and xterm-style terminals execute it) went through raw in the license header and in the --long lines.
  • The name@version line was not passed through printable() at all. The version column is the resolution rendered to text (resolution.fmt(...) at the top of exec): 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 named dep-<U+009B>.tgz whose manifest is named dep-<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>EVIL was printed as MIT[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>: a Display adapter 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, --json is the machine-readable form.
  • pm licenses prints the license header, the name, the version and the --long fields 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 --json emits. --json is 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_indices body 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 over Display that becomes EscapeControlChars(redacted(..)).
  • The escape spelling is pinned by test/cli/install/bun-pm-licenses.test.ts rather than by a Rust unit test: cargo test -p bun_core does not link (the crate's SIMD entry points such as highway_memmem live 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 --json round trip. Against a build of main these four tests fail (raw U+009B in the output, MIT[31mEVIL instead of MIT\x1b[31m\nEVIL); with the fix all 81 tests in the file pass.
  • test/internal/source-lints passes; cargo clippy -p bun_core is 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-level is_ascii_control filter does not cover them.
  • The version column is the lockfile Resolution formatted 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_chars finds candidates with strings::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 with wtf8_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