install: ignore leading whitespace in dist-tag, bare folder and local tarball versions

Closed
RO
1mo ago
+110-6

Problem

  • A package.json version with leading whitespace fails to install when it is a dist-tag, a bare folder path or a bare local tarball path, while the same value without the whitespace (and whitespace in front of ^1.0.0, npm:, file: values) installs fine:
    • "bar": " latest": error: Package "bar" with tag " latest" not found, but package exists / error: bar@ latest failed to resolve
    • "pkg": " ./pkg": error: Could not find package.json for "file: ./pkg" dependency "pkg" / error: pkg@ ./pkg failed to resolve
    • "baz": " ./baz-0.0.3.tgz": error: ENOENT extracting tarball from baz / error: baz@ ./baz-0.0.3.tgz failed to resolve
  • Cause, in parse_with_tag (src/install/dependency.rs): parse strips leading \t\n\r before Tag::infer classifies the literal and passes the stripped bytes on as dependency, but the Tag::DistTag arm, the bare-path fallback of Tag::Folder, the bare-path fallback of Tag::Tarball and the fallback of Tag::Symlink build their value from sliced.value(), the untrimmed literal. The other arms build it from dependency. So the value is classified as the dist-tag latest but looked up as " latest".
  • Second cause, same function: Version::to_version (every dependency loaded from a bun.lockb) and Dependency::clone_with_different_buffers (every lockfile save) call parse_with_tag directly with the stored literal, which is not trimmed. The arms slice prefixes off dependency (npm:, file:, git+, catalog:, ...), so a literal with leading whitespace takes the wrong branch on the re-parse. Visible with a binary lockfile: "baz": " file:./baz-0.0.3.tgz" installs, and the next bun install fails with error: ENOENT extracting tarball from baz because the re-parsed value is " file:./baz-0.0.3.tgz" and Version::eql no longer matches package.json.

Fix

  • The four arms above build their value from sliced.sub(dependency), like the remote-tarball and file: branches next to them already do. literal stays sliced.value(), so bun.lock and package.json keep the text as written (the new test checks bun.lock still says " latest") and existing lockfiles do not churn.
  • parse_with_tag trims dependency on entry (trim_literal, also used by parse and parse_with_optional_tag instead of their two inline copies), so the two stored-literal callers get the same value the first parse produced. The trim is the identity for every caller that already passed trimmed bytes; the only inputs it changes are stored literals, whose whitespace was already ignored when they were first parsed.
  • Why this is correct: the value must describe the same bytes Tag::infer classified, otherwise the tag and the value disagree. For dist-tags this is also what npm does (npm-package-arg trims registry specs: " latest" resolves the tag latest); for paths npm keeps the whitespace and fails, but bun already accepts " file:./x" and " ./x" is classified as a folder, so the value has to follow the classification.
  • Tag::Symlink can only be inferred from a link: literal, so its fallback is unreachable with leading whitespace; it is changed for consistency only and has no test.
  • Relation to #38309 (open): that PR trims at the sites that classify a literal on their own (package.json editor, string buffer pre-count, workspace: check, CLI rewrites) and, in its current form, at the two stored-literal callers (clone_with_different_buffers, to_version); its description leaves these arms to a separate fix, which is this PR. The arm changes are needed regardless of it: " latest", " ./pkg" and " ./x.tgz" fail with that branch too. The overlap is the entry trim here vs. its two caller-site trims, which cover the same two paths (both are needed here so that this PR stands on its own; either becomes redundant once the other lands and can be dropped by whichever rebases second). The trim_literal helper and the parse / parse_with_optional_tag hunks are byte-identical in both PRs, and git merge-tree of this branch with its current head (a7d2c3803e) merges without conflicts, so the order they land in does not matter.
  • Tests: test/cli/install/bun-install.test.ts, leading whitespace in the version literal (6 tests). Each installs once from package.json and once more with --frozen-lockfile from the lockfile the first run wrote, and checks what landed in node_modules: " latest" and "\tlatest" dist-tags, " latest" with a text lockfile (also checks the literal in bun.lock), the folder " ./pkg", the tarballs " ./baz-0.0.3.tgz" and " file:./baz-0.0.3.tgz". All 6 fail on the released binary (the errors above; the file: tarball fails on the second install). With only the arm changes applied, the two tarball tests still fail on the second install, so they are what covers the entry trim.
  • Also run with the debug build: bun-install.test.ts (the 14 remaining failures are the bitbucket/gitlab/public-URL tests and fail the same way with the released binary here, no network), bun-lockb, bun-lock, bun-add, bun-update, overrides, catalogs, bun-workspaces, migration/* (complex-workspace needs network and fails identically with the released binary) and bun-link (its should link dependency without crashing compares exact stdout and trips over the install-failure stack trace debug builds print; the link: path itself is unchanged, link: literals always take the branch above the fallback). cargo fmt and cargo clippy -p bun_install are clean.

Background

  • Version literal: the string value of a dependency entry in package.json ("^1.0.0", "latest", "./pkg", "file:./x.tgz"). Tag::infer classifies it by its first byte and prefix; parse_with_tag turns it into a Dependency.version, which stores the text verbatim as literal (what bun.lock prints and what Version::eql compares for dist-tags and folders) plus a tag-specific value (the tag name, the folder path, the tarball path) that resolution uses.
  • SlicedString: a pair of (string buffer, slice into it). sliced.value() makes a String handle for the whole slice, sliced.sub(bytes).value() for a sub-range of it; both point into the same buffer, so taking a sub-range costs nothing and is how the other arms extract the part of the literal they need.
  • Stored-literal re-parse: the lockfile keeps only the tag and the literal for each dependency. Loading a bun.lockb (to_version) and saving any lockfile (clone_with_different_buffers, which copies each dependency into a fresh string buffer) rebuild the value by calling parse_with_tag again with the stored literal and tag; bun.lock (text) goes through parse instead, which is why the second symptom only shows up with a binary lockfile.