Default block-cookies to 24 hours only

Rather than use wgCookieExpiration as the basis for the maximum
life of a block cookie, just use 1 day.

Tests have been updated also.

Bug: T153347
Change-Id: I3447d97af3170308834f365c5c600430f47c66a7

diff --git a/includes/Block.php b/includes/Block.php
index 792bcd9..20cb614 100644 (file)
--- a/includes/Block.php
+++ b/includes/Block.php
@@ -1446,8 +1446,7 @@ class Block {
 
        /**
         * Set the 'BlockID' cookie to this block's ID and expiry time. The cookie's expiry will be
-        * the same as the block's, unless it's greater than $wgCookieExpiration in which case
-        * $wgCookieExpiration will be used instead (defaults to 30 days).
+        * the same as the block's, to a maximum of 24 hours.
         *
         * An empty value can also be set, in order to retain the cookie but remove the block ID
         * (e.g. as used in User::getBlockedStatus).
@@ -1457,18 +1456,18 @@ class Block {
         */
        public function setCookie( WebResponse $response, $setEmpty = false ) {
                // Calculate the default expiry time.
-               $config = RequestContext::getMain()->getConfig();
-               $defaultExpiry = wfTimestamp() + $config->get( 'CookieExpiration' );
+               $maxExpiryTime = wfTimestamp( TS_MW, wfTimestamp() + ( 24 * 60 * 60 ) );
 
                // Use the Block's expiry time only if it's less than the default.
-               $expiry = wfTimestamp( TS_UNIX, $this->getExpiry() );
-               if ( $expiry > $defaultExpiry ) {
-                       // The *default* default expiry is 30 days.
-                       $expiry = $defaultExpiry;
+               $expiryTime = $this->getExpiry();
+               if ( $expiryTime === 'infinity' || $expiryTime > $maxExpiryTime ) {
+                       $expiryTime = $maxExpiryTime;
                }
 
+               // Set the cookie. Reformat the MediaWiki datetime as a Unix timestamp for the cookie.
                $cookieValue = $setEmpty ? '' : $this->getId();
-               $response->setCookie( 'BlockID', $cookieValue, $expiry );
+               $expiryValue = DateTime::createFromFormat( "YmdHis", $expiryTime );
+               $response->setCookie( 'BlockID', $cookieValue, $expiryValue->format( "U" ) );
        }
 
        /**

diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 7cbae2d..5d9cda7 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -603,10 +603,10 @@ class UserTest extends MediaWikiTestCase {
                $user1tmp = $this->getTestUser()->getUser();
                $request1 = new FauxRequest();
                $request1->getSession()->setUser( $user1tmp );
-               $expiryFiveDays = time() + ( 5 * 24 * 60 * 60 );
+               $expiryFiveHours = wfTimestamp() + ( 5 * 60 * 60 );
                $block = new Block( [
                        'enableAutoblock' => true,
-                       'expiry' => wfTimestamp( TS_MW, $expiryFiveDays ),
+                       'expiry' => wfTimestamp( TS_MW, $expiryFiveHours ),
                ] );
                $block->setTarget( $user1tmp );
                $block->insert();
@@ -625,7 +625,7 @@ class UserTest extends MediaWikiTestCase {
                $cookies = $request1->response()->getCookies();
                $this->assertArrayHasKey( 'wmsitetitleBlockID', $cookies );
                $this->assertEquals( $block->getId(), $cookies['wmsitetitleBlockID']['value'] );
-               $this->assertEquals( $expiryFiveDays, $cookies['wmsitetitleBlockID']['expire'] );
+               $this->assertEquals( $expiryFiveHours, $cookies['wmsitetitleBlockID']['expire'] );
 
                // 2. Create a new request, set the cookies, and see if the (anon) user is blocked.
                $request2 = new FauxRequest();
@@ -696,14 +696,12 @@ class UserTest extends MediaWikiTestCase {
 
        /**
         * When a user is autoblocked and a cookie is set to track them, the expiry time of the cookie
-        * should match the block's expiry. If the block is infinite, the cookie expiry time should
-        * match $wgCookieExpiration. If the expiry time is changed, the cookie's should change with it.
+        * should match the block's expiry, to a maximum of 24 hours. If the expiry time is changed,
+        * the cookie's should change with it.
         */
        public function testAutoblockCookieInfiniteExpiry() {
-               $cookieExpiration = 20 * 24 * 60 * 60; // 20 days
                $this->setMwGlobals( [
                        'wgCookieSetOnAutoblock' => true,
-                       'wgCookieExpiration' => $cookieExpiration,
                        'wgCookiePrefix' => 'wm_infinite_block',
                ] );
                // 1. Log in a test user, and block them indefinitely.
@@ -724,20 +722,21 @@ class UserTest extends MediaWikiTestCase {
                $this->assertTrue( $block->isAutoblocking() );
                $this->assertGreaterThanOrEqual( 1, $user1->getBlockId() );
                $cookies = $request1->response()->getCookies();
-               // Calculate the expected cookie expiry date.
+               // Test the cookie's expiry to the nearest minute.
                $this->assertArrayHasKey( 'wm_infinite_blockBlockID', $cookies );
+               $expOneDay = wfTimestamp() + ( 24 * 60 * 60 );
                // Check for expiry dates in a 10-second window, to account for slow testing.
                $this->assertGreaterThan(
-                       time() + $cookieExpiration - 5,
+                       $expOneDay - 5,
                        $cookies['wm_infinite_blockBlockID']['expire']
                );
                $this->assertLessThan(
-                       time() + $cookieExpiration + 5,
+                       $expOneDay + 5,
                        $cookies['wm_infinite_blockBlockID']['expire']
                );
 
-               // 3. Change the block's expiry (to 2 days), and the cookie's should be changed also.
-               $newExpiry = time() + 2 * 24 * 60 * 60;
+               // 3. Change the block's expiry (to 2 hours), and the cookie's should be changed also.
+               $newExpiry = wfTimestamp() + 2 * 60 * 60;
                $block->mExpiry = wfTimestamp( TS_MW, $newExpiry );
                $block->update();
                $user2tmp = $this->getTestUser()->getUser();
@@ -747,6 +746,7 @@ class UserTest extends MediaWikiTestCase {
                $user2->mBlock = $block;
                $user2->load();
                $cookies = $request2->response()->getCookies();
+               $this->assertEquals( wfTimestamp( TS_MW, $newExpiry ), $block->getExpiry() );
                $this->assertEquals( $newExpiry, $cookies['wm_infinite_blockBlockID']['expire'] );
 
                // Clean up.