rdbms: make getCPInfoFromCookieValue() stricter about allowed values

All components, not just the write index, must now be present.

Bug: T194403
Change-Id: I279ba3e16d470aca09fdb74cec91d28efb5e2f95

diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index 34436fb..130a097 100644 (file)
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -650,14 +650,14 @@ abstract class LBFactory implements ILBFactory {
 
        /**
         * @param string $value Possible result of LBFactory::makeCookieValueFromCPIndex()
-        * @param int $minTimestamp Lowest UNIX timestamp of non-expired values (if present)
+        * @param int $minTimestamp Lowest UNIX timestamp that a non-expired value can have
         * @return array (index: int or null, clientId: string or null)
         * @since 1.32
         */
        public static function getCPInfoFromCookieValue( $value, $minTimestamp ) {
                static $placeholder = [ 'index' => null, 'clientId' => null ];
 
-               if ( !preg_match( '/^(\d+)(?:@(\d+))?(?:#([0-9a-f]{32}))?$/', $value, $m ) ) {
+               if ( !preg_match( '/^(\d+)@(\d+)#([0-9a-f]{32})$/', $value, $m ) ) {
                        return $placeholder; // invalid
                }
 

diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index fac3486..82ca66a 100644 (file)
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -630,35 +630,41 @@ class LBFactoryTest extends MediaWikiTestCase {
                        '1@542#c47dcfb0566e7d7bc110a6128a45c93a',
                        LBFactory::makeCookieValueFromCPIndex( 1, 542, $agentId )
                );
+
                $this->assertSame(
-                       5,
-                       LBFactory::getCPInfoFromCookieValue( "5", $time - 10 )['index'],
+                       null,
+                       LBFactory::getCPInfoFromCookieValue( "5#$agentId", $time - 10 )['index'],
                        'No time set'
                );
                $this->assertSame(
                        null,
-                       LBFactory::getCPInfoFromCookieValue( "0", $time - 10 )['index'],
+                       LBFactory::getCPInfoFromCookieValue( "5@$time", $time - 10 )['index'],
+                       'No agent set'
+               );
+               $this->assertSame(
+                       null,
+                       LBFactory::getCPInfoFromCookieValue( "0@$time#$agentId", $time - 10 )['index'],
                        'Bad index'
                );
 
                $this->assertSame(
                        2,
-                       LBFactory::getCPInfoFromCookieValue( "2@$time", $time - 10 )['index'],
+                       LBFactory::getCPInfoFromCookieValue( "2@$time#$agentId", $time - 10 )['index'],
                        'Fresh'
                );
                $this->assertSame(
                        2,
-                       LBFactory::getCPInfoFromCookieValue( "2@$time", $time + 9 - 10 )['index'],
+                       LBFactory::getCPInfoFromCookieValue( "2@$time#$agentId", $time + 9 - 10 )['index'],
                        'Almost stale'
                );
                $this->assertSame(
                        null,
-                       LBFactory::getCPInfoFromCookieValue( "0@$time", $time + 9 - 10 )['index'],
+                       LBFactory::getCPInfoFromCookieValue( "0@$time#$agentId", $time + 9 - 10 )['index'],
                        'Almost stale; bad index'
                );
                $this->assertSame(
                        null,
-                       LBFactory::getCPInfoFromCookieValue( "2@$time", $time + 11 - 10 )['index'],
+                       LBFactory::getCPInfoFromCookieValue( "2@$time#$agentId", $time + 11 - 10 )['index'],
                        'Stale'
                );
 
@@ -669,7 +675,7 @@ class LBFactoryTest extends MediaWikiTestCase {
                );
                $this->assertSame(
                        null,
-                       LBFactory::getCPInfoFromCookieValue( "5@$time", $time + 11 - 10 )['clientId'],
+                       LBFactory::getCPInfoFromCookieValue( "5@$time#$agentId", $time + 11 - 10 )['clientId'],
                        'Stale (client ID)'
                );
        }