random: Fix off-by-one in fast path selection of Randomizer::getBytesFromString()

ext/random/randomizer.c

@@ -401,7 +401,7 @@ PHP_METHOD(Random_Randomizer, getBytesFromString)
 
 	retval = zend_string_alloc(length, 0);
 
-	if (source_length > 0xFF) {
+	if (source_length > 0x100) {
 		while (total_size < length) {
 			uint64_t offset = randomizer->algo->range(randomizer->status, 0, source_length - 1);
 

ext/random/tests/03_randomizer/methods/getBytesFromString_fast_path.phpt

@@ -0,0 +1,112 @@
+--TEST--
+Random: Randomizer: getBytesFromString(): Fast Path Masking
+--FILE--
+<?php
+
+use Random\Engine\Test\TestWrapperEngine;
+use Random\Engine\Xoshiro256StarStar;
+use Random\Randomizer;
+
+require __DIR__ . "/../../engines.inc";
+
+$allBytes = implode('', array_map(
+    fn ($byte) => chr($byte),
+    range(0x00, 0xff)
+));
+
+// Xoshiro256** is the fastest engine available.
+$xoshiro = new Xoshiro256StarStar();
+
+var_dump(strlen($allBytes));
+echo PHP_EOL;
+
+// Fast path: Inputs less than or equal to 256.
+for ($i = 1; $i <= strlen($allBytes); $i *= 2) {
+    echo "{$i}:", PHP_EOL;
+
+    $wrapper = new TestWrapperEngine($xoshiro);
+    $r = new Randomizer($wrapper);
+    $result = $r->getBytesFromString(substr($allBytes, 0, $i), 20000);
+
+    // Xoshiro256** is a 64 Bit engine and thus generates 8 bytes at once.
+    // For powers of two we expect no rejections and thus exactly
+    // 20000/8 = 2500 calls to the engine.
+    var_dump($wrapper->getCount());
+
+    $count = [];
+    for ($j = 0; $j < strlen($result); $j++) {
+        $b = $result[$j];
+        $count[ord($b)] ??= 0;
+        $count[ord($b)]++;
+    }
+
+    // We also expect that each possible value appears at least once, if
+    // not is is very likely that some bits were erroneously masked away.
+    var_dump(count($count));
+
+    echo PHP_EOL;
+}
+
+echo "Slow Path:", PHP_EOL;
+
+$wrapper = new TestWrapperEngine($xoshiro);
+$r = new Randomizer($wrapper);
+$result = $r->getBytesFromString($allBytes . $allBytes, 20000);
+
+// In the slow path we expect one call per byte, i.e. 20000
+var_dump($wrapper->getCount());
+
+$count = [];
+for ($j = 0; $j < strlen($result); $j++) {
+    $b = $result[$j];
+    $count[ord($b)] ??= 0;
+    $count[ord($b)]++;
+}
+
+// We also expect that each possible value appears at least once, if
+// not is is very likely that some bits were erroneously masked away.
+var_dump(count($count));
+
+?>
+--EXPECT--
+int(256)
+
+1:
+int(2500)
+int(1)
+
+2:
+int(2500)
+int(2)
+
+4:
+int(2500)
+int(4)
+
+8:
+int(2500)
+int(8)
+
+16:
+int(2500)
+int(16)
+
+32:
+int(2500)
+int(32)
+
+64:
+int(2500)
+int(64)
+
+128:
+int(2500)
+int(128)
+
+256:
+int(2500)
+int(256)
+
+Slow Path:
+int(20000)
+int(256)

ext/random/tests/engines.inc

@@ -27,14 +27,23 @@ final class TestShaEngine implements Engine
 
 final class TestWrapperEngine implements Engine
 {
+    private int $count = 0;
+
     public function __construct(private readonly Engine $engine)
     {
     }
 
     public function generate(): string
     {
+        $this->count++;
+
         return $this->engine->generate();
     }
+
+    public function getCount(): int
+    {
+        return $this->count;
+    }
 }
 
 final class TestXoshiro128PlusPlusEngine implements Engine