Problem/Motivation

See #2613926: The PECL UUID implementation can return invalid UUIDs for background information.

We only consider lowercase UUIDs as valid for consistency and compatibility reasons even though the spec for UUIDs does not require this. \Drupal\Component\Uuid\Uuid::isValid() does not mention this, however, although this is an important detail that we should tell people.

Proposed resolution

See #15

Remaining tasks

Review
Commit

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#9 2625142-9.patch435 bytesfelribeiro
#6 2625142-6.patch364 bytesfelribeiro
#3 2625142-3.patch354 bytesfelribeiro

Issue fork drupal-2625142

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Related issues: +#2613926: The PECL UUID implementation can return invalid UUIDs
felribeiro’s picture

felribeiro commented
StatusFileSize
new 2625142-3.patch354 bytes
felribeiro’s picture

felribeiro commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: 2625142-3.patch, failed testing.

felribeiro’s picture

felribeiro commented
StatusFileSize
new 2625142-6.patch364 bytes
felribeiro’s picture

felribeiro commented
Status: Needs work » Needs review
cilefen’s picture

cilefen commented
Status: Needs review » Needs work 
+++ b/core/lib/Drupal/Component/Uuid/Uuid.php
@@ -19,6 +19,7 @@
    * Checks that a string appears to be in the format of a UUID.
+   * It only allows lowercase string.
    *

There must be one space after the single-line summary comment. See the documentation standards.

"It only allows lowercase string." doesn't really work. What about "Only lower case strings are considered valid for consistency and compatibility reasons."?

felribeiro’s picture

felribeiro commented
StatusFileSize
new 2625142-9.patch435 bytes

@cilefen Thanks for your considerations.

felribeiro’s picture

felribeiro commented
Status: Needs work » Needs review
cilefen’s picture

cilefen commented
Status: Needs review » Postponed

+1 This looks good to me. It must wait for the parent issue to go in so I am marking it "postponed".

hass’s picture

hass commented
Category: Task » Bug report
Status: Postponed » Needs work

If the spec allows upper case we need to support the same or people run into compatibility issues with other 3rd party integrations.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Category: Bug report » Task
Status: Needs work » Postponed

@hass: Please see #2613926: The PECL UUID implementation can return invalid UUIDs for more information and possibly comment there.

arla’s picture

arla commented

The relevant part of RFC 4122 is:

The hexadecimal values "a" through "f" are output as lower case characters and are case insensitive on input.

And the technically equivalent ITU-T Rec. X.667:

6.5.4 Software generating the hexadecimal representation of a UUID shall not use upper case letters.

NOTE – It is recommended that the hexadecimal representation used in all human-readable formats be restricted to lower-case letters. Software processing this representation is, however, required to accept both upper and lower case letters as specified in 6.5.2.

So when it comes to implementing a uuid validator, you have to know if you are validating output or input. We seem to be using this validator specifically for our own generated UUIDs, which I guess we can classify as "output". Then perhaps we should specify that (add another statement to the method doc), so people won't use it to validate "input".

Re: hass' argument, which was elaborated in the other issue as "E.g. Webservices can return this to me and than I need to save as is as it is a unique reference in a remote system that is not in my hand." Per the specs, web services must return lower-case uuids. If you are getting upper-case strings, then they cannot be proper uuids.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Regarding this patch, it would be a lot more concise if you just put the "lower-case" into the existing one-line documentation:

Checks that a string appears to be in the format of a lower-case UUID.
hass’s picture

hass commented

I just edited an MSI setup 4 hours ago and it complained when i used uuids with lowercase letters. It was required to make them uppercase only. Lowercase have been invalid here.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Windows environments mostly use *G*UID, which are often upper case.

See https://en.wikipedia.org/wiki/Globally_unique_identifier vs. https://en.wikipedia.org/wiki/Universally_unique_identifier, the examples are uppercase vs. lowercase.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone made their first commit to this issue’s fork.

quietone opened merge request !9056

quietone’s picture

quietone commented
Issue summary: View changes
Issue tags: -Novice

Implemented solution in #15.

quietone’s picture

quietone commented
Status: Postponed » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Seems straight forward.

nod_ closed merge request !9056

  • nod_ committed 4abd5620 on 10.4.x 
    Issue #2625142 by felribeiro, quietone, tstoeckler, cilefen, Arla,...

  • nod_ committed b8dc375a on 11.x 
    Issue #2625142 by felribeiro, quietone, tstoeckler, cilefen, Arla,...
nod_’s picture

nod_
Primary language French
Location Lille
commented
Version: 11.x-dev » 10.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed b8dc375a05 to 11.x and 4abd56206c to 10.4.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.