Skip to content

Add UUID v4 generation to the standard library. #6618

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

Closed
wants to merge 2 commits into from
Closed

Add UUID v4 generation to the standard library. #6618

wants to merge 2 commits into from

Conversation

IridescentRose
Copy link
Contributor

This is a small addition to the standard library which allows you to create a UUID v4 UUID with an allocator and random number generator.

Example use:

const std = @import("std);

pub fn main() !void {
    const uuid = try std.uuid.newv4(your_allocator, your_random);
    std.debug.print("{}", .{uuid.id});
}

Output:
b29f4b7f-945b-4e55-b95b-b6ebeab198e2

@daurnimator daurnimator added the standard library This issue involves writing Zig code for the standard library. label Oct 9, 2020
@jedisct1
Copy link
Contributor

jedisct1 commented Oct 9, 2020

UUIDs must be unique, and accepting an arbitrary RNG doesn't guarantee that. An application may inadvertently use a deterministic or badly seeded RNG.

Do we really want the application to provide the RNG? Not having to do so would also simplify the interface.

@IridescentRose
Copy link
Contributor Author

@jedisct1 How would you propose making it truly unique?

@jedisct1
Copy link
Contributor

jedisct1 commented Oct 9, 2020

@jedisct1 How would you propose making it truly unique?

With crypto.randomBytes(). That wouldn't be "truly unique", but the probability of a UUID collision would be negligible.

@data-man
Copy link
Contributor

data-man commented Oct 9, 2020

For comparison: uuid-rs/uuid

v1 - adds the Uuid::new_v1 function and the ability to create a V1 using an implementation of uuid::v1::ClockSequence (usually uuid::v1::Context) and a timestamp from time::timespec.
v3 - adds the Uuid::new_v3 function and the ability to create a V3 UUID based on the MD5 hash of some data.
v4 - adds the Uuid::new_v4 function and the ability to randomly generate a Uuid.
v5 - adds the Uuid::new_v5 function and the ability to create a V5 UUID based on the SHA1 hash of some data.

@jsyrjala
Copy link
Contributor

jsyrjala commented Oct 9, 2020

@jedisct1 How would you propose making it truly unique?

With crypto.randomBytes(). That wouldn't be "truly unique", but the probability of a UUID collision would be negligible.

I think there should be methods for both with and without rng parameter. Sometimes gathering entropy in kernel can be slow. E.g app running in a freshly started Docker container. I have seen cases where it takes minutes to gather enough entropy to generate a couple of random UUIDs.

@jedisct1
Copy link
Contributor

jedisct1 commented Oct 9, 2020

Was is with the randomBytes() function?

On Linux, it is using the getrandom() system call which should never block; there's enough entropy gathered when the system starts to provide all the randomness you will ever need later.

On BSDs, it is using arc4random() that never blocks. Same on WASI. If these functions are not available, it is using /dev/urandom (not /dev/random) that doesn't block either.

If something ever blocks, it means that the system is unable to generate random numbers, so it would be unable to generate UUIDs based on random numbers anyway.

@jsyrjala
Copy link
Contributor

jsyrjala commented Oct 9, 2020

Was is with the randomBytes() function?

On Linux, it is using the getrandom() system call which should never block; there's enough entropy gathered when the system starts to provide all the randomness you will ever need later.

On BSDs, it is using arc4random() that never blocks. Same on WASI. If these functions are not available, it is using /dev/urandom (not /dev/random) that doesn't block either.

If something ever blocks, it means that the system is unable to generate random numbers, so it would be unable to generate UUIDs based on random numbers anyway.

It was not related to zig. I don't remember exact details, it used /dev/random or some other thing that definitely blocked, though.

@daurnimator
Copy link
Contributor

daurnimator commented Oct 9, 2020 via email

@jedisct1
Copy link
Contributor

jedisct1 commented Oct 9, 2020

#6622 explains my concerns about requiring a Rand parameter in functions needing secure random numbers.

We currently don't have any safe option to use in that context. And if we had one, would we need more?

@IridescentRose
Copy link
Contributor Author

I'll get on this tomorrow

Copy link
Contributor

@daurnimator daurnimator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good enough to me now!

Left a couple of nits that don't need to be fixed

Also, should have space after // before comment.

lib/std/uuid.zig Outdated
const flip: u128 = 0b1111 << 48;

//We then set the 4 bits at 48 to 0x4 (0b0100)
return Self{ .id = ((gen.int(u128) & ~flip) | (0x4 << 48)) };
Copy link
Contributor

@daurnimator daurnimator Oct 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's an extra set of parenthesis here (the outermost pair)

lib/std/uuid.zig Outdated
const selector = @truncate(u4, self.id >> shift);
shift += 4;

buf[i] = chars[@intCast(usize, selector)];
Copy link
Contributor

@daurnimator daurnimator Oct 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for an @intCast here: a cast from u4 to usize will be implicit.

lib/std/uuid.zig Outdated
pub fn format(value: Self, comptime fmt: []const u8, options: std.fmt.FormatOptions, writer: anytype) !void {
var buf = [_]u8{0} ** 36;

var chars = "0123456789ABCDEF";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should set to undefined here: var chars: *const [16]u8 = undefined;

Or could reorganise to have:

const chars = if ( ..... ) ... else if (....) ... else ...;

@andrewrk
Copy link
Member

UUIDs must be unique, and accepting an arbitrary RNG doesn't guarantee that. An application may inadvertently use a deterministic or badly seeded RNG.

Do we really want the application to provide the RNG? Not having to do so would also simplify the interface.

Here's a proposal which is intended to address this use case: #6704

Idea here would be the API could be a simple, global, infallible call to gimmeDatUUID() and it would use this thread-local CSPRNG as the backing RNG.

@andrewrk
Copy link
Member

To be clear, this PR is blocking on #6704

@andrewrk
Copy link
Member

This is about to get unblocked by #7482

@andrewrk
Copy link
Member

std.crypto.random is now available to use.

@Vexu Vexu closed this Dec 23, 2020
@Vexu Vexu reopened this Dec 23, 2020
@andrewrk
Copy link
Member

Upon further reflection, I think this code is appropriate for a third party library rather than the std lib.

@andrewrk andrewrk closed this Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants