From 19f01c79851cc69fa4a73e8046861755fdd431bb Mon Sep 17 00:00:00 2001 From: Ben Ramsey Date: Tue, 21 Jan 2020 16:38:07 -0600 Subject: [PATCH] No longer set variant/version bits inside the time generator --- CHANGELOG.md | 2 + src/BinaryUtils.php | 34 ++- src/Generator/DefaultTimeGenerator.php | 14 +- src/UuidFactory.php | 10 +- tests/BinaryUtilsTest.php | 236 +++++++++++++++++++ tests/Generator/DceSecurityGeneratorTest.php | 6 +- tests/Generator/DefaultTimeGeneratorTest.php | 57 +---- 7 files changed, 272 insertions(+), 87 deletions(-) create mode 100644 tests/BinaryUtilsTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index b512743..3809377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. from `array $fields` to `string $bytes`. Rather than accepting an array of hexadecimal strings as UUID fields, the `build()` method now expects a byte string. +* `Generator\DefaultTimeGenerator` no longer adds the variant and version bits + to the bytes it returns. These must be applied to the bytes afterwards. * Change methods in converter interfaces to accept and return string values instead of `mixed`; this simplifies the interface and makes it consistent: * `NumberConverterInterface::fromHex(string $hex): string` diff --git a/src/BinaryUtils.php b/src/BinaryUtils.php index 0c8608f..fb8ba9a 100644 --- a/src/BinaryUtils.php +++ b/src/BinaryUtils.php @@ -14,54 +14,50 @@ declare(strict_types=1); namespace Ramsey\Uuid; -use function is_int; - /** * Provides binary math utilities */ class BinaryUtils { /** - * Applies the RFC 4122 variant field to the `clock_seq_hi_and_reserved` field + * Applies the RFC 4122 variant field to the 16-bit clock sequence * * @link http://tools.ietf.org/html/rfc4122#section-4.1.1 RFC 4122, § 4.1.1: Variant * - * @param int $clockSeqHi The value of `clock_seq_hi_and_reserved` field - * before the RFC 4122 variant is applied + * @param int $clockSeq The 16-bit clock sequence value before the RFC 4122 + * variant is applied * - * @return int The high field of the clock sequence multiplexed with the variant + * @return int The 16-bit clock sequence multiplexed with the UUID variant * * @psalm-pure */ - public static function applyVariant(int $clockSeqHi): int + public static function applyVariant(int $clockSeq): int { - // Set the variant to RFC 4122. - $clockSeqHi = $clockSeqHi & 0x3f; - $clockSeqHi |= 0x80; + $clockSeq = $clockSeq & 0x3fff; + $clockSeq |= 0x8000; - return $clockSeqHi; + return $clockSeq; } /** - * Applies the RFC 4122 version number to the `time_hi_and_version` field + * Applies the RFC 4122 version number to the 16-bit `time_hi_and_version` field * * @link http://tools.ietf.org/html/rfc4122#section-4.1.3 RFC 4122, § 4.1.3: Version * - * @param string $timeHi The value of the `time_hi_and_version` field before - * the RFC 4122 version is applied + * @param int $timeHi The value of the 16-bit `time_hi_and_version` field + * before the RFC 4122 version is applied * @param int $version The RFC 4122 version to apply to the `time_hi` field * - * @return int The high field of the timestamp multiplexed with the version number + * @return int The 16-bit time_hi field of the timestamp multiplexed with + * the UUID version number * * @psalm-pure */ - public static function applyVersion(string $timeHi, int $version): int + public static function applyVersion(int $timeHi, int $version): int { - $timeHi = hexdec($timeHi) & 0x0fff; + $timeHi = $timeHi & 0x0fff; $timeHi |= $version << 12; - assert(is_int($timeHi)); - return $timeHi; } } diff --git a/src/Generator/DefaultTimeGenerator.php b/src/Generator/DefaultTimeGenerator.php index 74251bc..8ea3606 100644 --- a/src/Generator/DefaultTimeGenerator.php +++ b/src/Generator/DefaultTimeGenerator.php @@ -14,12 +14,12 @@ declare(strict_types=1); namespace Ramsey\Uuid\Generator; -use Ramsey\Uuid\BinaryUtils; use Ramsey\Uuid\Converter\TimeConverterInterface; use Ramsey\Uuid\Exception\InvalidArgumentException; use Ramsey\Uuid\Exception\RandomSourceException; use Ramsey\Uuid\Provider\NodeProviderInterface; use Ramsey\Uuid\Provider\TimeProviderInterface; +use Throwable; /** * DefaultTimeGenerator generates strings of binary data based on a node ID, @@ -66,7 +66,7 @@ class DefaultTimeGenerator implements TimeGeneratorInterface try { // This does not use "stable storage"; see RFC 4122, Section 4.2.1.1. $clockSeq = random_int(0, 0x3fff); - } catch (\Throwable $exception) { + } catch (Throwable $exception) { throw new RandomSourceException( $exception->getMessage(), (int) $exception->getCode(), @@ -82,17 +82,13 @@ class DefaultTimeGenerator implements TimeGeneratorInterface $this->timeProvider->getTime()->getMicroSeconds()->toString() ); - $timeHi = BinaryUtils::applyVersion((string) ($uuidTime['hi'] ?? 0), 1); - $clockSeqHi = BinaryUtils::applyVariant($clockSeq >> 8); - $hex = vsprintf( - '%08s%04s%04s%02s%02s%012s', + '%08s%04s%04s%04s%012s', [ $uuidTime['low'] ?? 0, $uuidTime['mid'] ?? 0, - sprintf('%04x', $timeHi), - sprintf('%02x', $clockSeqHi), - sprintf('%02x', $clockSeq & 0xff), + $uuidTime['hi'] ?? 0, + sprintf('%04x', $clockSeq), $node, ] ); diff --git a/src/UuidFactory.php b/src/UuidFactory.php index 32ab1b4..b6f0a21 100644 --- a/src/UuidFactory.php +++ b/src/UuidFactory.php @@ -341,15 +341,17 @@ class UuidFactory implements UuidFactoryInterface */ private function uuidFromHashedName(string $hash, int $version): UuidInterface { - $timeHi = BinaryUtils::applyVersion(substr($hash, 12, 4), $version); - $clockSeqHi = BinaryUtils::applyVariant((int) hexdec(substr($hash, 16, 2))); + $timeHi = BinaryUtils::applyVersion((int) hexdec(substr($hash, 12, 4)), $version); + $clockSeq = BinaryUtils::applyVariant((int) hexdec(substr($hash, 16, 4))); + + $clockSeqHex = str_pad(dechex($clockSeq), 4, '0', STR_PAD_LEFT); $fields = [ 'time_low' => substr($hash, 0, 8), 'time_mid' => substr($hash, 8, 4), 'time_hi_and_version' => str_pad(dechex($timeHi), 4, '0', STR_PAD_LEFT), - 'clock_seq_hi_and_reserved' => str_pad(dechex($clockSeqHi), 2, '0', STR_PAD_LEFT), - 'clock_seq_low' => substr($hash, 18, 2), + 'clock_seq_hi_and_reserved' => substr($clockSeqHex, 0, 2), + 'clock_seq_low' => substr($clockSeqHex, 2), 'node' => substr($hash, 20, 12), ]; diff --git a/tests/BinaryUtilsTest.php b/tests/BinaryUtilsTest.php new file mode 100644 index 0000000..739c956 --- /dev/null +++ b/tests/BinaryUtilsTest.php @@ -0,0 +1,236 @@ +assertSame($expectedInt, BinaryUtils::applyVersion($timeHi, $version)); + $this->assertSame($expectedHex, dechex(BinaryUtils::applyVersion($timeHi, $version))); + } + + /** + * @dataProvider provideVariantTestValues + */ + public function testApplyVariant(int $clockSeq, int $expectedInt, string $expectedHex): void + { + $this->assertSame($expectedInt, BinaryUtils::applyVariant($clockSeq)); + $this->assertSame($expectedHex, dechex(BinaryUtils::applyVariant($clockSeq))); + } + + /** + * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingTraversableTypeHintSpecification + */ + public function provideVersionTestValues(): array + { + return [ + [ + 'timeHi' => 1001, + 'version' => 1, + 'expectedInt' => 5097, + 'expectedHex' => '13e9', + ], + [ + 'timeHi' => 1001, + 'version' => 2, + 'expectedInt' => 9193, + 'expectedHex' => '23e9', + ], + [ + 'timeHi' => 1001, + 'version' => 3, + 'expectedInt' => 13289, + 'expectedHex' => '33e9', + ], + [ + 'timeHi' => 1001, + 'version' => 4, + 'expectedInt' => 17385, + 'expectedHex' => '43e9', + ], + [ + 'timeHi' => 1001, + 'version' => 5, + 'expectedInt' => 21481, + 'expectedHex' => '53e9', + ], + [ + 'timeHi' => 65535, + 'version' => 1, + 'expectedInt' => 8191, + 'expectedHex' => '1fff', + ], + [ + 'timeHi' => 65535, + 'version' => 2, + 'expectedInt' => 12287, + 'expectedHex' => '2fff', + ], + [ + 'timeHi' => 65535, + 'version' => 3, + 'expectedInt' => 16383, + 'expectedHex' => '3fff', + ], + [ + 'timeHi' => 65535, + 'version' => 4, + 'expectedInt' => 20479, + 'expectedHex' => '4fff', + ], + [ + 'timeHi' => 65535, + 'version' => 5, + 'expectedInt' => 24575, + 'expectedHex' => '5fff', + ], + [ + 'timeHi' => 0, + 'version' => 1, + 'expectedInt' => 4096, + 'expectedHex' => '1000', + ], + [ + 'timeHi' => 0, + 'version' => 2, + 'expectedInt' => 8192, + 'expectedHex' => '2000', + ], + [ + 'timeHi' => 0, + 'version' => 3, + 'expectedInt' => 12288, + 'expectedHex' => '3000', + ], + [ + 'timeHi' => 0, + 'version' => 4, + 'expectedInt' => 16384, + 'expectedHex' => '4000', + ], + [ + 'timeHi' => 0, + 'version' => 5, + 'expectedInt' => 20480, + 'expectedHex' => '5000', + ], + ]; + } + + /** + * @phpcsSuppress SlevomatCodingStandard.TypeHints.ReturnTypeHint.MissingTraversableTypeHintSpecification + */ + public function provideVariantTestValues(): array + { + return [ + [ + 'clockSeq' => 0, + 'expectedInt' => 32768, + 'expectedHex' => '8000', + ], + [ + 'clockSeq' => 4096, + 'expectedInt' => 36864, + 'expectedHex' => '9000', + ], + [ + 'clockSeq' => 8192, + 'expectedInt' => 40960, + 'expectedHex' => 'a000', + ], + [ + 'clockSeq' => 12288, + 'expectedInt' => 45056, + 'expectedHex' => 'b000', + ], + [ + 'clockSeq' => 4095, + 'expectedInt' => 36863, + 'expectedHex' => '8fff', + ], + [ + 'clockSeq' => 8191, + 'expectedInt' => 40959, + 'expectedHex' => '9fff', + ], + [ + 'clockSeq' => 12287, + 'expectedInt' => 45055, + 'expectedHex' => 'afff', + ], + [ + 'clockSeq' => 16383, + 'expectedInt' => 49151, + 'expectedHex' => 'bfff', + ], + [ + 'clockSeq' => 16384, + 'expectedInt' => 32768, + 'expectedHex' => '8000', + ], + [ + 'clockSeq' => 20480, + 'expectedInt' => 36864, + 'expectedHex' => '9000', + ], + [ + 'clockSeq' => 24576, + 'expectedInt' => 40960, + 'expectedHex' => 'a000', + ], + [ + 'clockSeq' => 28672, + 'expectedInt' => 45056, + 'expectedHex' => 'b000', + ], + [ + 'clockSeq' => 32768, + 'expectedInt' => 32768, + 'expectedHex' => '8000', + ], + [ + 'clockSeq' => 36864, + 'expectedInt' => 36864, + 'expectedHex' => '9000', + ], + [ + 'clockSeq' => 40960, + 'expectedInt' => 40960, + 'expectedHex' => 'a000', + ], + [ + 'clockSeq' => 45056, + 'expectedInt' => 45056, + 'expectedHex' => 'b000', + ], + [ + 'clockSeq' => 36863, + 'expectedInt' => 36863, + 'expectedHex' => '8fff', + ], + [ + 'clockSeq' => 40959, + 'expectedInt' => 40959, + 'expectedHex' => '9fff', + ], + [ + 'clockSeq' => 45055, + 'expectedInt' => 45055, + 'expectedHex' => 'afff', + ], + [ + 'clockSeq' => 49151, + 'expectedInt' => 49151, + 'expectedHex' => 'bfff', + ], + ]; + } +} diff --git a/tests/Generator/DceSecurityGeneratorTest.php b/tests/Generator/DceSecurityGeneratorTest.php index d52ede1..751ef18 100644 --- a/tests/Generator/DceSecurityGeneratorTest.php +++ b/tests/Generator/DceSecurityGeneratorTest.php @@ -92,7 +92,7 @@ class DceSecurityGeneratorTest extends TestCase 'expectedId' => '000003e9', 'expectedDomain' => '00', 'expectedNode' => '001122334455', - 'expectedTimeMidHi' => '37f211ea', + 'expectedTimeMidHi' => '37f201ea', ], [ 'uid' => '1001', @@ -107,7 +107,7 @@ class DceSecurityGeneratorTest extends TestCase 'expectedId' => '000007d1', 'expectedDomain' => '01', 'expectedNode' => '001122334455', - 'expectedTimeMidHi' => '37f211ea', + 'expectedTimeMidHi' => '37f201ea', ], [ 'uid' => 0, @@ -122,7 +122,7 @@ class DceSecurityGeneratorTest extends TestCase 'expectedId' => 'ffffffff', 'expectedDomain' => '02', 'expectedNode' => '001122334455', - 'expectedTimeMidHi' => '37f211ea', + 'expectedTimeMidHi' => '37f201ea', ], ]; } diff --git a/tests/Generator/DefaultTimeGeneratorTest.php b/tests/Generator/DefaultTimeGeneratorTest.php index 4351fc2..d240d05 100644 --- a/tests/Generator/DefaultTimeGeneratorTest.php +++ b/tests/Generator/DefaultTimeGeneratorTest.php @@ -126,21 +126,17 @@ class DefaultTimeGeneratorTest extends TestCase * @runInSeparateProcess * @preserveGlobalState disabled */ - public function testGenerateAppliesVersionAndVariant(): void + public function testGenerateDoesNotApplyVersionAndVariant(): void { - $expectedBytes = hex2bin('83cb98e098e003cb8fe2122f80ca9e06'); + $expectedBytes = hex2bin('83cb98e098e003cb0fe2122f80ca9e06'); $this->timeConverter->method('calculateTime') ->with($this->currentTime['sec'], $this->currentTime['usec']) ->willReturn($this->calculatedTime); + $binaryUtils = Mockery::mock('alias:' . BinaryUtils::class); - $binaryUtils->shouldReceive('applyVersion') - ->with($this->calculatedTime['hi'], 1) - ->andReturn(971); - $clockSeqShifted = 15; - $binaryUtils->shouldReceive('applyVariant') - ->with($clockSeqShifted) - ->andReturn(143); + $binaryUtils->shouldNotReceive('applyVersion'); + $binaryUtils->shouldNotReceive('applyVariant'); $defaultTimeGenerator = new DefaultTimeGenerator( $this->nodeProvider, @@ -151,49 +147,6 @@ class DefaultTimeGeneratorTest extends TestCase $this->assertSame($expectedBytes, $defaultTimeGenerator->generate($this->nodeId, $this->clockSeq)); } - /** - * @runInSeparateProcess - * @preserveGlobalState disabled - */ - public function testGenerateReturnsBinaryStringInUuidFormat(): void - { - $this->timeConverter->method('calculateTime')->willReturn($this->calculatedTime); - $binaryUtils = Mockery::mock('alias:' . BinaryUtils::class); - $binaryUtils->shouldReceive('applyVersion')->andReturn(971); - $binaryUtils->shouldReceive('applyVariant')->andReturn(143); - - $defaultTimeGenerator = new DefaultTimeGenerator( - $this->nodeProvider, - $this->timeConverter, - $this->timeProvider - ); - $result = $defaultTimeGenerator->generate($this->nodeId, $this->clockSeq); - - // Given we use values: - // $low = '83cb98e0'; - // $mid = '98e0'; - // $timeHi = 971; - // $clockSeqHi = 143; - // $clockSeq = 4066; - // $node = '122f80ca9e06'; - // - // $values = [ - // $low, - // $mid, - // sprintf('%04x', $timeHi), - // sprintf('%02x', $clockSeqHi), - // sprintf('%02x', $clockSeq & 0xff), - // $node - // ]; - // - // then: - // $hex = vsprintf('%08s%04s%04s%02s%02s%012s', $values); - - $hex = '83cb98e098e003cb8fe2122f80ca9e06'; - $binary = hex2bin($hex); - $this->assertEquals($binary, $result); - } - /** * @runInSeparateProcess * @preserveGlobalState disabled