From 13ce73b9b83cbac1da89b7669ec8bcc2cfa51428 Mon Sep 17 00:00:00 2001 From: Sam Wilson Date: Tue, 20 Dec 2016 20:08:38 +0800 Subject: [PATCH] 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 --- includes/Block.php | 17 ++++++++--------- tests/phpunit/includes/user/UserTest.php | 24 ++++++++++++------------ 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/includes/Block.php b/includes/Block.php index 792bcd9320..20cb6145cc 100644 --- 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 7cbae2deb9..5d9cda7c44 100644 --- 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. -- 2.20.1