Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid 14 bit integer overflow in clockseq #26

Merged
merged 14 commits into from
Dec 30, 2011

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Dec 19, 2011

I just got the quite unlikely case of a clockseq overlow:

$ node test/test.js 
Pass: uuids with current time have expected order
Pass: uuids with time option have expected order
Pass: IDs created at same msec are different
Fail: IDs created at same 100 nsec throw an error
Pass: IDs created at t and t - 100ns have different clockseq
Pass: Explicit options produce expected id
Pass: 1ns separation between adjacent uuids
Pass: Short parse
Pass: Dirty parse

Sanity check 10000 v1 uuids

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
Error: 5470a8a0-2a5d-11e1-c000-7938e6977d1e is not a valid UUID string
    at Error (unknown source)
    at Object.<anonymous> (/****/node-uuid/test/test.js:157:13)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)

The clock-seq-part is c000 which is invalid. Apparently we got an overflow from 0x3fff to 0x4000 of the clockseq here.

I tried to write a test to catch this, but it's currently not possible since we cannot control the _seedBytes from the outside. Still I think my patch should fix this.

Sorry, that there's some other old stuff in the commit range, I don't really get it how to keep that out of here....

broofa added a commit that referenced this pull request Dec 30, 2011
Avoid 14 bit integer overflow in clockseq
@broofa broofa merged commit 2f6608a into uuidjs:explicit_time Dec 30, 2011
@broofa
Copy link
Member

broofa commented Dec 30, 2011

Good catch, btw. Sorry I didn't merge this before now - 'Been really busy with work and holiday stuff.

qgerome pushed a commit to qgerome/uuid that referenced this pull request May 27, 2020
Update mocha to the latest version 🚀
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants