Repository navigation
test: see that the calls of a race overlapped, on every run - #9
Merged
Merged
Conversation
…um order The ceiling was 10,000, a guard against scripted abuse carried over from the previous implementation. The owner set it to 500 (design §11, decision 9), which makes it a rule of the shop that a buyer will meet. So it is judged where the minimum order is, in quantityIssue: a line above it that is already in a cart cannot be ordered either, and says the figure. A quantity field carries it as max, on a product page and in the cart, from max_quantity in both views. The admin refuses a minimum order above it.
An inquiry is quoted by a number of the same shape as an order's: a prefix, the date and eight characters of Crockford's alphabet. The generator moves out of the order code so that both use it, with a test of the shape it makes and of how one is recognised under its own prefix.
Phase 1's last piece, which the owner decided is the shop plugin's to build (design §11, decision 10). A form on the cart page takes who to answer and a message, and posts to cart/inquiry. The route stores the cart as an inquiry with its lines as a snapshot, queues the job that owes its emails, and empties the cart, in one batch; it answers with a redirect to the cart page, which shows the inquiry's number to the browser whose cart it was. A part with no price can be asked about, so a product page now offers a form for a size without one. The write is conditional on the cart still having lines and on the visitor being under five inquiries an hour, both asked inside the statement, so two forms at once store one inquiry and a refused one leaves the cart alone. The seller is told by email, answerable to the buyer, and reads inquiries in a panel of the admin: to mark, export as CSV, or delete. The email confirming to the buyer is built and off by default, because the form has no challenge in front of it: Mallok has nowhere to draw one on a plugin's page.
The design gains the owner's decision and §18. The security notes say what personal data an inquiry holds, what stands in place of a challenge, and what that does not stop.
…ependent review found The review found the guards inside the inquiry's write sound, and these around them: - A cart that changed between being read and being written lost the change: a part added in between was in neither the inquiry nor the cart. The write is now conditional on the cart being exactly what is sent, and a cart that changed is answered with cart_changed and left alone. - An address could carry a mailto link's own syntax, which the admin links as stored. It is held to a list of what an address may be made of. - A browser let through what the server refused, and the form came back empty: a name of spaces, a tab in a name, a message whose line breaks the field counted once. The name has a pattern, white space in a one-line field becomes one space, a line break is counted once. - A semicolon let a formula through the CSV export where it is the list separator. Every piece that could begin a cell is made text. - A request with no mark of its visitor was not limited. They are counted together, as Mallok's own rate limit counts them. - A cart refused for the limit inside the write could be told it was sent. - The export scanned every line for every inquiry. And the tests of the two races proved less than they claimed: Promise.all interleaves two requests about half the time here, and the other half never reaches the guard. They now hold both requests at the write (holdWrites), and each guard, broken, is red every time.
An email handed to Mallok keeps what a buyer typed in Mallok's own queue, where deleting the inquiry does not reach it. The security notes, the guidance file and the design said otherwise; they say what is true now, with what an operator has to do about it. The design gains what the independent review of the inquiry cart found and what changed.
An independent review showed that two requests sent with Promise.all interleave only about half the time. The nine older tests of a race — payments, refunds, cancellations — call functions side by side instead, and a probe found those overlap every time: both readings before either write, in twenty-five rounds of twenty-five. They now run through atTheSameMoment, which gives each call a database that notes when it is used and fails unless every call had read before any wrote. A change that put a wait in front of a reading would have turned such a race into a sequence and left its test passing; now it fails. The helper has tests of its own, for the case it must refuse. With everything that stops its second write taken away, each of the nine was red eight times in eight.
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.
What this is
The review of the inquiry cart showed that two requests sent with
Promise.allinterleave only about half the time. This looks at the nine older tests of a race — a payment reported twice at once, two refunds, a payment against a cancellation — for the same weakness.They do not have it. They call functions side by side rather than send requests, and such calls overlap every time: in twenty-five rounds of two payments at once, both readings reached the database before either write, in one and the same order.
So none had to be rewritten. What changes is that this is now seen on every run instead of taken on trust: the nine run through
atTheSameMoment, which hands each call a database that notes when it is used and fails unless every call had read before any wrote. A change that put a wait in front of a reading would have turned such a race into a sequence and left its test passing.How it was verified
One thing that showed: a payment's write is stopped by several keys at once, so removing the shared status condition turns no race between two payments red. The races between two different changes of an order have no key in common, and do go red without it.