http: escape control characters in the verbose request/response trace

RO
3mo ago
+186-19

Problem

  • The HTTP client's verbose trace (fetch(url, { verbose: true | "curl" }), BUN_CONFIG_VERBOSE_FETCH, bun install --verbose) writes the request URL, every request and response header name and value, and the response status text to stderr byte for byte: print_request in src/http/lib.rs:1439-1459 and the Display impls for Header, HeaderCurlFormatter, RequestCurlFormatter and Response in src/picohttp/lib.rs.
  • Those bytes are chosen by the peer or by a dependency:
    • HTTP/1.1 header values and the reason phrase may contain HT and any byte >= 0x80 (picohttpparser only rejects the other C0 bytes and DEL), which includes the UTF-8 encoding of the C1 controls; HPACK/QPACK values on h2/h3 may contain anything except NUL, CR and LF (is_malformed_response_value), so ESC gets through there too.
    • bun install passes the tarball URL from a dependency's package.json / registry manifest to the trace unmodified.
  • Example (released bun, bun install --verbose with "dep": "http://127.0.0.1:PORT/dep-\u009b31m.tgz"): stderr receives HTTP/1.1 GET http://127.0.0.1:PORT/dep-<U+009B>31m.tgz; a 404 response with x-evil: a<C2 9B>31mb prints < x-evil: a<U+009B>31mb. U+009B is the one-byte CSI, so <U+009B>31m is a colour change when a terminal reads it, and CSI/OSC sequences in general can erase or repaint the lines around them.
  • The install commands' own output for the same strings is being escaped in #38631 (and #38525, #38536, #38557, #38615 for other commands); this PR covers the HTTP trace those strings also flow through.

Fix

  • Adds bun_core::fmt::escape_control_chars / EscapeControlChars (the same hunk as #38631, #38525 and #38615, so whichever lands first the others merge cleanly) and prints every peer- or dependency-supplied field of the trace through it: the request URL, header names and values in both the >/< lines and the curl line, the redacted Authorization scheme, and the response status text. Headers (used to build the WebSocket upgrade request) and the method (a Method enum name) are untouched.
  • Escaping happens inside the Display impls rather than around them, because those impls also emit bun's own ANSI colour codes when stderr is a TTY; only the data is escaped (checked by hand with FORCE_COLOR=1).
  • The request line keeps its secret redaction and wraps it: EscapeControlChars(redacted_npm_url(url)), so redaction runs on the real bytes and escaping on the result. For that to be sound RedactedNpmUrlFormatter now writes its runs through BStr instead of write_bytes (from_utf8_unchecked): bun install hands the trace whatever bytes the package.json contained, and a Latin-1 file gets bytes that are not UTF-8 (verified: a dependency URL containing 82 41 9B 41 reaches print_request as-is). Runs only ever split at ASCII bytes, so valid UTF-8 renders exactly as before; invalid sequences now render as U+FFFD, which is how the package manager's own error lines already print them.
  • Why this is the right output: the trace exists to show what went over the wire, so the bytes are spelled out (\u009b, \x1b, \r) rather than dropped, matching the convention the sibling PRs establish for the rest of the install output. The request body in the curl line is already a JSON string literal (C0 controls escaped) and is the caller's own data, so it is left alone.
  • Verified with:
    • test/js/web/fetch/fetch.test.ts "verbose fetch logging escapes control characters coming from the peer": raw TCP server sends a reason phrase and a header value containing U+009B, the client sends x-req and Authorization values containing it, verbose: "curl". Covers the > header lines, the redacted Authorization branch, the curl line and the < status and header lines. Fails on the released binary (the raw U+009B is printed in all five places), passes with this branch.
    • test/cli/install/bun-install.test.ts "--verbose escapes control characters in the tarball URL and response headers it traces": package.json with one tarball URL containing U+009B and one containing raw non-UTF-8 bytes, against a local 404 server. Covers the request-line URL path (which fetch() cannot reach, since it percent-encodes the path) and the non-UTF-8 rendering. Fails on the released binary, passes with this branch.
    • bun bd test test/regression/issue/12042.test.ts (curl trace body) and test/cli/install/redacted-config-logs.test.ts (URL redaction) still pass; the remaining failures when running the two edited files in full here are localhost/internet-dependent tests that fail identically with the released binary.

Background

  • The verbose trace is printed from the HTTP client thread by print_request (HTTP/1.1, h2 and h3 all call it) and print_response in src/http/lib.rs; the line formatting for headers and the response lives in the Display impls of the bun_picohttp crate. HTTPVerboseLevel::Headers prints the >/< lines, HTTPVerboseLevel::Curl additionally prints a copy-pasteable curl command first; bun install --verbose uses Headers.
  • C1 controls are U+0080..U+009F. Terminals treat several of them as one-byte forms of two-byte ESC sequences (U+009B = ESC [, CSI; U+009D = ESC ], OSC), and in UTF-8 mode xterm and others recognise them when they arrive UTF-8 encoded (C2 9B), which is the form that passes through an HTTP/1.1 parser.
  • bstr::BStr's Display writes bytes as UTF-8 and substitutes U+FFFD for invalid sequences, which is what the rest of the trace already used; escape_control_chars(bytes) builds on it, and EscapeControlChars<T: Display> escapes whatever the inner Display writes, so it must only ever be fed valid &str chunks (the reason for the RedactedNpmUrlFormatter change).
  • redacted_npm_url replaces URL passwords, UUIDs and npm_ tokens with ***; it is the existing formatter for printing registry/tarball URLs.