gh-138862: fix timestamp increment after UUIDv7 counter overflow#146606
gh-138862: fix timestamp increment after UUIDv7 counter overflow#146606picnixz wants to merge 10 commits intopython:mainfrom
Conversation
|
I'll need to add a test for multiple overflows to see that I only increment on overflow. |
|
@hugovk @eendebakpt Could you have a look at this? TL;DR: it shouldn't happen in practice because the counter is a randomized 41-bit integer and its max value is From a practical PoV, So, for a realistic attack, (or issue) one needs one of the following situations to happen:
|
| # after a counter overflow. We follow the RFC for in the former | ||
| # case. In the latter case, we re-use the already advanced | ||
| # timestamp (it was updated when we detected the overflow). | ||
| if _last_counter_v7_overflow: |
There was a problem hiding this comment.
The first case from the if-else is the "latter" case in the comments above. Maybe swap the order in the comments.
There was a problem hiding this comment.
Oh right :') I changed so much this block that I forgot to update the comment. Thanks!
|
@picnixz I agree with your analysis. Whether or not to merge this PR (it fixes the bug) or leave as is (this will not happen in practice) I am 50/50 on. |
|
I don't think we are loosing much speed. We are only adding one comparison every time we are within the same millisecond. But this adds a bit complexity that we will need to replicate in C though not necessarily hard to do. But still, it's a bit annoying I think. I can also document the implementation more. |
Uh oh!
There was an error while loading. Please reload this page.