From 504dc47656eedb46feebad0e78ea964d25b09dd2 Mon Sep 17 00:00:00 2001 From: Ioannis Igoumenos Date: Tue, 29 Sep 2026 12:52:44 +0300 Subject: [PATCH] Fix TypeError and bounds calculation for random collision mode in assignCollisionNumber --- .../src/Model/Table/FormatAssignersTable.php | 59 +++++++++++-------- 1 file changed, 36 insertions(+), 23 deletions(-) diff --git a/app/plugins/CoreAssigner/src/Model/Table/FormatAssignersTable.php b/app/plugins/CoreAssigner/src/Model/Table/FormatAssignersTable.php index a541c0dfc..4266a8eec 100644 --- a/app/plugins/CoreAssigner/src/Model/Table/FormatAssignersTable.php +++ b/app/plugins/CoreAssigner/src/Model/Table/FormatAssignersTable.php @@ -33,6 +33,8 @@ use Cake\ORM\Table; use Cake\Validation\Validator; use CoreAssigner\Lib\Enum\CollisionModeEnum; +use InvalidArgumentException; +use Random\RandomException; class FormatAssignersTable extends Table { use \App\Lib\Traits\AutoViewVarsTrait; @@ -194,11 +196,11 @@ public function assign($ia, $entity): string { * @since COmanage Registry v5.0.0 * @param int $formatAssignerId Format Assigner ID * @param string $sequenced Sequenced string as returned by selectSequences() - * @param CollisionModeEnum $collisionMode Collision number assignment mode + * @param string $collisionMode Collision number assignment mode * @param int $min Minimum number to assign - * @param int $max Maximum number to assign (for Random mode only) + * @param int|null $max Maximum number to assign (for Random mode only) * @return string Candidate string, possibly with a collision number assigned - * @throws InvalidArgumentException + * @throws InvalidArgumentException|RandomException */ protected function assignCollisionNumber( @@ -218,25 +220,36 @@ protected function assignCollisionNumber( switch($collisionMode) { case CollisionModeEnum::Random: // Simply pick a number between $min and $max. + $lmax = mt_getrandmax(); - $lmax = $max; - - if(!$max) { - // We have to be a bit careful with min and max vs mt_rand(). substituteParameters() + if(!empty($max) && $max < $lmax) { + // Smaller configured Maximum + $lmax = $max; + } + + if(str_contains($sequenced, '%s')) { + // Unbounded collision placeholder + return sprintf($sequenced, random_int($min, $lmax)); + } elseif(preg_match('/\%[0-9.]+s/', $sequenced, $matches)) { + // We have to be a bit careful with min and max. substituteParameters() // will generate something like (%05.5s). If no explicit $max is configured by the - // admin, we used mt_getrandmax. However, that could generate a string like 172500398. - // We take the first (eg) 5 digits, which are "17250". If $min is 20000, we'll - // incorrectly assign a collision number outside the permitted range (CO-1933). - + // admin, we default to mt_getrandmax. However, that could generate a string like 172500398. + // When formatted with width truncation, taking the first (eg) 5 digits ("17250") + // could result in a collision number outside the permitted range if $min is 20000 (CO-1933). + // Pull the width out of the string - $width = (int)rtrim(ltrim(strstr($matches[0], '.'), "."), "s"); - - // And calculate a new max - $lmax = (10 ** $width) - 1; - } + $width = (int)rtrim(ltrim(strstr($matches[0], '.'), '.'), 's'); + $wmax = (10 ** $width) - 1; - $n = random_int($min, $lmax); - return sprintf($sequenced, $n); + if($wmax < $lmax) { + // Smaller implicit maximum from width + $lmax = $wmax; + } + + return sprintf($sequenced, random_int($min, $lmax)); + } else { + return $sequenced; + } break; case CollisionModeEnum::Sequential: return sprintf($sequenced, $this->FormatAssignerSequences->next( @@ -245,7 +258,7 @@ protected function assignCollisionNumber( start: $min)); break; default: - throw new InvalidArgumentException(__d('error', 'unknown', $algorithm)); + throw new InvalidArgumentException(__d('error', 'unknown', $collisionMode)); break; } } else { @@ -261,7 +274,7 @@ protected function assignCollisionNumber( * @since COmanage Registry v5.0.0 * @param string $base Base string as returned by substituteParameters * @param int $iteration Iteration number (between 0 and 9) - * @param PermittedCharactersEnum $permitted Acceptable characters for substituted parameters + * @param string $permitted Acceptable characters for substituted parameters * @return string Format with sequenced segments selected */ @@ -273,7 +286,7 @@ protected function selectSequences( $sequenced = ""; // Loop through the string - for($j = 0;$j < strlen($base);$j++) { + for($j = 0, $jMax = strlen($base); $j < $jMax; $j++) { switch($base[$j]) { case '\\': // Copy the next character directly @@ -371,7 +384,7 @@ protected function substituteParameters( ); // Loop through the format string - for($i = 0;$i < strlen($format);$i++) { + for($i = 0, $iMax = strlen($format); $i < $iMax; $i++) { switch($format[$i]) { case '\\': // Copy the next character directly @@ -391,7 +404,7 @@ protected function substituteParameters( // Check if the next character is a width specifier if($format[$i+1] == ':') { // Don't advance $i yet since we still need it, so use $j instead - for($j = $i+2;$j < strlen($format);$j++) { + for($j = $i+2, $jMax = strlen($format); $j < $jMax; $j++) { if($format[$j] != ')') { $width .= $format[$j]; } else {