-
Notifications
You must be signed in to change notification settings - Fork 363
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
Parsing "Microsoft encoding" is very lenient #60
Comments
Thank you for pointing this out. Yes, this is a bug and should be fixed. Would be happy to have you send in a PR for this. |
Carrotman42
added a commit
to Carrotman42/uuid
that referenced
this issue
May 19, 2020
It was not intentional to simply ignore these characters, and examples/docs/tests all indicate that curly braces were the intended characters. See google#60
Carrotman42
added a commit
to Carrotman42/uuid
that referenced
this issue
May 19, 2020
It was not intentional to simply ignore these characters, and examples/docs/tests all indicate that curly braces were the intended characters. I've added some test cases to ensure things work. Fixes google#60
I've just come across this by virtue of the fact that:
Turns out it's not that it ignores whitespace if present at both ends, but it's just another version of this bug. |
12 tasks
This was referenced Dec 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I was reading through the code for Parse and I noticed that there is no validation on the first and last characters of the uuid-to-parse when the length of the input is 38 characters:
uuid/uuid.go
Lines 38 to 61 in 16ca3ea
I would expect there to be a check that
s[0] == '{' && s[37] == '}'
rather than simply ignoring those two characters. I am happy to send a PR if requested, it's a simple change. I am just verifying that this loose behavior is not actually desired.(fwiw, it means that parsing something like
a01234567-abcd-cdef-abcd-012345678901a
would be parsed without an error, even though it has extra characters at the beginning and end. That's quite unexpected IMO.)The text was updated successfully, but these errors were encountered: