install: use uppercase %2F for scoped package manifest URLs - #33774
install: use uppercase %2F for scoped package manifest URLs#33774erik-balfe wants to merge 1 commit into
Conversation
Match the npm CLI and RFC 3986 §2.1 canonical form. GitLab's npm registry returns 404 for lowercase %2f but accepts %2F. AWS CodeArtifact and Azure Artifacts accept both. Port of the fix from oven-sh#30312 to the Rust codebase: - src/install/NetworkTask.rs (bun install manifest fetch) - src/bun_core/fmt.rs (DependencyUrlFormatter for bun pm view/publish) Adds regression tests and updates install test assertions. Fixes oven-sh#30311 Fixes oven-sh#26241
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughThis PR changes scoped package name percent-encoding of "/" from lowercase %2f to uppercase %2F in ChangesUppercase %2F Encoding Fix
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for this. I have pulled these commits onto a repo branch so CI can run, with some test cleanups layered on top: #33784. Authorship is preserved on the first commit. |
|
Thanks for the PR, and sorry for the runaround here — closing this along with #33784, which carried the same change. Short version: the premise doesn't hold. The npm CLI sends lowercase And GitLab decodes both casings identically. Against live Full evidence in #33784 (comment). The underlying problem in #30311 looks like auth, not encoding — the latest report there is a |
Fixes #30311
Fixes #26241
What does this PR do?
Re-applies the fix from #30312 for the Rust codebase (the prior PR targeted the Zig sources and was closed when Bun migrated to Rust).
Since bun 1.3.0 (from #5716), bun percent-encodes the
/between scope and name for private-registry manifest requests as lowercase%2f:GitLab's npm registry returns 404 for this URL. It does not normalize
%2fto/and does not treat%2fand%2Fas equivalent. bun 1.2.23 sent the unencoded slash, which GitLab accepted.Production changes (two characters):
src/install/NetworkTask.rs— scoped manifest URL construction forbun install(%2f→%2F)src/bun_core/fmt.rs—DependencyUrlFormatterused bybun pm view/bun publish(%2f→%2F)This matches the npm CLI reference implementation and RFC 3986 §2.1. AWS CodeArtifact, Azure Artifacts, npmjs.org, and GitLab all accept
%2F; no known registry accepts%2fbut rejects%2F.Also updates install test assertions and mock-registry URL normalization helpers to expect/accept uppercase
%2F, and adds two regression tests.How did you verify your code works?
Added regression tests in
test/cli/install/bun-install-registry.test.ts:scoped package manifest url uses uppercase %2F— spins up aBun.servemock GitLab-style registry that 404s any URL containing lowercase%2fand serves a manifest only for uppercase%2F. Asserts bun sends%2Fon the wire andbun installcompletes.bun pm view uses uppercase %2F for scoped names— same mock for theDependencyUrlFormattercode path viabun pm view.Gate check (per CLAUDE.md): with the unfixed build, the new test should fail because bun sends lowercase
%2f; withbun bd test test/cli/install/bun-install-registry.test.ts -t "scoped package manifest url uses uppercase", it should pass.Note: Full
bun bdbuild was not run in this environment (sparse checkout / missing LLVM toolchain). CI on BuildKite is the authoritative verification.Supersedes the closed Zig PR #30312 for the current Rust tree.