Conversation
`bun info pkg@<range>` / `bun pm view pkg@<range>` parse the range out of the command line argument, so prerelease tags longer than the 8 byte inline limit are stored as offsets into that argument. find_best_version was handed the manifest's string buffer for the range, so those tags were read out of the wrong buffer and ranges like `>=1.0.0-beta.20240101` matched the wrong versions or nothing at all. Exact versions were not affected because they are looked up by tag hash.
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: fix is up for review.
|
Problem
bun info pkg@<range>andbun pm view pkg@<range>resolve the wrong version, or none, when a comparator in the range carries a prerelease tag longer than 8 bytes. Against a registry whose only versions are1.0.0-beta.20240101and1.0.0-beta.20240301(latest=.20240301), bun 1.4.0 gives:pkg@'>=1.0.0-beta.20240101':error: No version of "pkg" satisfying ">=1.0.0-beta.20240101" found(should resolve to.20240301)pkg@'~1.0.0-beta.20240301': same error (should resolve to.20240301)pkg@'<1.0.0-beta.20240201': resolves to.20240301(should be.20240101)pkg@'<1.0.0-beta.20240101': resolves to.20240301(should match nothing)src/runtime/cli/pm_view_command.rs:213parses the range out of the command line argument (SlicedString::init(version, version)) but then callsfind_best_version(&query, &parsed_manifest.string_buf), so the range's prerelease tags are read out of the manifest's string buffer. The comment on that line said it mirrorsoutdated_command.rs, but there the range comes from the lockfile and the lockfile buffer is passed, which is the matching buffer for that caller.pkg@1.0.0-beta.20240101) is unaffected becausefind_best_versiontakes the exact-version fast path, which looks the version up by tag hash. Only ranges hit the bad path.Fix
version(the argument slice the query was parsed from) as the group buffer, the same contract every otherfind_best_version/find_best_version_with_filtercaller follows (outdated_command.rs,update_interactive_command.rs,PackageManagerEnqueue.rsall pass the lockfile buffer their range was parsed from).find_best_version(group, group_buf)slices the group's tags out ofgroup_buf(Group::satisfies(v, group_buf, &self.string_buf)andleft.version.order(latest, group_buf, &self.string_buf)insrc/install/npm.rs), andquery::parse(version, SlicedString::init(version, version))stores those tags as offsets intoversion.test/cli/install/bun-info.test.ts,describe("version ranges with prerelease tags longer than 8 bytes"). ABun.servemock registry serves the two-version packument above; for bothbun infoandbun pm viewit checks the two cases resolved through thelatestdist-tag, the case that walks the prerelease list, and the case that must match nothing. All 8 fail on the released binary with the outputs listed under Problem and pass with this change; the whole file (28 tests) passes with the debug build.Background
Version, its prerelease/buildTag, rangeGroups) do not own their text. A string of up to 8 bytes is stored inline in the 8-bytesemver::String; anything longer is stored as an offset and length into whichever buffer it was parsed from, and everyslice/order/fmton it has to be handed that same buffer. Passing a different buffer is bounds-checked, so it yields unrelated bytes (or an empty string) rather than a crash, which is why this shows up as a wrong answer instead of a failure.bun pm viewhas two such buffers in play: the command line argument the range was parsed from, and the manifest'sstring_buf, which holds everything parsed out of the registry response (it starts with the package name, followed by the version strings that have tags).find_best_versiontakes the range's buffer as a parameter and uses its ownstring_buffor the candidate versions.Tag::order_pre) compares the two tags dot-segment by dot-segment; the major/minor/patch and "has a prerelease at all" comparisons happen before that and do not touch the buffers. So the wrong buffer only matters when both the comparator and the candidate have a prerelease tag at the same major.minor.patch, which is exactly the case the test exercises.