checkChannelHashcash looked a channel stamp up in spent_hashcash with one statement and recorded it with a second. The database connection is released between the two, so concurrent requests carrying the same stamp could all pass the lookup before any of them recorded it, and each of them was sent.
RecordSpentHashcash now returns whether its INSERT OR IGNORE added a row (RowsAffected), and checkChannelHashcash refuses the message with ERR_CANNOTSENDTOCHAN (404) when it did not. One statement is atomic in SQLite. IsHashcashSpent had no other caller and is removed.
TestChannelHashcashRejectConcurrentReplay in internal/handlers posts one stamp in 16 requests at once and checks that one is accepted and that the other channel member receives one copy.
Disclosures:
Judgement call: a failed write of the spent stamp now refuses the message with an internal error; before, the error was logged and the message sent anyway. The write is now the check, so sending without it would leave the stamp reusable.
Partially verified: on the code before this change the new test fails only on some runs, because it depends on how the requests interleave. With the change no interleaving can make it fail.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/neoirc/issues/150.
`checkChannelHashcash` looked a channel stamp up in `spent_hashcash` with one statement and recorded it with a second. The database connection is released between the two, so concurrent requests carrying the same stamp could all pass the lookup before any of them recorded it, and each of them was sent.
`RecordSpentHashcash` now returns whether its `INSERT OR IGNORE` added a row (`RowsAffected`), and `checkChannelHashcash` refuses the message with `ERR_CANNOTSENDTOCHAN` (404) when it did not. One statement is atomic in SQLite. `IsHashcashSpent` had no other caller and is removed.
`TestChannelHashcashRejectConcurrentReplay` in `internal/handlers` posts one stamp in 16 requests at once and checks that one is accepted and that the other channel member receives one copy.
Disclosures:
- Judgement call: a failed write of the spent stamp now refuses the message with an internal error; before, the error was logged and the message sent anyway. The write is now the check, so sending without it would leave the stamp reusable.
- Partially verified: on the code before this change the new test fails only on some runs, because it depends on how the requests interleave. With the change no interleaving can make it fail.
Model: opus-5-5
checkChannelHashcash looked a stamp up with one statement and recorded
it with another, so concurrent requests carrying the same stamp could
all pass the lookup before any of them recorded it, and each message
was sent. RecordSpentHashcash now reports whether its INSERT OR IGNORE
added a row, and the message is refused when it did not. A single
statement is atomic in SQLite. IsHashcashSpent had no other caller and
is removed. A failed write of the stamp now refuses the message instead
of being logged and ignored.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #150.
checkChannelHashcashlooked a channel stamp up inspent_hashcashwith one statement and recorded it with a second. The database connection is released between the two, so concurrent requests carrying the same stamp could all pass the lookup before any of them recorded it, and each of them was sent.RecordSpentHashcashnow returns whether itsINSERT OR IGNOREadded a row (RowsAffected), andcheckChannelHashcashrefuses the message withERR_CANNOTSENDTOCHAN(404) when it did not. One statement is atomic in SQLite.IsHashcashSpenthad no other caller and is removed.TestChannelHashcashRejectConcurrentReplayininternal/handlersposts one stamp in 16 requests at once and checks that one is accepted and that the other channel member receives one copy.Disclosures:
Model: opus-5-5
PASS
Model: opus-5-5