fix: drop polonius-the-crab, inline unsafe reborrow (RUSTSEC-2024-0436) - #422
Open
catinspace-au wants to merge 1 commit into
Open
fix: drop polonius-the-crab, inline unsafe reborrow (RUSTSEC-2024-0436)#422catinspace-au wants to merge 1 commit into
catinspace-au wants to merge 1 commit into
Conversation
catinspace-au
requested review from
abonander,
joe-clickhouse,
kavirajk and
peter-leonov-ch
as code owners
May 7, 2026 01:10
This was referenced May 7, 2026
…436) Drop polonius-the-crab and the chain of unmaintained crates underneath it. The borrow-check trick polonius wrapped behind a macro is replaced with a documented inline raw-pointer reborrow in `RowCursor::poll_next` and `Next::poll` (the latter already used the same pattern upstream post-PR-ClickHouse#397, so this PR completes the migration). Why now `paste`, the transitive dependency that polonius-the-crab uses, was flagged as unmaintained by RUSTSEC-2024-0436. polonius-the-crab itself has had no meaningful commits in 12+ months. The advisory + the four- stagnant-crate dependency stack is a poor trade for a macro that expands to one line of `unsafe { &mut *(bytes as *mut BytesExt) }`. What changes - `Cargo.toml`: drop polonius-the-crab and the three transitive deps (paste, higher-kinded-types, macro_rules_attribute). - `src/cursors/row.rs`: replace the `polonius!()` macro in `RowCursor::poll_next` with the same raw-pointer reborrow already in use in `Next::poll`. Existing upstream features (tracing span enter, returned_rows counter, debug log on deserialize error) are preserved. - `src/cursors/row.rs`: extensive SAFETY comment documenting why the reborrow is sound (Ok / NotEnoughData / Err arms) and why each safer alternative (TryRow enum, async-only API, UnsafeCell, double-deserialise) was rejected. Once Polonius lands in stable rustc, the reborrow can be removed. - `rustfmt.toml`: bump `edition` 2021 -> 2024 to match `Cargo.toml`. - `tests/it/cursor_reborrow.rs` (new): 4 mock-based tests covering single row, multi-row, empty result, fetch_all/fetch_one; plus 3 integration tests covering large results spanning chunks, borrowed rows, and small block size. Verification - `cargo build --no-default-features` clean. - `cargo test --features test-util --test it cursor_reborrow` runs 7 tests; the 4 mock-based pass without network. The 3 integration tests need a live ClickHouse to connect to. - The unsafe reborrow pattern in the new `poll_next` is the same one already in use in `Next::poll` upstream, so reviewers comparing the two will see identical structure.
catinspace-au
force-pushed
the
hyperi-port/01-remediation
branch
from
May 25, 2026 04:12
bde3dcb to
53e362f
Compare
abonander
approved these changes
Jun 1, 2026
abonander
left a comment
Contributor
There was a problem hiding this comment.
@catinspace-au this looks fine now but can you run cargo fmt please?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #404.
Drop polonius-the-crab and the chain of unmaintained crates underneath
it. The borrow-check trick polonius wrapped behind a macro is replaced
with a documented inline raw-pointer reborrow in
RowCursor::poll_nextand
Next::poll(the latter already used the same pattern upstreampost-PR-#397, so this PR completes the migration).
Why now
paste, the transitive dependency that polonius-the-crab uses, wasflagged as unmaintained by RUSTSEC-2024-0436. polonius-the-crab itself
has had no meaningful commits in 12+ months. The advisory + the four-
stagnant-crate dependency stack is a poor trade for a macro that
expands to one line of
unsafe { &mut *(bytes as *mut BytesExt) }.What changes
Cargo.toml: drop polonius-the-crab and the three transitive deps(paste, higher-kinded-types, macro_rules_attribute).
src/cursors/row.rs: replace thepolonius!()macro inRowCursor::poll_nextwith the same raw-pointer reborrow alreadyin use in
Next::poll. Existing upstream features (tracing spanenter, returned_rows counter, debug log on deserialize error) are
preserved.
src/cursors/row.rs: extensive SAFETY comment documenting why thereborrow is sound (Ok / NotEnoughData / Err arms) and why each
safer alternative (TryRow enum, async-only API, UnsafeCell,
double-deserialise) was rejected. Once Polonius lands in stable
rustc, the reborrow can be removed.
rustfmt.toml: bumpedition2021 -> 2024 to matchCargo.toml.tests/it/cursor_reborrow.rs(new): 4 mock-based tests coveringsingle row, multi-row, empty result, fetch_all/fetch_one; plus 3
integration tests covering large results spanning chunks, borrowed
rows, and small block size.
Verification
cargo build --no-default-featuresclean.cargo test --features test-util --test it cursor_reborrowruns7 tests; the 4 mock-based pass without network. The 3 integration
tests need a live ClickHouse to connect to.
poll_nextis the same onealready in use in
Next::pollupstream, so reviewers comparingthe two will see identical structure.