Skip to content

dotenv: stop byte-trimming 0xA0 from UTF-8 .env values - #34001

Merged
Jarred-Sumner merged 3 commits into
mainfrom
farm/c57c94be/dotenv-utf8-nbsp-trim
Jul 19, 2026
Merged

dotenv: stop byte-trimming 0xA0 from UTF-8 .env values#34001
Jarred-Sumner merged 3 commits into
mainfrom
farm/c57c94be/dotenv-utf8-nbsp-trim

Conversation

@robobun

@robobun robobun commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Reproduction

import { parseEnv } from "node:util";
parseEnv("A=xठ\n").A        // bun: "x�"   node: "xठ"
parseEnv("A=\u00A0x\u00A0\n").A  // bun: " x�"  node: " x "

Same corruption on auto-loaded .env files and --env-file.

Cause

WHITESPACE_CHARS in src/dotenv/env_loader.rs included raw 0xA0 and was applied byte-at-a-time by skip_whitespaces() and strings::trim() over UTF-8 input. In UTF-8, 0xA0 is a continuation byte: NBSP is C2 A0, U+0920 is E0 A4 A0, etc. Trimming it as a single byte severs multi-byte sequences at value boundaries, leaving invalid UTF-8 that surfaces as U+FFFD.

Fix

Drop 0xA0 from the whitespace set. Node.js does not trim NBSP from unquoted .env values (verified against v26.3.0), so this also brings util.parseEnv and .env loading in line with Node.

Verification

New test in test/cli/run/env.test.ts covers util.parseEnv and .env file loading with a 3-byte sequence ending in 0xA0, NBSP-wrapped values, a bare NBSP value, and ASCII-space trimming (unchanged). Fails on main, passes with this change; full env.test.ts suite and test-util-parse-env.js still pass.


no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/run/env.test.ts

The parser's WHITESPACE_CHARS set included raw 0xA0 and was applied
byte-at-a-time to UTF-8 input. 0xA0 is a continuation byte in UTF-8
(NBSP is C2 A0), so trimming it severed multi-byte sequences at value
boundaries: A=xठ (U+0920 = E0 A4 A0) lost its final byte and became
invalid UTF-8, and NBSP-padded values were left with a bare C2 lead
byte. Node.js does not trim NBSP from unquoted values, so drop 0xA0
from the set entirely.
@coderabbitai

coderabbitai Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c577764e-fa4d-4d83-9fd8-6e6ebe3f80c8

📥 Commits

Reviewing files that changed from the base of the PR and between 8624c2b and a631cb7.

📒 Files selected for processing (2)
  • src/dotenv/env_loader.rs
  • test/cli/run/env.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 12:13 AM PT - Jul 12th, 2026

@robobun, your commit a631cb7 has 1 failures in Build #72067 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34001

That installs a local version of the PR into your bun-34001 executable, so you can run:

bun-34001 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — one-byte constant fix with a clear root cause and Node-compat justification.

What was reviewed:

  • WHITESPACE_CHARS uses in skip_whitespaces, parse_key (: branch), and parse_value's strings::trim — removing 0xA0 only affects trailing/leading trim of unquoted values; leading-NBSP-before-key and :-whitespace paths already stopped on 0xC2 so no regression there.
  • Test covers both util.parseEnv and .env file loading, includes a 3-byte sequence ending in 0xA0 (U+0920), NBSP-wrapped/bare values, and a guard that ASCII-space trimming still works (D= x x).
  • No CODEOWNERS entry for src/dotenv/.
Extended reasoning...

Overview

Removes the raw byte 0xA0 from the WHITESPACE_CHARS constant in src/dotenv/env_loader.rs, adds a two-line comment explaining why the set must be ASCII-only, and adds a regression test in test/cli/run/env.test.ts. The constant is applied byte-at-a-time by skip_whitespaces() and strings::trim() over UTF-8 input; 0xA0 is a UTF-8 continuation byte (NBSP = C2 A0, U+0920 = E0 A4 A0), so trimming it as a standalone byte severed multi-byte sequences at value boundaries and produced U+FFFD.

