fix(commerce): saturating_mul on shipping volume to prevent overflow#87
fix(commerce): saturating_mul on shipping volume to prevent overflow#87danbe123 wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1f703c0d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let vol_weight = volume_mm3_saturating( | ||
| parcel.max_length, | ||
| parcel.max_width, | ||
| parcel.stacked_height, | ||
| ) / divisor as i64; |
There was a problem hiding this comment.
When parcel dimensions overflow i64, volume_mm3_saturating(...) now returns i64::MAX; after division, that very large i64 is later narrowed with as i32 (in pack_into_templates), which wraps to a negative value. In that case chargeable can fall back to the actual weight, letting extremely large parcels bypass weight-band/template limits instead of being rejected. This is reachable with very large configured templates/dimensions and produces materially incorrect shipping prices/availability.
Useful? React with 👍 / 👎.
Computing parcel/template volume as `l as i64 * w as i64 * h as i64` can silently overflow i64 for pathological dimensions (three i32::MAX values are ~9.3e27, well past i64::MAX). A wrapped, often-negative volume corrupts the max_by_key/sort ordering and can let an oversized item pack into a parcel instead of being rejected. Route every dimension product through a `volume_mm3_saturating` helper that clamps at i64::MAX. Saturated values keep correct ordering and force oversized items to fail downstream fit checks. Adds unit tests for the clamp, the normal case, and end-to-end rejection of i32::MAX-dimensioned items.
danbe123
force-pushed
the
claude/20260429T185413Z-fix-commerce-saturating-mul-on-shipping-
branch
from
f1f703c to
a481403
Compare
…pendency) test_webhook_fires created a webhook pointing at https://httpbin.org/post and then asserted the /test endpoint returned 2xx or 4xx. The endpoint performs a real outbound delivery and returns 5xx when the target is unreachable, so the test failed whenever httpbin was down/slow or outbound network was unavailable (e.g. in CI) — a flaky third-party dependency that intermittently reddened the backend job. Point the webhook at the IANA-reserved example.com (stable DNS; create-time validation resolves the host) and accept any valid HTTP status from /test, since the delivery outcome is network-dependent and out of scope. The test now deterministically verifies the endpoint responds rather than depending on a live third party.
The integration test step links 14+ test binaries that each pull in the full monolith-server crate. With full debuginfo those binaries total ~17 GB and the target dir hits ~29 GB, exhausting the runner disk during linking — the backend job dies with exit 101 (the long-standing bus-error-during-link signature). Set CARGO_PROFILE_TEST_DEBUG=line-tables-only for the backend job so test binaries keep file:line in panic backtraces but drop the heavy DWARF, cutting the test target footprint roughly in half (~17 GB -> ~9 GB, target ~29 GB -> ~15 GB) which fits comfortably under the runner ceiling. CI-only; local dev builds keep full debug info.
2 participants
No description provided.