Skip to content

[Uid] Add support for binary, base-32 and base-58 representations in Uuid::isValid() #57940

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

Merged
merged 1 commit into from
Aug 13, 2024
Merged

[Uid] Add support for binary, base-32 and base-58 representations in Uuid::isValid() #57940

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
1 change: 1 addition & 0 deletions src/Symfony/Component/Uid/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CHANGELOG
---

* Make `AbstractUid` implement `Ds\Hashable` if available
* Add support for binary, base-32 and base-58 representations in `Uuid::isValid()`

7.1
---
Expand Down
32 changes: 32 additions & 0 deletions src/Symfony/Component/Uid/Tests/UuidTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,38 @@ public function testIsValid()
$this->assertTrue(UuidV4::isValid(self::A_UUID_V4));
}

public function testIsValidWithVariousFormat()
{
$uuid = Uuid::v4();

$this->assertTrue(Uuid::isValid($uuid->toBase32(), Uuid::FORMAT_BASE_32));
$this->assertFalse(Uuid::isValid($uuid->toBase58(), Uuid::FORMAT_BASE_32));
$this->assertFalse(Uuid::isValid($uuid->toBinary(), Uuid::FORMAT_BASE_32));
$this->assertFalse(Uuid::isValid($uuid->toRfc4122(), Uuid::FORMAT_BASE_32));

$this->assertFalse(Uuid::isValid($uuid->toBase32(), Uuid::FORMAT_BASE_58));
$this->assertTrue(Uuid::isValid($uuid->toBase58(), Uuid::FORMAT_BASE_58));
$this->assertFalse(Uuid::isValid($uuid->toBinary(), Uuid::FORMAT_BASE_58));
$this->assertFalse(Uuid::isValid($uuid->toRfc4122(), Uuid::FORMAT_BASE_58));

$this->assertFalse(Uuid::isValid($uuid->toBase32(), Uuid::FORMAT_BINARY));
$this->assertFalse(Uuid::isValid($uuid->toBase58(), Uuid::FORMAT_BINARY));
$this->assertTrue(Uuid::isValid($uuid->toBinary(), Uuid::FORMAT_BINARY));
$this->assertFalse(Uuid::isValid($uuid->toRfc4122(), Uuid::FORMAT_BINARY));

$this->assertFalse(Uuid::isValid($uuid->toBase32(), Uuid::FORMAT_RFC_4122));
$this->assertFalse(Uuid::isValid($uuid->toBase58(), Uuid::FORMAT_RFC_4122));
$this->assertFalse(Uuid::isValid($uuid->toBinary(), Uuid::FORMAT_RFC_4122));
$this->assertTrue(Uuid::isValid($uuid->toRfc4122(), Uuid::FORMAT_RFC_4122));

$this->assertTrue(Uuid::isValid($uuid->toBase32(), Uuid::FORMAT_ALL));
$this->assertTrue(Uuid::isValid($uuid->toBase58(), Uuid::FORMAT_ALL));
$this->assertTrue(Uuid::isValid($uuid->toBinary(), Uuid::FORMAT_ALL));
$this->assertTrue(Uuid::isValid($uuid->toRfc4122(), Uuid::FORMAT_ALL));

$this->assertFalse(Uuid::isValid('30J7CNpDMfXPZrCsn4Cgey', Uuid::FORMAT_BASE_58), 'Fake base-58 string with the "O" forbidden char is not valid');
}

