Problem/Motivation
With current implementations uuids with uppercase letters are not valid. There should not be difference between uppercase and lowercase letters in uuids?
Proposed resolution
Fix regex pattern in Uuid to consider uppercase letters.
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | uuid-is-valid-2576331-11.patch | 1.56 KB | djevans |
| #5 | uuid_isvalid_is-2576331-5.patch | 536 bytes | cilefen |
| #4 | uuid_isvalid_is-2576331-4.patch | 529 bytes | mbovan |
Comments
Comment #2
cilefen commentedAs far as I know, uppercase characters are not allowed in UUIDs, so an uppercased string should be invalid.
Comment #3
djevans commentedAccording to RFC4122:
So the current regex pattern in
Uuid::isValid()is correct, if we're reading an existing entity's UUID.Would it be better to:
a) add something to the long description of the function, to recommend wrapping the argument in
strtolower()if it contains digits A-F, orb) add a default parameter to the method signature - something like this:
I'd suggest a) as being simpler.
Comment #4
mbovan commentedExtended method documentation with a comment about handling uppercase letters in a UUID.
Comment #5
cilefen commentedRe #3:
I see now. Wouldn't this be better?
Comment #6
cilefen commentedOh, I see #3 vs #5, the issue is input vs output.
Comment #7
dawehnerYeah better adapt the regex we use. Do you pointing to the right location in the RFC from within the documentation?
Comment #8
cilefen commentedTo anyone interested, you need to edit Drupal\Tests\Component\Uuid\UuidTest::providerTestValidation and add some uppercased UUIDs.
Comment #9
cilefen commentedThe test modification is suitable for a Novice.
Comment #10
djevans commentedComment #11
djevans commentedAttached a patch to update the regex (with documentation), and test a UUID with uppercase digits.
Comment #12
cilefen commented@djevans That's looking pretty good.
Comment #15
cilefen commentedI am wondering what the effect of this patch could be on storing UUIDs. Could we now store an upper-cased UUID and maybe output it the same way, which (I guess) by the RFC is not ok?
Comment #16
mbovan commentedIs
Uuid::isValid()considered when we generate a UUID? I guess not.Btw, for some reason, I have all UUIDs in my database saved in uppercase format.
Comment #17
djevans commented@mbovan No,
UuidInterface::generate()doesn't make any validation checks.You can also pass arbitrary UUIDs into
Entity::create(), so something like this will work (although maybe it shouldn't):Comment #18
berdirWe could define a valid uuid constraint on UuidItem but that seems like a separate task.
We've seen uppercase UUID's when importing data from external systems and yes, for some weird reasons, @mbovan's Drupal generates uppercase UUID's ;)
It's not exactly obvious that you have to convert them to lowercase or somewhere down the line it suddenly won't work. But I guess that's how it is, I agree that we shouldn't allow something that is clearly against the standard.
Comment #19
berdirFrom that quite in #3:
However, see my explanation in #2613926: The PECL UUID implementation can return invalid UUIDs, it's better to enforce it, so we should possibly close this as duplicate of that issue?
Comment #20
cilefen commentedI agree.