Security risks

None. This narrows what gets stripped from .env values — previously-corrupted bytes are now preserved verbatim. No new parsing, no allocation changes, no untrusted-length arithmetic.

Level of scrutiny

Low. The functional change is a single byte removed from a 7-byte constant. I traced all three consumers of WHITESPACE_CHARS in the parser: skip_whitespaces() (leading whitespace before keys/values/quotes), the b':' key-separator branch, and the unquoted-value strings::trim. In the first two, a leading NBSP already stopped scanning at 0xC2 (not in the set), so behavior is unchanged there. The only observable effect is that unquoted values with UTF-8 sequences ending in 0xA0 at either boundary are no longer byte-truncated — which is the bug being fixed and matches Node.js's util.parseEnv.

Other factors

The test is well-constructed: it exercises both the in-process parseEnv path and the subprocess .env-file path, asserts exact values (not lengths), and includes D= x "x" to guard that ASCII-space trimming is unchanged. It sits alongside the existing .env space edgecase test in the correct file. The PR description states the full env.test.ts suite and Node's test-util-parse-env.js still pass. No prior human reviews or outstanding comments on the PR.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — removing raw 0xA0 from a byte-wise whitespace set is the correct fix for UTF-8 corruption, and it matches Node's parseEnv behavior.

What was reviewed:

  • Confirmed WHITESPACE_CHARS is applied byte-at-a-time in skip_whitespaces / parse_key / strings::trim, so 0xA0 (a UTF-8 continuation byte) was severing multi-byte sequences at value boundaries.
  • Test covers a 3-byte sequence ending in 0xA0, leading/trailing/bare NBSP, and confirms ASCII-space trimming is unchanged (D= x x).
  • No other 0xA0 byte-trimming sites in src/dotenv/; no CODEOWNERS entry for this path.
Extended reasoning...

Overview

One-byte deletion from the WHITESPACE_CHARS constant in src/dotenv/env_loader.rs (dropping \\xA0), plus a two-line comment and a new test in test/cli/run/env.test.ts. The constant is used by skip_whitespaces(), the : separator check in parse_key(), and strings::trim() in parse_value() — all byte-at-a-time over UTF-8 input. Since 0xA0 is a UTF-8 continuation byte (NBSP = C2 A0, U+0920 = E0 A4 A0), trimming it as a lone byte splits multi-byte code points and yields invalid UTF-8 that surfaces as U+FFFD.

Security risks

None. This narrows the whitespace set (fewer bytes trimmed), so it cannot introduce new parse ambiguities; it only stops corrupting valid UTF-8. The constant is local to the dotenv parser and does not touch auth, crypto, or network paths.

Level of scrutiny

Low. The mechanism is unambiguous (byte-wise trim of a continuation byte over UTF-8), the fix is a single-byte constant edit, and Node.js compatibility was verified — Node does not trim NBSP from unquoted .env values, so preserving NBSP verbatim is the correct behavior for both correctness and compat. The added comment documents the ASCII-only invariant to prevent regression.

Other factors

The new test exercises both entry points (util.parseEnv and auto-loaded .env), asserts the positive case (ASCII spaces still trimmed) alongside the fix, and is placed in the existing env.test.ts next to related edge-case tests. Grep confirms no other 0xA0 byte-set usage in src/dotenv/. No CODEOWNERS covers this path and there are no outstanding reviewer comments.

@robobun

robobun commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator Author

This diff is ready. The new test and the full env.test.ts suite pass on all lanes.

CI is red on tests unrelated to this change (both already being handled separately):

Neither touches the .env parser. Needs a maintainer to merge past the unrelated flake.

@Jarred-Sumner
Jarred-Sumner merged commit c070df1 into main Jul 19, 2026
77 of 78 checks passed
@Jarred-Sumner
Jarred-Sumner deleted the farm/c57c94be/dotenv-utf8-nbsp-trim branch July 19, 2026 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants