fix: widen the order number and stop a cart quantity wrapping negative - #87
Merged
Conversation
Two defects found reading the checkout path after the hardening pass.
Order numbers collided at commerce scale. The number is WW-{date}-{suffix} and
the suffix was six hex characters of the order's Guid - 24 bits, scoped to a
single day. order_number carries a unique index, so a collision was never a
data leak, but it was an INSERT that violated the constraint and rolled the
whole placement back: a customer meeting a hard failure at checkout.
Collisions arrive by the birthday bound, not when the space runs out, so this
bites far earlier than 16.7 million. At a thousand orders in a day the chance of
at least one collision is around three per cent; at five thousand it is a coin
flip; at ten thousand it is near certain. The suffix is now ten characters - 40
bits - which stays under a rounding error past a million orders a day. The cost
is four characters on a number people read out.
The failure was at least in the safe order: TryPlaceAsync runs before the
payment is charged, so a collision cost the customer an error rather than money.
A test pins the width so it cannot be shortened again for tidiness.
Cart quantity wrapped instead of clamping. AddCartItemHandler summed the
existing line and the requested amount in int arithmetic before clamping to
available stock. A quantity near int.MaxValue wrapped negative, Math.Min then
chose the negative, and the shopper was told the item was out of stock - a
misleading answer rather than a dangerous one, since nothing negative reached
the cart. The sum is now widened to long before the clamp, so the clamp does
the clamping and five available means five in the cart.
482 backend tests pass locally against PostgreSQL 16; dotnet format clean.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EA4mmpcb1rcvNntHR1iG6j
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.
Two defects found reading the checkout path after the hardening pass.
Order numbers collided at commerce scale
The number is
WW-{date}-{suffix}, and the suffix was six hex characters of the order's Guid — 24 bits, scoped to a single day.order_numbercarries a unique index, so a collision was never a data leak. But it was an INSERT that violated the constraint and rolled the whole placement back: a customer meeting a hard failure at checkout.Collisions arrive by the birthday bound, not when the space runs out, so this bites far earlier than 16.7 million:
The suffix is now ten characters — 40 bits, which stays under a rounding error past a million orders a day. The cost is four characters on a number people read aloud.
Worth noting the failure was at least in the safe order:
TryPlaceAsyncruns before the payment is charged, so a collision cost the customer an error rather than money.A test pins the width so it cannot be shortened again for tidiness.
Cart quantity wrapped instead of clamping
AddCartItemHandlersummed the existing line and the requested amount inintarithmetic before clamping to available stock:A quantity near
int.MaxValuewraps negative,Math.Minthen picks the negative, and the shopper is told the item is out of stock — misleading rather than dangerous, since nothing negative reached the cart.The sum is widened to
longbefore the clamp, so the clamp does the clamping: five available means five in the cart. No new policy, no new limit — just arithmetic that can't wrap.Verification
dotnet format --verify-no-changesclean, build clean under-warnaserrorGenerated by Claude Code