From 37d02ef160be79efe22f7654fee811c52afc9e71 Mon Sep 17 00:00:00 2001 From: Chad Parry Date: Thu, 26 Dec 2024 11:26:45 -0800 Subject: [PATCH] Fix #124: Manage entropy bits in TimeBasedEpochGenerator --- .../uuid/impl/TimeBasedEpochGenerator.java | 61 ++++++++------ .../impl/TimeBasedEpochGeneratorTest.java | 84 +++++++++++++++++++ 2 files changed, 120 insertions(+), 25 deletions(-) create mode 100644 src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java diff --git a/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java b/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java index 42157ce..aa67239 100644 --- a/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java +++ b/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java @@ -3,8 +3,7 @@ import java.security.SecureRandom; import java.util.Random; import java.util.UUID; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Consumer; import com.fasterxml.uuid.NoArgGenerator; import com.fasterxml.uuid.UUIDClock; @@ -35,9 +34,9 @@ public class TimeBasedEpochGenerator extends NoArgGenerator */ /** - * Random number generator that this generator uses. + * Random number generator that fills a byte array with entropy */ - protected final Random _random; + protected final Consumer _randomNextBytes; /** * Underlying {@link UUIDClock} used for accessing current time, to use for @@ -49,7 +48,6 @@ public class TimeBasedEpochGenerator extends NoArgGenerator private long _lastTimestamp = -1; private final byte[] _lastEntropy = new byte[ENTROPY_BYTE_LENGTH]; - private final Lock lock = new ReentrantLock(); /* /********************************************************************** @@ -76,10 +74,12 @@ public TimeBasedEpochGenerator(Random rnd) { */ public TimeBasedEpochGenerator(Random rnd, UUIDClock clock) { - if (rnd == null) { - rnd = LazyRandom.sharedSecureRandom(); - } - _random = rnd; + this((rnd == null ? LazyRandom.sharedSecureRandom() : rnd)::nextBytes, clock); + } + + TimeBasedEpochGenerator(Consumer randomNextBytes, UUIDClock clock) + { + _randomNextBytes = randomNextBytes; _clock = clock; } @@ -120,28 +120,39 @@ public UUID generate() */ public UUID construct(long rawTimestamp) { - lock.lock(); - try { + final long mostSigBits, leastSigBits; + synchronized (_lastEntropy) { if (rawTimestamp == _lastTimestamp) { - boolean c = true; - for (int i = ENTROPY_BYTE_LENGTH - 1; i >= 0; i--) { - if (c) { - byte temp = _lastEntropy[i]; - temp = (byte) (temp + 0x01); - c = _lastEntropy[i] == (byte) 0xff; - _lastEntropy[i] = temp; + carry: + { + for (int i = ENTROPY_BYTE_LENGTH - 1; i > 0; i--) { + _lastEntropy[i] = (byte) (_lastEntropy[i] + 1); + if (_lastEntropy[i] != 0x00) { + break carry; + } + } + _lastEntropy[0] = (byte) (_lastEntropy[0] + 1); + if (_lastEntropy[0] >= 0x04) { + throw new IllegalStateException("overflow on same millisecond"); } - } - if (c) { - throw new IllegalStateException("overflow on same millisecond"); } } else { _lastTimestamp = rawTimestamp; - _random.nextBytes(_lastEntropy); + _randomNextBytes.accept(_lastEntropy); + // In the most significant byte, only 2 bits will fit in the UUID, and one of those should be cleared + // to guard against overflow. + _lastEntropy[0] &= 0x01; } - return UUIDUtil.constructUUID(UUIDType.TIME_BASED_EPOCH, (rawTimestamp << 16) | _toShort(_lastEntropy, 0), _toLong(_lastEntropy, 2)); - } finally { - lock.unlock(); + mostSigBits = rawTimestamp << 16 | + (long) UUIDType.TIME_BASED_EPOCH.raw() << 12 | + Byte.toUnsignedLong(_lastEntropy[0]) << 10 | + Byte.toUnsignedLong(_lastEntropy[1]) << 2 | + Byte.toUnsignedLong(_lastEntropy[2]) >>> 6; + long right62Mask = (1L << 62) - 1; + long variant = 0x02; + leastSigBits = variant << 62 | + _toLong(_lastEntropy, 2) & right62Mask; } + return new UUID(mostSigBits, leastSigBits); } } diff --git a/src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java b/src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java new file mode 100644 index 0000000..4a9388c --- /dev/null +++ b/src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java @@ -0,0 +1,84 @@ +package com.fasterxml.uuid.impl; + +import com.fasterxml.uuid.UUIDClock; +import junit.framework.TestCase; + +import java.math.BigInteger; +import java.util.Arrays; +import java.util.UUID; +import java.util.function.Consumer; + +public class TimeBasedEpochGeneratorTest extends TestCase { + + public void testFormat() { + BigInteger minEntropy = BigInteger.ZERO; + long minTimestamp = 0; + TimeBasedEpochGenerator generatorEmpty = new TimeBasedEpochGenerator(staticEntropy(minEntropy), staticClock(minTimestamp)); + UUID uuidEmpty = generatorEmpty.generate(); + assertEquals(0x07, uuidEmpty.version()); + assertEquals(0x02, uuidEmpty.variant()); + assertEquals(minTimestamp, getTimestamp(uuidEmpty)); + assertEquals(minEntropy, getEntropy(uuidEmpty)); + + Consumer entropyFull = bytes -> Arrays.fill(bytes, (byte) 0xFF); + long maxTimestamp = rightBitmask(48); + TimeBasedEpochGenerator generatorFull = new TimeBasedEpochGenerator(entropyFull, staticClock(maxTimestamp)); + UUID uuidFull = generatorFull.generate(); + assertEquals(0x07, uuidFull.version()); + assertEquals(0x02, uuidFull.variant()); + assertEquals(maxTimestamp, getTimestamp(uuidFull)); + assertEquals(BigInteger.ONE.shiftLeft(73).subtract(BigInteger.ONE), getEntropy(uuidFull)); + } + + public void testIncrement() { + TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(BigInteger.ZERO), staticClock(0)); + assertEquals(BigInteger.valueOf(0), getEntropy(generator.generate())); + assertEquals(BigInteger.valueOf(1), getEntropy(generator.generate())); + assertEquals(BigInteger.valueOf(2), getEntropy(generator.generate())); + assertEquals(BigInteger.valueOf(3), getEntropy(generator.generate())); + } + + public void testCarryOnce() { + TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(BigInteger.valueOf(0xFF)), staticClock(0)); + assertEquals(BigInteger.valueOf(0xFF), getEntropy(generator.generate())); + assertEquals(BigInteger.valueOf(0x100), getEntropy(generator.generate())); + } + + public void testCarryAll() { + BigInteger largeEntropy = BigInteger.ONE.shiftLeft(73).subtract(BigInteger.ONE); + TimeBasedEpochGenerator generator = new TimeBasedEpochGenerator(staticEntropy(largeEntropy), staticClock(0)); + assertEquals(largeEntropy, getEntropy(generator.generate())); + assertEquals(BigInteger.ONE.shiftLeft(73), getEntropy(generator.generate())); + } + + private long getTimestamp(UUID uuid) { + return uuid.getMostSignificantBits() >>> 16; + } + + private BigInteger getEntropy(UUID uuid) { + return BigInteger.valueOf(uuid.getMostSignificantBits() & rightBitmask(12)).shiftLeft(62).or( + BigInteger.valueOf(uuid.getLeastSignificantBits() & rightBitmask(62))); + } + + private Consumer staticEntropy(BigInteger entropy) { + byte[] entropyBytes = entropy.toByteArray(); + return bytes -> { + int offset = bytes.length - entropyBytes.length; + Arrays.fill(bytes, 0, offset, (byte) 0x00); + System.arraycopy(entropyBytes, 0, bytes, offset, entropyBytes.length); + }; + } + + private UUIDClock staticClock(long timestamp) { + return new UUIDClock() { + @Override + public long currentTimeMillis() { + return timestamp; + } + }; + } + + private long rightBitmask(int bits) { + return (1L << bits) - 1; + } +}