public function testIsValidWithNilUuid()
{
$this->assertTrue(Uuid::isValid('00000000-0000-0000-0000-000000000000'));
Expand Down
68 changes: 51 additions & 17 deletions src/Symfony/Component/Uid/Uuid.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
public const NAMESPACE_OID = '6ba7b812-9dad-11d1-80b4-00c04fd430c8';
public const NAMESPACE_X500 = '6ba7b814-9dad-11d1-80b4-00c04fd430c8';

public const FORMAT_BINARY = 1;
public const FORMAT_BASE_32 = 1 << 1;
public const FORMAT_BASE_58 = 1 << 2;
public const FORMAT_RFC_4122 = 1 << 3;
public const FORMAT_ALL = -1;

protected const TYPE = 0;
protected const NIL = '00000000-0000-0000-0000-000000000000';
protected const MAX = 'ffffffff-ffff-ffff-ffff-ffffffffffff';
Expand All @@ -44,22 +50,7 @@

public static function fromString(string $uuid): static
{
if (22 === \strlen($uuid) && 22 === strspn($uuid, BinaryUtil::BASE58[''])) {
$uuid = str_pad(BinaryUtil::fromBase($uuid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);
}

if (16 === \strlen($uuid)) {
// don't use uuid_unparse(), it's slower
$uuid = bin2hex($uuid);
$uuid = substr_replace($uuid, '-', 8, 0);
$uuid = substr_replace($uuid, '-', 13, 0);
$uuid = substr_replace($uuid, '-', 18, 0);
$uuid = substr_replace($uuid, '-', 23, 0);
} elseif (26 === \strlen($uuid) && Ulid::isValid($uuid)) {
$ulid = new NilUlid();
$ulid->uid = strtoupper($uuid);
$uuid = $ulid->toRfc4122();
}
$uuid = self::transformToRfc4122($uuid, self::FORMAT_ALL);

if (__CLASS__ !== static::class || 36 !== \strlen($uuid)) {
return new static($uuid);
Expand Down Expand Up @@ -130,8 +121,19 @@
return new UuidV8($uuid);
}

public static function isValid(string $uuid): bool
/**
* @param int-mask-of<Uuid::FORMAT_*> $format
*/
public static function isValid(string $uuid /*, int $format = self::FORMAT_RFC_4122 */): bool

Check failure on line 127 in src/Symfony/Component/Uid/Uuid.php

View workflow job for this annotation

GitHub Actions / Psalm

ParamNameMismatch 

src/Symfony/Component/Uid/Uuid.php:127:43: ParamNameMismatch: Argument 1 of Symfony\Component\Uid\Uuid::isValid has wrong name $uuid, expecting $uid as defined by Symfony\Component\Uid\AbstractUid::isValid (see https://psalm.dev/230)

Check failure on line 127 in src/Symfony/Component/Uid/Uuid.php

View workflow job for this annotation

GitHub Actions / Psalm

ParamNameMismatch 

src/Symfony/Component/Uid/Uuid.php:127:43: ParamNameMismatch: Argument 1 of Symfony\Component\Uid\Uuid::isValid has wrong name $uuid, expecting $uid as defined by Symfony\Component\Uid\AbstractUid::isValid (see https://psalm.dev/230)
{
$format = 1 < \func_num_args() ? func_get_arg(1) : self::FORMAT_RFC_4122;

if (36 === \strlen($uuid) && !($format & self::FORMAT_RFC_4122)) {
return false;
}

$uuid = self::transformToRfc4122($uuid, $format);

Comment on lines +131 to +136
Copy link
Member

Choose a reason for hiding this comment

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

Hi @alexandre-daubois sorry to dig this up, but while 7.2 is not release..

I find this early condition non "optimal", as default case will be in fact $format = FORMAT_RFC_4122..

What if we did something like this here (before? instead?), avoiding the regexp if the string is not 36 chars long ?

    if (($format & self::FORMAT_RFC_4122) && 36 !== \strlen($uuid)) {
        return false;
    }

What do you think ?

Copy link
Member

Choose a reason for hiding this comment

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

(before line 131, i messed up my selection 😅 )

Copy link
Member Author

Choose a reason for hiding this comment

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

I did try a few things but it would require some refactoring because of how the control flow is currently. I'm not sure it would worth it and if performances would greatly improve 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Yep that is very true!

if (self::NIL === $uuid && \in_array(static::class, [__CLASS__, NilUuid::class], true)) {
return true;
}
Expand Down Expand Up @@ -182,4 +184,36 @@

return substr_replace($uuid, '-', 23, 0);
}

/**
* Transforms a binary string, a base-32 string or a base-58 string to a RFC4122 string.
*
* @param int-mask-of<Uuid::FORMAT_*> $format
*
* @return non-empty-string
Copy link
Member

Choose a reason for hiding this comment

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

@alexandre-daubois this is currently not true, afaik ... should we enforce it (or remove it) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, but more than that, it's a bit confusing. I proposed this update: #58089

*/
private static function transformToRfc4122(string $uuid, int $format): string
{
$fromBase58 = false;
if (22 === \strlen($uuid) && 22 === strspn($uuid, BinaryUtil::BASE58['']) && $format & self::FORMAT_BASE_58) {
$uuid = str_pad(BinaryUtil::fromBase($uuid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);
$fromBase58 = true;
}

// base-58 are always transformed to binary string, but they must only be valid when the format is FORMAT_BASE_58
if (16 === \strlen($uuid) && $format & self::FORMAT_BINARY || $fromBase58 && $format & self::FORMAT_BASE_58) {
// don't use uuid_unparse(), it's slower
$uuid = bin2hex($uuid);
$uuid = substr_replace($uuid, '-', 8, 0);
$uuid = substr_replace($uuid, '-', 13, 0);
$uuid = substr_replace($uuid, '-', 18, 0);
$uuid = substr_replace($uuid, '-', 23, 0);
} elseif (26 === \strlen($uuid) && Ulid::isValid($uuid) && $format & self::FORMAT_BASE_32) {
$ulid = new NilUlid();
$ulid->uid = strtoupper($uuid);
$uuid = $ulid->toRfc4122();
}

return $uuid;
}
}
Loading