Skip to content

Commit d8e5ec4

Browse files
authored
[11.x] Use secure randomness in Arr::random() and Arr::shuffle() (#49642)
* Use Randomizer::pickArrayKeys() within Arr::random() * Use Randomizer::shuffleArray() in Arr::shuffle() * Remove unused $seed from shuffle * Fix failing shuffle tests
1 parent ca52cb3 commit d8e5ec4

File tree

6 files changed

+36
-42
lines changed

6 files changed

+36
-42
lines changed

src/Illuminate/Collections/Arr.php

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use ArrayAccess;
77
use Illuminate\Support\Traits\Macroable;
88
use InvalidArgumentException;
9+
use Random\Randomizer;
910

1011
class Arr
1112
{
@@ -661,24 +662,24 @@ public static function random($array, $number = null, $preserveKeys = false)
661662
);
662663
}
663664

664-
if (is_null($number)) {
665-
return $array[array_rand($array)];
665+
if (empty($array) || (! is_null($number) && $number <= 0)) {
666+
return is_null($number) ? null : [];
666667
}
667668

668-
if ((int) $number === 0) {
669-
return [];
670-
}
669+
$keys = (new Randomizer)->pickArrayKeys($array, $requested);
671670

672-
$keys = array_rand($array, $number);
671+
if (is_null($number)) {
672+
return $array[$keys[0]];
673+
}
673674

674675
$results = [];
675676

676677
if ($preserveKeys) {
677-
foreach ((array) $keys as $key) {
678+
foreach ($keys as $key) {
678679
$results[$key] = $array[$key];
679680
}
680681
} else {
681-
foreach ((array) $keys as $key) {
682+
foreach ($keys as $key) {
682683
$results[] = $array[$key];
683684
}
684685
}
@@ -730,20 +731,11 @@ public static function set(&$array, $key, $value)
730731
* Shuffle the given array and return the result.
731732
*
732733
* @param array $array
733-
* @param int|null $seed
734734
* @return array
735735
*/
736-
public static function shuffle($array, $seed = null)
736+
public static function shuffle($array)
737737
{
738-
if (is_null($seed)) {
739-
shuffle($array);
740-
} else {
741-
mt_srand($seed);
742-
shuffle($array);
743-
mt_srand();
744-
}
745-
746-
return $array;
738+
return (new Randomizer)->shuffleArray($array);
747739
}
748740

749741
/**

src/Illuminate/Collections/Collection.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,12 +1125,11 @@ public function shift($count = 1)
11251125
/**
11261126
* Shuffle the items in the collection.
11271127
*
1128-
* @param int|null $seed
11291128
* @return static
11301129
*/
1131-
public function shuffle($seed = null)
1130+
public function shuffle()
11321131
{
1133-
return new static(Arr::shuffle($this->items, $seed));
1132+
return new static(Arr::shuffle($this->items));
11341133
}
11351134

11361135
/**

src/Illuminate/Collections/Enumerable.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -889,10 +889,9 @@ public function search($value, $strict = false);
889889
/**
890890
* Shuffle the items in the collection.
891891
*
892-
* @param int|null $seed
893892
* @return static
894893
*/
895-
public function shuffle($seed = null);
894+
public function shuffle();
896895

897896
/**
898897
* Create chunks representing a "sliding window" view of the items in the collection.

src/Illuminate/Collections/LazyCollection.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,12 +1058,11 @@ public function search($value, $strict = false)
10581058
/**
10591059
* Shuffle the items in the collection.
10601060
*
1061-
* @param int|null $seed
10621061
* @return static
10631062
*/
1064-
public function shuffle($seed = null)
1063+
public function shuffle()
10651064
{
1066-
return $this->passthru('shuffle', func_get_args());
1065+
return $this->passthru('shuffle', []);
10671066
}
10681067

10691068
/**

tests/Support/SupportArrTest.php

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -907,14 +907,30 @@ public function testSet()
907907
$this->assertEquals([1 => 'hAz'], Arr::set($array, 1, 'hAz'));
908908
}
909909

910-
public function testShuffleWithSeed()
910+
public function testShuffle()
911911
{
912-
$this->assertEquals(
913-
Arr::shuffle(range(0, 100, 10), 1234),
914-
Arr::shuffle(range(0, 100, 10), 1234)
912+
$input = ['a', 'b', 'c', 'd', 'e', 'f'];
913+
914+
$this->assertNotEquals(
915+
$input,
916+
Arr::shuffle($input)
917+
);
918+
919+
$this->assertNotEquals(
920+
Arr::shuffle($input),
921+
Arr::shuffle($input)
915922
);
916923
}
917924

925+
public function testShuffleKeepsSameValues()
926+
{
927+
$input = ['a', 'b', 'c', 'd', 'e', 'f'];
928+
$shuffled = Arr::shuffle($input);
929+
sort($shuffled);
930+
931+
$this->assertEquals($input, $shuffled);
932+
}
933+
918934
public function testEmptyShuffle()
919935
{
920936
$this->assertEquals([], Arr::shuffle([]));

tests/Support/SupportCollectionTest.php

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -442,17 +442,6 @@ public function testCollectionIsConstructed($collection)
442442
$this->assertEmpty($data->all());
443443
}
444444

445-
#[DataProvider('collectionClassProvider')]
446-
public function testCollectionShuffleWithSeed($collection)
447-
{
448-
$data = new $collection(range(0, 100, 10));
449-
450-
$firstRandom = $data->shuffle(1234);
451-
$secondRandom = $data->shuffle(1234);
452-
453-
$this->assertEquals($firstRandom, $secondRandom);
454-
}
455-
456445
#[DataProvider('collectionClassProvider')]
457446
public function testSkipMethod($collection)
458447
{

0 commit comments

Comments
 (0)