bug #41801 [Uid] Fix fromString() with low base58 values (fancyweb)

nicolas-grekas · nicolas-grekas · commit 67c3ffaae54b · 2021-06-23T20:30:41.000+02:00
This PR was merged into the 5.2 branch. Discussion ---------- [Uid] Fix fromString() with low base58 values | Q | A | ------------- | --- | Branch? | 5.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - When converting base58 values to binary, we forgot to pad the result with null byte, so if the result is too "short", the code breaks. Commits ------- cd12921 [Uid] Fix fromString() with low base58 values
diff --git a/src/Symfony/Component/Uid/AbstractUid.php b/src/Symfony/Component/Uid/AbstractUid.php
@@ -43,15 +43,15 @@ abstract public static function fromString(string $uid): self;
     abstract public function toBinary(): string;
 
     /**
-     * Returns the identifier as a base-58 case sensitive string.
+     * Returns the identifier as a base58 case sensitive string.
      */
     public function toBase58(): string
     {
         return strtr(sprintf('%022s', BinaryUtil::toBase($this->toBinary(), BinaryUtil::BASE58)), '0', '1');
     }
 
     /**
-     * Returns the identifier as a base-32 case insensitive string.
+     * Returns the identifier as a base32 case insensitive string.
      */
     public function toBase32(): string
     {
diff --git a/src/Symfony/Component/Uid/Tests/UlidTest.php b/src/Symfony/Component/Uid/Tests/UlidTest.php
@@ -124,4 +124,9 @@ public function testFromStringOnExtendedClassReturnsStatic()
     {
         $this->assertInstanceOf(CustomUlid::class, CustomUlid::fromString((new CustomUlid())->toBinary()));
     }
+
+    public function testFromStringBase58Padding()
+    {
+        $this->assertInstanceOf(Ulid::class, Ulid::fromString('111111111u9QRyVM94rdmZ'));
+    }
 }
diff --git a/src/Symfony/Component/Uid/Tests/UuidTest.php b/src/Symfony/Component/Uid/Tests/UuidTest.php
@@ -187,14 +187,24 @@ public function testCompare()
         $this->assertSame([$a, $b, $c, $d], $uuids);
     }
 
-    public function testNilUuid()
+    /**
+     * @testWith    ["00000000-0000-0000-0000-000000000000"]
+     *              ["1111111111111111111111"]
+     *              ["00000000000000000000000000"]
+     */
+    public function testNilUuid(string $uuid)
     {
-        $uuid = Uuid::fromString('00000000-0000-0000-0000-000000000000');
+        $uuid = Uuid::fromString($uuid);
 
         $this->assertInstanceOf(NilUuid::class, $uuid);
         $this->assertSame('00000000-0000-0000-0000-000000000000', (string) $uuid);
     }
 
+    public function testNewNilUuid()
+    {
+        $this->assertSame('00000000-0000-0000-0000-000000000000', (string) new NilUuid());
+    }
+
     public function testFromStringOnExtendedClassReturnsStatic()
     {
         $this->assertInstanceOf(CustomUuid::class, CustomUuid::fromString(self::A_UUID_V4));
@@ -208,4 +218,9 @@ public function testGetTime()
         $this->assertSame(-0.0000001, (new UuidV1('13813fff-1dd2-11b2-a456-426655440000'))->getTime());
         $this->assertSame(-12219292800.0, ((new UuidV1('00000000-0000-1000-a456-426655440000'))->getTime()));
     }
+
+    public function testFromStringBase58Padding()
+    {
+        $this->assertInstanceOf(Uuid::class, Uuid::fromString('111111111u9QRyVM94rdmZ'));
+    }
 }
diff --git a/src/Symfony/Component/Uid/Ulid.php b/src/Symfony/Component/Uid/Ulid.php
@@ -61,7 +61,7 @@ public static function fromString(string $ulid): parent
         if (36 === \strlen($ulid) && Uuid::isValid($ulid)) {
             $ulid = (new Uuid($ulid))->toBinary();
         } elseif (22 === \strlen($ulid) && 22 === strspn($ulid, BinaryUtil::BASE58[''])) {
-            $ulid = BinaryUtil::fromBase($ulid, BinaryUtil::BASE58);
+            $ulid = str_pad(BinaryUtil::fromBase($ulid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);
         }
 
         if (16 !== \strlen($ulid)) {
diff --git a/src/Symfony/Component/Uid/Uuid.php b/src/Symfony/Component/Uid/Uuid.php
@@ -37,7 +37,7 @@ public function __construct(string $uuid)
     public static function fromString(string $uuid): parent
     {
         if (22 === \strlen($uuid) && 22 === strspn($uuid, BinaryUtil::BASE58[''])) {
-            $uuid = BinaryUtil::fromBase($uuid, BinaryUtil::BASE58);
+            $uuid = str_pad(BinaryUtil::fromBase($uuid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);
         }
 
         if (16 === \strlen($uuid)) {

Original file line number	Diff line number	Diff line change
`@@ -43,15 +43,15 @@ abstract public static function fromString(string $uid): self;`
`43`	`43`	`abstract public function toBinary(): string;`
`44`	`44`
`45`	`45`	`/**`
`46`		`- * Returns the identifier as a base-58 case sensitive string.`
	`46`	`+ * Returns the identifier as a base58 case sensitive string.`
`47`	`47`	`*/`
`48`	`48`	`public function toBase58(): string`
`49`	`49`	`{`
`50`	`50`	`return strtr(sprintf('%022s', BinaryUtil::toBase($this->toBinary(), BinaryUtil::BASE58)), '0', '1');`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`/**`
`54`		`- * Returns the identifier as a base-32 case insensitive string.`
	`54`	`+ * Returns the identifier as a base32 case insensitive string.`
`55`	`55`	`*/`
`56`	`56`	`public function toBase32(): string`
`57`	`57`	`{`
Original file line number	Diff line number	Diff line change
`@@ -124,4 +124,9 @@ public function testFromStringOnExtendedClassReturnsStatic()`
`124`	`124`	`{`
`125`	`125`	`$this->assertInstanceOf(CustomUlid::class, CustomUlid::fromString((new CustomUlid())->toBinary()));`
`126`	`126`	`}`
	`127`	`+`
	`128`	`+ public function testFromStringBase58Padding()`
	`129`	`+ {`
	`130`	`+ $this->assertInstanceOf(Ulid::class, Ulid::fromString('111111111u9QRyVM94rdmZ'));`
	`131`	`+ }`
`127`	`132`	`}`
Original file line number	Diff line number	Diff line change
`@@ -187,14 +187,24 @@ public function testCompare()`
`187`	`187`	`$this->assertSame([$a, $b, $c, $d], $uuids);`
`188`	`188`	`}`
`189`	`189`
`190`		`- public function testNilUuid()`
	`190`	`+ /**`
	`191`	`+ * @testWith ["00000000-0000-0000-0000-000000000000"]`
	`192`	`+ * ["1111111111111111111111"]`
	`193`	`+ * ["00000000000000000000000000"]`
	`194`	`+ */`
	`195`	`+ public function testNilUuid(string $uuid)`
`191`	`196`	`{`
`192`		`- $uuid = Uuid::fromString('00000000-0000-0000-0000-000000000000');`
	`197`	`+ $uuid = Uuid::fromString($uuid);`
`193`	`198`
`194`	`199`	`$this->assertInstanceOf(NilUuid::class, $uuid);`
`195`	`200`	`$this->assertSame('00000000-0000-0000-0000-000000000000', (string) $uuid);`
`196`	`201`	`}`
`197`	`202`
	`203`	`+ public function testNewNilUuid()`
	`204`	`+ {`
	`205`	`+ $this->assertSame('00000000-0000-0000-0000-000000000000', (string) new NilUuid());`
	`206`	`+ }`
	`207`	`+`
`198`	`208`	`public function testFromStringOnExtendedClassReturnsStatic()`
`199`	`209`	`{`
`200`	`210`	`$this->assertInstanceOf(CustomUuid::class, CustomUuid::fromString(self::A_UUID_V4));`
`@@ -208,4 +218,9 @@ public function testGetTime()`
`208`	`218`	`$this->assertSame(-0.0000001, (new UuidV1('13813fff-1dd2-11b2-a456-426655440000'))->getTime());`
`209`	`219`	`$this->assertSame(-12219292800.0, ((new UuidV1('00000000-0000-1000-a456-426655440000'))->getTime()));`
`210`	`220`	`}`
	`221`	`+`
	`222`	`+ public function testFromStringBase58Padding()`
	`223`	`+ {`
	`224`	`+ $this->assertInstanceOf(Uuid::class, Uuid::fromString('111111111u9QRyVM94rdmZ'));`
	`225`	`+ }`
`211`	`226`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ public static function fromString(string $ulid): parent`
`61`	`61`	`if (36 === \strlen($ulid) && Uuid::isValid($ulid)) {`
`62`	`62`	`$ulid = (new Uuid($ulid))->toBinary();`
`63`	`63`	`} elseif (22 === \strlen($ulid) && 22 === strspn($ulid, BinaryUtil::BASE58[''])) {`
`64`		`- $ulid = BinaryUtil::fromBase($ulid, BinaryUtil::BASE58);`
	`64`	`+ $ulid = str_pad(BinaryUtil::fromBase($ulid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);`
`65`	`65`	`}`
`66`	`66`
`67`	`67`	`if (16 !== \strlen($ulid)) {`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public function __construct(string $uuid)`
`37`	`37`	`public static function fromString(string $uuid): parent`
`38`	`38`	`{`
`39`	`39`	`if (22 === \strlen($uuid) && 22 === strspn($uuid, BinaryUtil::BASE58[''])) {`
`40`		`- $uuid = BinaryUtil::fromBase($uuid, BinaryUtil::BASE58);`
	`40`	`+ $uuid = str_pad(BinaryUtil::fromBase($uuid, BinaryUtil::BASE58), 16, "\0", \STR_PAD_LEFT);`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`if (16 === \strlen($uuid)) {`