Merge "Special:NewFiles: Use a proper user widget instead"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 28 Aug 2019 20:57:14 +0000 (20:57 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 28 Aug 2019 20:57:14 +0000 (20:57 +0000)
includes/Permissions/PermissionManager.php
includes/specialpage/WantedQueryPage.php
includes/upload/UploadFromChunks.php
resources/Resources.php
resources/src/startup/mediawiki.js
tests/phpunit/includes/Permissions/PermissionManagerTest.php
tests/phpunit/includes/user/UserTest.php
tests/qunit/suites/resources/mediawiki/mediawiki.loader.test.js

index 43d57a7..f9ad3eb 100644 (file)
@@ -1244,11 +1244,12 @@ class PermissionManager {
         */
        public function getUserPermissions( UserIdentity $user ) {
                $user = User::newFromIdentity( $user );
-               if ( !isset( $this->usersRights[ $user->getId() ] ) ) {
-                       $this->usersRights[ $user->getId() ] = $this->getGroupPermissions(
+               $rightsCacheKey = $this->getRightsCacheKey( $user );
+               if ( !isset( $this->usersRights[ $rightsCacheKey ] ) ) {
+                       $this->usersRights[ $rightsCacheKey ] = $this->getGroupPermissions(
                                $user->getEffectiveGroups()
                        );
-                       Hooks::run( 'UserGetRights', [ $user, &$this->usersRights[ $user->getId() ] ] );
+                       Hooks::run( 'UserGetRights', [ $user, &$this->usersRights[ $rightsCacheKey ] ] );
 
                        // Deny any rights denied by the user's session, unless this
                        // endpoint has no sessions.
@@ -1256,17 +1257,17 @@ class PermissionManager {
                                // FIXME: $user->getRequest().. need to be replaced with something else
                                $allowedRights = $user->getRequest()->getSession()->getAllowedUserRights();
                                if ( $allowedRights !== null ) {
-                                       $this->usersRights[ $user->getId() ] = array_intersect(
-                                               $this->usersRights[ $user->getId() ],
+                                       $this->usersRights[ $rightsCacheKey ] = array_intersect(
+                                               $this->usersRights[ $rightsCacheKey ],
                                                $allowedRights
                                        );
                                }
                        }
 
-                       Hooks::run( 'UserGetRightsRemove', [ $user, &$this->usersRights[ $user->getId() ] ] );
+                       Hooks::run( 'UserGetRightsRemove', [ $user, &$this->usersRights[ $rightsCacheKey ] ] );
                        // Force reindexation of rights when a hook has unset one of them
-                       $this->usersRights[ $user->getId() ] = array_values(
-                               array_unique( $this->usersRights[ $user->getId() ] )
+                       $this->usersRights[ $rightsCacheKey ] = array_values(
+                               array_unique( $this->usersRights[ $rightsCacheKey ] )
                        );
 
                        if (
@@ -1275,13 +1276,13 @@ class PermissionManager {
                                $user->getBlock()
                        ) {
                                $anon = new User;
-                               $this->usersRights[ $user->getId() ] = array_intersect(
-                                       $this->usersRights[ $user->getId() ],
+                               $this->usersRights[ $rightsCacheKey ] = array_intersect(
+                                       $this->usersRights[ $rightsCacheKey ],
                                        $this->getUserPermissions( $anon )
                                );
                        }
                }
-               $rights = $this->usersRights[ $user->getId() ];
+               $rights = $this->usersRights[ $rightsCacheKey ];
                foreach ( $this->temporaryUserRights[ $user->getId() ] ?? [] as $overrides ) {
                        $rights = array_values( array_unique( array_merge( $rights, $overrides ) ) );
                }
@@ -1298,14 +1299,24 @@ class PermissionManager {
         */
        public function invalidateUsersRightsCache( $user = null ) {
                if ( $user !== null ) {
-                       if ( isset( $this->usersRights[ $user->getId() ] ) ) {
-                               unset( $this->usersRights[$user->getId()] );
+                       $rightsCacheKey = $this->getRightsCacheKey( $user );
+                       if ( isset( $this->usersRights[ $rightsCacheKey ] ) ) {
+                               unset( $this->usersRights[ $rightsCacheKey ] );
                        }
                } else {
                        $this->usersRights = null;
                }
        }
 
+       /**
+        * Gets a unique key for user rights cache.
+        * @param UserIdentity $user
+        * @return string
+        */
+       private function getRightsCacheKey( UserIdentity $user ) {
+               return $user->isRegistered() ? "u:{$user->getId()}" : "anon:{$user->getName()}";
+       }
+
        /**
         * Check, if the given group has the given permission
         *
@@ -1583,7 +1594,8 @@ class PermissionManager {
                if ( !defined( 'MW_PHPUNIT_TEST' ) ) {
                        throw new Exception( __METHOD__ . ' can not be called outside of tests' );
                }
-               $this->usersRights[ $user->getId() ] = is_array( $rights ) ? $rights : [ $rights ];
+               $this->usersRights[ $this->getRightsCacheKey( $user ) ] =
+                       is_array( $rights ) ? $rights : [ $rights ];
        }
 
 }
index 83ffe40..72fe57d 100644 (file)
@@ -116,7 +116,7 @@ abstract class WantedQueryPage extends QueryPage {
         * @param object $result Result row
         * @return string
         */
-       private function makeWlhLink( $title, $result ) {
+       protected function makeWlhLink( $title, $result ) {
                $wlh = SpecialPage::getTitleFor( 'Whatlinkshere', $title->getPrefixedText() );
                $label = $this->msg( 'nlinks' )->numParams( $result->value )->text();
                return $this->getLinkRenderer()->makeLink( $wlh, $label );
index 8c6b2f9..c28890b 100644 (file)
@@ -161,7 +161,7 @@ class UploadFromChunks extends UploadFromFile {
                $ext = FileBackend::extensionFromPath( $this->mVirtualTempPath );
                // Get a 0-byte temp file to perform the concatenation at
                $tmpFile = MediaWikiServices::getInstance()->getTempFSFileFactory()
-                       ->getTempFSFile( 'chunkedupload_', $ext );
+                       ->newTempFSFile( 'chunkedupload_', $ext );
                $tmpPath = false; // fail in concatenate()
                if ( $tmpFile ) {
                        // keep alive with $this
index 72fe627..8234e89 100644 (file)
@@ -1376,7 +1376,7 @@ return [
                        'oojs-ui-core',
                ],
                'messages' => [
-                       // Keep the uses message keys in sync with EditPage#setHeaders
+                       // Keep these message keys in sync with EditPage#setHeaders
                        'creating',
                        'editconflict',
                        'editing',
index a3249de..a4ee488 100644 (file)
                                        // Whether the store is in use on this page.
                                        enabled: null,
 
-                                       // Modules whose string representation exceeds 100 kB are
-                                       // ineligible for storage. See bug T66721.
-                                       MODULE_SIZE_MAX: 100 * 1000,
+                                       // Modules whose serialised form exceeds 100 kB won't be stored (T66721).
+                                       MODULE_SIZE_MAX: 1e5,
 
                                        // The contents of the store, mapping '[name]@[version]' keys
                                        // to module implementations.
                                         * @return {Object} Module store contents.
                                         */
                                        toJSON: function () {
-                                               return { items: mw.loader.store.items, vary: mw.loader.store.vary };
+                                               return {
+                                                       items: mw.loader.store.items,
+                                                       vary: mw.loader.store.vary,
+                                                       // Store with 1e7 ms accuracy (1e4 seconds, or ~ 2.7 hours),
+                                                       // which is enough for the purpose of expiring after ~ 30 days.
+                                                       asOf: Math.ceil( Date.now() / 1e7 )
+                                               };
                                        },
 
                                        /**
                                                        this.enabled = true;
                                                        // If null, JSON.parse() will cast to string and re-parse, still null.
                                                        data = JSON.parse( raw );
-                                                       if ( data && typeof data.items === 'object' && data.vary === this.vary ) {
+                                                       if ( data &&
+                                                               typeof data.items === 'object' &&
+                                                               data.vary === this.vary &&
+                                                               // Only use if it's been less than 30 days since the data was written
+                                                               // 30 days = 2,592,000 s = 2,592,000,000 ms = ± 259e7 ms
+                                                               Date.now() < ( data.asOf * 1e7 ) + 259e7
+                                                       ) {
+                                                               // The data is not corrupt, matches our vary context, and has not expired.
                                                                this.items = data.items;
                                                                return;
                                                        }
index 122f377..44b7f67 100644 (file)
@@ -1865,4 +1865,26 @@ class PermissionManagerTest extends MediaWikiLangTestCase {
                        ->getPermissionManager()
                        ->getNamespaceRestrictionLevels( $ns, $user ) );
        }
+
+       /**
+        * @covers \MediaWiki\Permissions\PermissionManager::getRightsCacheKey
+        * @throws \Exception
+        */
+       public function testAnonPermissionsNotClash() {
+               $user1 = User::newFromName( 'User1' );
+               $user2 = User::newFromName( 'User2' );
+               $pm = MediaWikiServices::getInstance()->getPermissionManager();
+               $pm->overrideUserRightsForTesting( $user2, [] );
+               $this->assertNotSame( $pm->getUserPermissions( $user1 ), $pm->getUserPermissions( $user2 ) );
+       }
+
+       /**
+        * @covers \MediaWiki\Permissions\PermissionManager::getRightsCacheKey
+        */
+       public function testAnonPermissionsNotClashOneRegistered() {
+               $user1 = User::newFromName( 'User1' );
+               $user2 = $this->getTestSysop()->getUser();
+               $pm = MediaWikiServices::getInstance()->getPermissionManager();
+               $this->assertNotSame( $pm->getUserPermissions( $user1 ), $pm->getUserPermissions( $user2 ) );
+       }
 }
index f48385d..9ff0521 100644 (file)
@@ -929,13 +929,8 @@ class UserTest extends MediaWikiTestCase {
                $this->assertFalse( $user->isPingLimitable() );
 
                $this->setMwGlobals( 'wgRateLimitsExcludedIPs', [] );
-               $noRateLimitUser = $this->getMockBuilder( User::class )->disableOriginalConstructor()
-                       ->setMethods( [ 'getIP', 'getId', 'getGroups' ] )->getMock();
-               $noRateLimitUser->expects( $this->any() )->method( 'getIP' )->willReturn( '1.2.3.4' );
-               $noRateLimitUser->expects( $this->any() )->method( 'getId' )->willReturn( 0 );
-               $noRateLimitUser->expects( $this->any() )->method( 'getGroups' )->willReturn( [] );
-               $this->overrideUserPermissions( $noRateLimitUser, 'noratelimit' );
-               $this->assertFalse( $noRateLimitUser->isPingLimitable() );
+               $this->overrideUserPermissions( $user, 'noratelimit' );
+               $this->assertFalse( $user->isPingLimitable() );
        }
 
        public function provideExperienceLevel() {
index 894dd19..3258f8e 100644 (file)
@@ -21,6 +21,9 @@
                                window.Set = this.nativeSet;
                                mw.redefineFallbacksForTest();
                        }
+                       if ( this.resetStoreKey ) {
+                               localStorage.removeItem( mw.loader.store.key );
+                       }
                        // Remove any remaining temporary statics
                        // exposed for cross-file mocks.
                        delete mw.loader.testCallback;
                } );
        } );
 
+       QUnit.test( 'mw.loader.store.init - Invalid JSON', function ( assert ) {
+               // Reset
+               this.sandbox.stub( mw.loader.store, 'enabled', null );
+               this.sandbox.stub( mw.loader.store, 'items', {} );
+               this.resetStoreKey = true;
+               localStorage.setItem( mw.loader.store.key, 'invalid' );
+
+               mw.loader.store.init();
+               assert.strictEqual( mw.loader.store.enabled, true, 'Enabled' );
+               assert.strictEqual(
+                       $.isEmptyObject( mw.loader.store.items ),
+                       true,
+                       'Items starts fresh'
+               );
+       } );
+
+       QUnit.test( 'mw.loader.store.init - Wrong JSON', function ( assert ) {
+               // Reset
+               this.sandbox.stub( mw.loader.store, 'enabled', null );
+               this.sandbox.stub( mw.loader.store, 'items', {} );
+               this.resetStoreKey = true;
+               localStorage.setItem( mw.loader.store.key, JSON.stringify( { wrong: true } ) );
+
+               mw.loader.store.init();
+               assert.strictEqual( mw.loader.store.enabled, true, 'Enabled' );
+               assert.strictEqual(
+                       $.isEmptyObject( mw.loader.store.items ),
+                       true,
+                       'Items starts fresh'
+               );
+       } );
+
+       QUnit.test( 'mw.loader.store.init - Expired JSON', function ( assert ) {
+               // Reset
+               this.sandbox.stub( mw.loader.store, 'enabled', null );
+               this.sandbox.stub( mw.loader.store, 'items', {} );
+               this.resetStoreKey = true;
+               localStorage.setItem( mw.loader.store.key, JSON.stringify( {
+                       items: { use: 'not me' },
+                       vary: mw.loader.store.vary,
+                       asOf: 130161 // 2011-04-01 12:00
+               } ) );
+
+               mw.loader.store.init();
+               assert.strictEqual( mw.loader.store.enabled, true, 'Enabled' );
+               assert.strictEqual(
+                       $.isEmptyObject( mw.loader.store.items ),
+                       true,
+                       'Items starts fresh'
+               );
+       } );
+
+       QUnit.test( 'mw.loader.store.init - Good JSON', function ( assert ) {
+               // Reset
+               this.sandbox.stub( mw.loader.store, 'enabled', null );
+               this.sandbox.stub( mw.loader.store, 'items', {} );
+               this.resetStoreKey = true;
+               localStorage.setItem( mw.loader.store.key, JSON.stringify( {
+                       items: { use: 'me' },
+                       vary: mw.loader.store.vary,
+                       asOf: Math.ceil( Date.now() / 1e7 ) - 5 // ~ 13 hours ago
+               } ) );
+
+               mw.loader.store.init();
+               assert.strictEqual( mw.loader.store.enabled, true, 'Enabled' );
+               assert.deepEqual(
+                       mw.loader.store.items,
+                       { use: 'me' },
+                       'Stored items are loaded'
+               );
+       } );
+
        QUnit.test( 'require()', function ( assert ) {
                mw.loader.register( [
                        [ 'test.require1', '0' ],