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

Validate #72

Closed
wants to merge 2 commits into from
Closed

Validate #72

wants to merge 2 commits into from

Conversation

zeke
Copy link
Contributor

@zeke zeke commented Sep 21, 2013

Adds a uuid.validate(str) method; derived from the comments in https://github.com/broofa/node-uuid/issues/41

This was referenced Sep 21, 2013
@@ -91,6 +91,11 @@
bth[buf[i++]] + bth[buf[i++]];
}

// **`validate(str)` - Test whether given string a valid UUID**
function validate(str) {
return /[0-9a-f]{22}|[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i.test(str);
Copy link
Member

Choose a reason for hiding this comment

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

Hey zeke, I know you pulled his from #41, where I offered this as a one-liner solution, but if we're going to add a regex-based validate() method, it should conform as closely as possible to what RFC4122 does/doesn't allow. Specifically:

Thus, this regex:

/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-5][0-9a-fA-F]{3}-[89ABab][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/

(edited: to add ^ & $ to regex)

@defunctzombie
Copy link
Collaborator

The more I think about this, the more I fall on the side that this belongs in a separate lib. Possibly one of the web framework validation checker libs that already exist. Then again, the method implementation is so tiny that having it here might not be an issue.

@broofa
Copy link
Member

broofa commented Sep 22, 2013

@shtylman - yeah, that's kind of where my heads been at. This is the 3rd or 4th request for uuid validation I've had though, and I can see why it'd be nice to have here. But validation is a slippery slope; I expect everyone has slightly different requirements. I don't know if my suggested regex above is actually useful to anyone, or if it's just an interesting example that people are likely to want to modify to meet their specific needs.

(And I wouldn't be surprised if it leads to more issues than it solves, as people bitch about it rejecting uuids that look right, but that have invalid version/variant fields. That's probably just a documentation issue though.)

@zeke
Copy link
Contributor Author

zeke commented Oct 30, 2013

I just came across a module called validator that appears to have some UUID validations built in: https://github.com/zeke/node-validator/blob/afd1a45b018880d1cb627055d9722a5c7f3cc822/test/validator.test.js#L403

Maybe I should use that instead of trying to get this PR merged into node-uuid?

@zeke zeke closed this Oct 23, 2015
@zeke zeke deleted the validate branch October 23, 2015 02:33
@broofa broofa mentioned this pull request Feb 18, 2017
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