Skip to content
New issue

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

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

Already on GitHub? # to your account

Should we add checks for valid variant (and version)? #198

Open
kohenkatz opened this issue Feb 9, 2025 · 2 comments
Open

Should we add checks for valid variant (and version)? #198

kohenkatz opened this issue Feb 9, 2025 · 2 comments

Comments

@kohenkatz
Copy link
Contributor

kohenkatz commented Feb 9, 2025

Related, but minor, the tests above, ironically don't set the UUIDv7 variant bits correctly. (the relevant hex character should probably be a b for the max UUIDv7 and an 8 for the min UUDv7)

Originally posted by @nathanmcgarvey-modopayments in #195


Looking through our tests, we have a number of places where a text-format UUID ends with ffff-ffffffffffff, even though these should be bfff-ffffffffffff to indicate the correct variant.

Should we have a validation in UUID.Parse that makes sure the variant is valid? What about making sure the version is valid? Right now, we don't do any checking for this, and we don't have a method for the user to check validity either.

@nathanmcgarvey-modopayments
Copy link
Contributor

Variant will be hard to validate as you have all possible combinations of two bits:

// UUID layout variants.
const (
	VariantNCS byte = iota
	VariantRFC9562
	VariantMicrosoft
	VariantFuture
)

So what is "valid" in that field could get squishy unless you want strict RFC conformance. (And at that, I'd have to go look it up to see how strict they are.)

Version, however has to be validated somewhere. I can't just get a timestamp from a non-v1,v6,or v7 UUID. But maybe there's no ingest validation. Just when certain parsing is requested.

@kohenkatz
Copy link
Contributor Author

Version, however has to be validated somewhere. I can't just get a timestamp from a non-v1,v6,or v7 UUID. But maybe there's no ingest validation. Just when certain parsing is requested.

Validation for this happens in the TimestampFromV1, TimestampFromV6, and TimestampFromV7 methods. For example:

func TimestampFromV7(u UUID) (Timestamp, error) {
	if u.Version() != 7 {
		return 0, fmt.Errorf("%w %s is version %d, not version 7", ErrInvalidVersion, u, u.Version())
	}

	...

But there is no validation when creating the UUID object from text (or binary) input.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants