-
Notifications
You must be signed in to change notification settings - Fork 128
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
Port _CShims UUID to Swift #129
base: main
Are you sure you want to change the base?
Conversation
} | ||
if res != 0 { | ||
return nil | ||
public init?(version: Version = .v4, uuidString string: __shared String) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be an ABI breakage, since we're removing the previous initializer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks for letting me know Tony.
I restored the original initializers in c3cc89c, however I also understand that the changes in this PR are something that the team aren't ready to look at yet.
One interesting security feature of our UUID implementation is that it is designed to do its work in constant time regardless of the content of the UUID. e.g., for equality checking. This is done in order to defeat timing attacks. We would need to verify we keep that behavior with a new implementation. |
} | ||
|
||
static func v4_parse(uuidString string: __shared String) -> uuid_t? { | ||
let components = string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on #129 (comment), one obvious optimization we can do here is to guard
against string.count
. However, both String.count
and String.UTF8View.count
is not O(1), so I guess we should parse the string manually and throw
if it’s malformed, including too short or too long?
This is a spike for my own learning to see what's involved in removing the C-backed UUID implementation in favour of a Swift version.
I'm happy to revise this, as I realise that the implementation of some of this isn't ideal.