[JamBalaya56562]

Corrects one third of my own #11552. The reasoning I gave for the HTTP-side change there was wrong, and the comment stating it is now in main.

The claim, and why it's false

#11552 converted fetch_checksum_from_file from get_text to get_bytes + file::decode_text, with this comment:

Read the bytes rather than get_text, which decodes with the response charset and silently replaces anything it cannot map — a UTF-16 checksum file would be recorded as mojibake instead of failing.

reqwest's text() (reqwest-0.13.4/src/async_impl/response.rs:163) delegates to text_with_charset("utf-8") when the charset feature is on — which mise enables in Cargo.toml. Its own docs say it "decodes the response body with BOM sniffing" and that "the BOM is stripped from the returned String". The body is encoding.decode(&full), which is encoding_rs-0.8.35/src/lib.rs:3009:

pub fn decode<'a>(&'static self, bytes: &'a [u8]) -> (Cow<'a, str>, &'static Encoding, bool) {
    let (encoding, without_bom) = match Encoding::for_bom(bytes) {
        Some((encoding, bom_length)) => (encoding, &bytes[bom_length..]),
        None => (self, bytes),
    };
    ...

A BOM overrides the declared encoding and is stripped. The HTTP path already handled UTF-16 correctly, so that conversion fixed nothing.

What it cost

get_text goes through TextRequest::send (http.rs:845), which detects a <!DOCTYPE html> body and re-sends over https when the URL was plain http. get_bytes → get_async → send_with_https_fallback has no such branch. So an http:// checksum URL answered with an HTML error page would decode cleanly and get parsed as a checksum, where it used to be retried over https.

Registry checksum URLs are https, so the exposure is small — but it was behaviour given up for no gain.

To be precise about what was not lost: retries were fine either way. send_with_https_fallback (http.rs:484) delegates to send_with_https_fallback_with_retries with Settings::get().http_retries(), the same default TextRequest uses. I checked before writing this.

What stays

The two file-reading sites from #11552 are correct and untouched: aqua.rs:1785 and :2515 go through std::fs::read_to_string, which really does reject non-UTF-8 — that is the exact frame in #5399's stack trace, and file::read_to_string_bom is the right fix for it.

read_to_string_bom now documents the asymmetry, so the HTTP path doesn't get "fixed" again:

/// Only reads *from disk* need this. Bodies fetched over HTTP already arrive decoded: reqwest's
/// `text()` goes through `text_with_charset` -> `encoding_rs::Encoding::decode`, which sniffs a BOM
/// and lets it override the declared charset. `std::fs::read_to_string` has no such step, and that
/// asymmetry is the only reason this function exists — reaching for it on an `HTTP.get_text` result
/// would be redundant.

The same finding also applies to static_helpers.rs, backend/http.rs, node.rs and pkgx.rs, which all read checksums via get_text/get_text_cached. I had listed those in #11552 as adjacent work left for later; they need nothing, and this PR is the record of why.

Verification

Read from reqwest's and encoding_rs's source, cited above — I have not exercised the HTTP path locally. No local cargo run; rustfmt --edition 2024 only. The src/file.rs decoder tests and #11552's aqua UTF-16 fixture test both cover file paths and are unaffected.

Summary by CodeRabbit

• Bug Fixes

  • Improved checksum retrieval from HTTP and HTTPS sources.
  • Added more reliable support for text encodings and byte-order marks, including UTF-16 checksum files.
  • Improved handling of checksum download responses.

• Documentation

  • Clarified that BOM-aware text reading applies to local files, while HTTP responses are decoded automatically.

[coderabbitai]

📝 Walkthrough

Walkthrough

The aqua backend now retrieves checksum files with HTTP.get_text. The change removes explicit byte decoding and separate decode-error handling. Documentation clarifies that read_to_string_bom is for disk reads, while HTTP text decoding handles BOMs. A UTF-16LE regression test covers the retrieval path.

Changes

Checksum text retrieval

Layer / File(s)	Summary
HTTP checksum decoding src/backend/aqua.rs, src/file.rs	fetch_checksum_from_file uses HTTP.get_text. The retrieval comment describes transport URL use. A UTF-16LE regression test verifies checksum parsing. The documentation distinguishes HTTP response decoding from disk reads.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• jdx/mise#11552: Both changes update aqua checksum-file decoding and BOM handling.

Suggested reviewers: jdx, risu729

Poem

A rabbit checks the checksum line,
HTTP text decodes fine.
UTF-16 now passes through,
Disk-read notes explain what to do.
Hop, the checksum matches too!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: restoring checksum manifest fetching with get_text.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/backend/aqua.rs (2)

2177-2180: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the stale byte-only wording.

The neighboring comment still says that only bytes are read. This path now reads decoded text with HTTP.get_text. Keep the transport-URL rationale, but remove the byte-specific statement.

Proposed comment update

-                // Only the bytes are read here (no filename derivation), so the transport
-                // URL is all that is needed.
+                // No filename derivation is needed here, so the transport URL is all that
+                // is needed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/aqua.rs` around lines 2177 - 2180, Update the comment above the
checksum download to remove the stale byte-only wording, accurately describe
that HTTP.get_text reads decoded text, and retain the rationale about transport
URLs and HTML-body HTTPS retries.

---

2177-2187: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy lift

Add regression coverage for the HTTP checksum path.

Test UTF-16 checksum responses with a BOM and a plain-HTTP response that returns HTML before the HTTPS retry returns checksum text. Assert the parsed algorithm:digest result. The PR verification notes state that no HTTP-path exercise was performed.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/aqua.rs` around lines 2177 - 2187, Add regression tests for the
HTTP checksum flow using the checksum-fetching function around HTTP.get_text:
cover a UTF-16 response with a BOM and a plain-HTTP response containing HTML
followed by a successful HTTPS checksum response, asserting both produce the
parsed algorithm:digest result.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/backend/aqua.rs`:
- Around line 2177-2180: Update the comment above the checksum download to
remove the stale byte-only wording, accurately describe that HTTP.get_text reads
decoded text, and retain the rationale about transport URLs and HTML-body HTTPS
retries.
- Around line 2177-2187: Add regression tests for the HTTP checksum flow using
the checksum-fetching function around HTTP.get_text: cover a UTF-16 response
with a BOM and a plain-HTTP response containing HTML followed by a successful
HTTPS checksum response, asserting both produce the parsed algorithm:digest
result.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 2ddb57a3-b048-4a20-b67f-520924b67a89

📥 Commits

Reviewing files that changed from the base of the PR and between 5d14988 and f5625f5.

📒 Files selected for processing (2)

• src/backend/aqua.rs
• src/file.rs

[greptile-apps]

Greptile Summary

Restores the BOM-aware text transport for Aqua checksum manifests.

• Replaces the byte-fetch and local decoder path with HTTP.get_text, preserving HTTP-to-HTTPS fallback for HTML responses.
• Adds an HTTP UTF-16LE checksum-manifest regression test.
• Documents why BOM-aware local-file decoding is unnecessary for bodies fetched through the text HTTP client.

Confidence Score: 5/5

The PR appears safe to merge with no actionable defects identified.

The restored text-fetch path retains BOM-aware decoding while recovering HTML detection and HTTP-to-HTTPS fallback, and the added regression test covers the intended UTF-16 response behavior.

Important Files Changed

Filename	Overview
src/backend/aqua.rs	Restores text-based checksum retrieval and verifies that BOM-prefixed UTF-16 HTTP responses produce the expected digest.
src/file.rs	Clarifies the distinction between BOM decoding for local files and decoding already performed by the HTTP text transport.

_{Reviews (1): Last reviewed commit: "fix(aqua): fetch the checksum manifest w..." | Re-trigger Greptile}