Skip to content

Fix uuid() accepting {, }, and prefixes inside the string #359

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.

Sign up for GitHub

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

Open
edorian wants to merge 1 commit into
base: master
Choose a base branch
from
+13 −11
Open

Fix uuid() accepting {, }, and prefixes inside the string #359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions src/Assert.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2442,24 +2442,18 @@ public static function uuid(mixed $value, string|callable $message = ''): string
{
static::string($value, $message);

$originalValue = $value;
$value = \str_replace(['urn:', 'uuid:', '{', '}'], '', $value);

// The nil UUID is special form of UUID that is specified to have all
// 128 bits set to zero.
if ('00000000-0000-0000-0000-000000000000' === $value) {
Comment thread
shadowhand marked this conversation as resolved.
return $originalValue;
}

if (!\preg_match('/^[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}$/D', $value)) {
// Accepts the plain form (including the nil UUID with all 128 bits
// set to zero), optionally preceded by "urn:" and/or "uuid:" and
// optionally wrapped in a matching pair of curly braces.
if (!\preg_match('/^(?:urn:)?(?:uuid:)?(\{)?[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}(?(1)\})$/D', $value)) {
Comment on lines +2445 to +2448

@shadowhand shadowhand Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better to break out these different forms, such that it looks like:

if (\str_starts_with($value, 'urn:uuid:') && \preg_match(...)) {
    return $value;
}

if (\str_starts_with($value, 'uuid:') && \preg_match(...)) {
    return $value;
}

if (\str_starts_with($value, '{') && \str_ends_with('}') && \preg_match(...)) {
    return $value;
}

if (\preg_match(...)) {
    return $value;
}

// resolve message, report invalid

While more verbose, having separate forks for each form makes intent much more clear and provides more meaningful code coverage.

@edorian edorian Jun 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Given uuid:{ff6f8cb0-c57d-21e1-9b21-0800200c9a66} should stay valid at least the brace matching should be kept then? Or would you rather see the prefixes and braces removed and then just the raw uuid validated?

So do you want something like this that reuses the main regex:

diff --git a/src/Assert.php b/src/Assert.php
index 089b020..31c861b 100644
--- a/src/Assert.php
+++ b/src/Assert.php
@@ -2442,18 +2442,33 @@ class Assert
     {
         static::string($value, $message);
 
-        // Accepts the plain form (including the nil UUID with all 128 bits
-        // set to zero), optionally preceded by "urn:" and/or "uuid:" and
-        // optionally wrapped in a matching pair of curly braces.
-        if (!\preg_match('/^(?:urn:)?(?:uuid:)?(\{)?[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}(?(1)\})$/D', $value)) {
-            $message = self::resolveMessage($message);
-            static::reportInvalidArgument(\sprintf(
-                $message ?: 'Value %s is not a valid UUID.',
-                static::valueToString($value)
-            ));
+        $uuid = '[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}';
+
+        if (\str_starts_with($value, 'urn:uuid:') && \preg_match('/^urn:uuid:'.$uuid.'$/D', $value)) {
+            return $value;
         }
 
-        return $value;
+        if (\str_starts_with($value, 'uuid:') && \preg_match('/^uuid:(?:'.$uuid.'|\{'.$uuid.'\})$/D', $value)) {
+            return $value;
+        }
+
+        if (\str_starts_with($value, '{') && \str_ends_with($value, '}') && \preg_match('/^\{'.$uuid.'\}$/D', $value)) {
+            return $value;
+        }
+
+        if (\preg_match('/^'.$uuid.'$/D', $value)) {
+            return $value;
+        }
+
+        $message = self::resolveMessage($message);
+        static::reportInvalidArgument(\sprintf(
+            $message ?: 'Value %s is not a valid UUID.',
+            static::valueToString($value)
+        ));
     }

Alternatively stripping the str_starts_with matched parts from $value is possible.

I'm happy to do whatever you want here, no stake in the specific solution. Just trying to understand :)

@shadowhand shadowhand Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Your suggested code looks good to me.

$message = self::resolveMessage($message);
static::reportInvalidArgument(\sprintf(
$message ?: 'Value %s is not a valid UUID.',
static::valueToString($value)
));
}

return $originalValue;
return $value;
}

/**
Expand Down
8 changes: 8 additions & 0 deletions tests/AssertTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,9 @@ public static function getTests(): array
['isNonEmptyMap', [[1, 2, 3]], false],
['uuid', ['00000000-0000-0000-0000-000000000000'], true],
['uuid', ['urn:ff6f8cb0-c57d-21e1-9b21-0800200c9a66'], true],

@shadowhand shadowhand Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should actually be removed, it is technically not a valid urn: per RFC 4122/9562, since RFC 2141 defines a URN as urn:<NID>:<NSS>.

['uuid', ['urn:uuid:ff6f8cb0-c57d-21e1-9b21-0800200c9a66'], true],
['uuid', ['uuid:{ff6f8cb0-c57d-21e1-9b21-0800200c9a66}'], true],
['uuid', ['{ff6f8cb0-c57d-21e1-9b21-0800200c9a66}'], true],
['uuid', ['ff6f8cb0-c57d-21e1-9b21-0800200c9a66'], true],
['uuid', ['ff6f8cb0-c57d-11e1-9b21-0800200c9a66'], true],
['uuid', ['ff6f8cb0-c57d-31e1-9b21-0800200c9a66'], true],
Expand All @@ -603,6 +605,12 @@ public static function getTests(): array
['uuid', ['ff6f8cb0-c57da-51e1-9b21-0800200c9a66'], false],
['uuid', ['af6f8cb-c57d-11e1-9b21-0800200c9a66'], false],
['uuid', ['3f6f8cb0-c57d-11e1-9b21-0800200c9a6'], false],
['uuid', ['f{}f6f8cb0-c57d-21e1-9b21-0800200c9a66'], false],
['uuid', ['ff6f8cb0-c57d-21e1-9b21-08002urn:00c9a66'], false],
['uuid', ['urn:ff6f8cb0-c57d-21e1-9b21-0800200c9a66uuid:'], false],
['uuid', ['{}{}ff6f8cb0-c57d-21e1-9b21-0800200c9a66{}{}'], false],
['uuid', ['{ff6f8cb0-c57d-21e1-9b21-0800200c9a66'], false],
['uuid', ['ff6f8cb0-c57d-21e1-9b21-0800200c9a66}'], false],
['throws', [function () { throw new LogicException('test'); }, 'LogicException'], true],
['throws', [function () { throw new LogicException('test'); }, 'IllogicException'], false],
['throws', [function () { throw new Exception('test'); }], true],
Expand Down
Loading