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

uuid.Parse Function Does Not Handle Leading/Trailing Spaces in UUIDs #147

Open
mahmoudahmedd opened this issue Dec 31, 2023 · 2 comments
Open

Comments

@mahmoudahmedd
Copy link

The uuid.Parse function currently treats UUID strings with leading or trailing spaces as valid.

_, parseError := uuid.Parse(" a3bb189e-8bf9-3888-9912-ace4e6543002 ")
when parsing a UUID string like " a3bb189e-8bf9-3888-9912-ace4e6543002 " (with spaces around it), the function does not return an error (parseError is nil). https://go.dev/play/p/8eIO_36dfdD

It would be beneficial to either treat such strings as invalid ?

@jensilo
Copy link

jensilo commented Jul 20, 2024

Looking at uuid.Parse func and its doc block, this behaviour can be easily explained.

Microsoft's encoding of UUIDs encloses the UUID in { and }. Like this: {xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}. This UUID then has a length of 38 bytes. To be compatible with Microsoft's UUID encoding uuid.Parse, as the doc block states, only uses the middle 36 bytes of the passed 38 bytes, thereby cutting out the curly braces.

Now, you haven't provided a valid UUID in Microsoft's encoding, however the length is also 38 bytes. Hence, this does only work with ONE space on each side, not multiple. uuid.Parse uses only the length to detect the encoding, which mostly isn't a problem. However, here it leads to uuid.Parse believing your passed in UUID is in Microsoft's encoding. Therefore, it cuts out the first part (37 bytes left to process) and processes the next 36 bytes as a valid UUID. The last byte }, or in your case , is just ignored.

On a funny side note: Due to this behaviour you could enclose your valid UUID in any two bytes. As long as you get to 38 bytes length and have a valid UUID inside. See: https://go.dev/play/p/yIsYOhk4T9Y.

Of course, you could see this as an issue that needs to be fixed, but why? You could have a check that the 38 bytes long UUID in Microsoft's encoding starts with { and ends with } and otherwise have it return an error. I don't see a need for this. The current implementation is faster, as it includes less checks, and works just fine, there are no security-relevant errors to be expected and I couldn't come up with a valid reason for "fixing" this, apart from "it's just right that way".

Btw. your example is weird, you could prove your point with one line: fmt.Println(uuid.MustParse(" a3bb189e-8bf9-3888-9912-ace4e6543002 ")).

@jensilo
Copy link

jensilo commented Jul 20, 2024

Oh, I just saw this is a duplicate of #60 and has been discussed already.

TL;DR: Changing this behaviour in uuid.Parse could break existing implementations. Therefore, this is so far untouched.

I think this issue can be closed as well.

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

No branches or pull requests

2 participants