diff --git a/release-notes/CREDITS b/release-notes/CREDITS index a4819df..1f220ba 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -151,3 +151,7 @@ Daniel Albuquerque (worldtiki@github) Alexander Ilinykh (divinenickname@github) * Contributed improvements to README.md, pom.xml (OSGi inclusion) [5.1.1] + +Chad Parry (chadparry@github) + * Contributed #124: TimeBasedEpochGenerator should prevent overflow + [5.3.0] diff --git a/release-notes/VERSION b/release-notes/VERSION index d8773c8..c3e3933 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -6,6 +6,8 @@ Releases 5.2.0 (not yet released) +#124: TimeBasedEpochGenerator should prevent overflow + (Chad P) - Update to `oss-parent` v69 5.1.1 (26-Sep-2025) diff --git a/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java b/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java index 42157ce..daceb9d 100644 --- a/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java +++ b/src/main/java/com/fasterxml/uuid/impl/TimeBasedEpochGenerator.java @@ -1,10 +1,10 @@ package com.fasterxml.uuid.impl; import java.security.SecureRandom; +import java.util.Objects; 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 +35,11 @@ public class TimeBasedEpochGenerator extends NoArgGenerator */ /** - * Random number generator that this generator uses. + * Source for random numbers used to fill a byte array with entropy. + * + * @since 5.3 (replaced earlier {@code java.util.Random _random}) */ - protected final Random _random; + protected final Consumer _randomNextBytes; /** * Underlying {@link UUIDClock} used for accessing current time, to use for @@ -49,7 +51,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 +77,21 @@ 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); + } + + /** + * + * @param randomNextBytes Source for random numbers to use for generating UUIDs. + * Note that it is strongly recommend to use a good (pseudo) random number source; + * for example, JDK's {@code SecureRandom::nextBytes}. + * @param clock clock Object used for accessing current time to use for generation + * + * @since 5.3 + */ + protected TimeBasedEpochGenerator(Consumer randomNextBytes, UUIDClock clock) + { + _randomNextBytes = Objects.requireNonNull(randomNextBytes); _clock = clock; } @@ -120,28 +132,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/UUIDComparatorTest.java b/src/test/java/com/fasterxml/uuid/UUIDComparatorTest.java index 3d79ec9..6103a66 100644 --- a/src/test/java/com/fasterxml/uuid/UUIDComparatorTest.java +++ b/src/test/java/com/fasterxml/uuid/UUIDComparatorTest.java @@ -22,7 +22,6 @@ import com.fasterxml.uuid.impl.TimeBasedEpochGenerator; -import com.fasterxml.uuid.impl.TimeBasedEpochRandomGenerator; import junit.framework.TestCase; public class UUIDComparatorTest 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..a889f21 --- /dev/null +++ b/src/test/java/com/fasterxml/uuid/impl/TimeBasedEpochGeneratorTest.java @@ -0,0 +1,88 @@ +package com.fasterxml.uuid.impl; + +import java.math.BigInteger; +import java.util.Arrays; +import java.util.UUID; +import java.util.function.Consumer; + +import com.fasterxml.uuid.UUIDClock; + +import junit.framework.TestCase; + +/** + * @since 5.3 + */ +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; + } +}