Repository navigation
fix: reject a timer_id over 255 characters before the INSERT - #239
Merged
Merged
Conversation
SQLAlchemy 2.0.x binds String(255) values as $1::VARCHAR(255), and Postgres truncates an explicit cast instead of rejecting it, so an over-long timer_id was stored truncated and could dedup against a different timer sharing its first 255 characters. Newer SQLAlchemy raised an opaque DBAPIError instead. Validate it with the queue.
Benchmark gate✅ gate passed
Gated (fails the build): |
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.
Rejects a
timer_idover 255 characters withValueErrorbefore the INSERT, the same wayqueueis already rejected.Why
timer_idis aString(255)column, but nothing checked its length before the INSERT. What happened next depended on the SQLAlchemy version:$1::VARCHAR(255). Postgres truncates an explicit cast instead of raising, so the row landed with itstimer_idcut to 255 characters. Two different timers sharing their first 255 characters then deduplicated against each other, and the second publish returnedNone. I confirmed the cast by compiling the INSERT with 2.0.0. The floors 3.11 and 3.12 jobs on refactor: move the outbox insert and its NOTIFY into the client #237 showed the truncation.DBAPIError.The change
_validate_publish_argsnow checkstimer_idlength next toqueue. It runs forbroker.publish,publisher.publishandOutboxResponse, which raises when it's constructed, like its other checks. The message matches the one the fake client already raised:timer_id must be at most 255 characters (got N).Tests
Written first and failing:
test_publish_command_rejects_oversize_timer_idandtest_outbox_response_rejects_oversize_timer_id;test_publish_rejects_an_oversize_timer_id_without_storing_a_row, on real Postgres. It failed on current SQLAlchemy with aDBAPIError. On 2.0.0 it would have stored a truncated row; the floors jobs run it there.test_publish_command_accepts_timer_id_at_the_column_limitpins the boundary.Verification
ruffandtyare clean.