Merge "Force user id and actor id to 0 when loading from remote wikis"

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index bdaac7f..ea4cf88 100644 (file)
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -1740,7 +1740,8 @@ class RevisionStore
                        $user = User::newFromAnyId(
                                $row->ar_user ?? null,
                                $row->ar_user_text ?? null,
-                               $row->ar_actor ?? null
+                               $row->ar_actor ?? null,
+                               $this->wikiId
                        );
                } catch ( InvalidArgumentException $ex ) {
                        wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() );
@@ -1794,7 +1795,8 @@ class RevisionStore
                        $user = User::newFromAnyId(
                                $row->rev_user ?? null,
                                $row->rev_user_text ?? null,
-                               $row->rev_actor ?? null
+                               $row->rev_actor ?? null,
+                               $this->wikiId
                        );
                } catch ( InvalidArgumentException $ex ) {
                        wfWarn( __METHOD__ . ': ' . $title->getPrefixedDBkey() . ': ' . $ex->getMessage() );
@@ -1932,14 +1934,21 @@ class RevisionStore
                /** @var UserIdentity $user */
                $user = null;
 
-               if ( isset( $fields['user'] ) && ( $fields['user'] instanceof UserIdentity ) ) {
+               // If a user is passed in, use it if possible. We cannot use a user from a
+               // remote wiki with unsuppressed ids, due to issues described in T222212.
+               if ( isset( $fields['user'] ) &&
+                       ( $fields['user'] instanceof UserIdentity ) &&
+                       ( $this->wikiId === false ||
+                               ( !$fields['user']->getId() && !$fields['user']->getActorId() ) )
+               ) {
                        $user = $fields['user'];
                } else {
                        try {
                                $user = User::newFromAnyId(
                                        $fields['user'] ?? null,
                                        $fields['user_text'] ?? null,
-                                       $fields['actor'] ?? null
+                                       $fields['actor'] ?? null,
+                                       $this->wikiId
                                );
                        } catch ( InvalidArgumentException $ex ) {
                                $user = null;

diff --git a/includes/user/User.php b/includes/user/User.php
index d6e1081..57c5130 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -673,11 +673,20 @@ class User implements IDBAccessObject, UserIdentity {
         * @param int|null $userId User ID, if known
         * @param string|null $userName User name, if known
         * @param int|null $actorId Actor ID, if known
+        * @param bool|string $wikiId remote wiki to which the User/Actor ID applies, or false if none
         * @return User
         */
-       public static function newFromAnyId( $userId, $userName, $actorId ) {
+       public static function newFromAnyId( $userId, $userName, $actorId, $wikiId = false ) {
                global $wgActorTableSchemaMigrationStage;
 
+               // Stop-gap solution for the problem described in T222212.
+               // Force the User ID and Actor ID to zero for users loaded from the database
+               // of another wiki, to prevent subtle data corruption and confusing failure modes.
+               if ( $wikiId !== false ) {
+                       $userId = 0;
+                       $actorId = 0;
+               }
+
                $user = new User;
                $user->mFrom = 'defaults';
 

diff --git a/tests/phpunit/includes/user/UserTest.php b/tests/phpunit/includes/user/UserTest.php
index 02b2f3c..474decf 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1205,6 +1205,15 @@ class UserTest extends MediaWikiTestCase {
                $this->assertSame( 'Bogus', $test->getName() );
                $this->assertSame( 654321, $test->getActorId() );
 
+               // Loading remote user by name from remote wiki should succeed
+               $test = User::newFromAnyId( null, 'Bogus', null, 'foo' );
+               $this->assertSame( 0, $test->getId() );
+               $this->assertSame( 'Bogus', $test->getName() );
+               $this->assertSame( 0, $test->getActorId() );
+               $test = User::newFromAnyId( 123456, 'Bogus', 654321, 'foo' );
+               $this->assertSame( 0, $test->getId() );
+               $this->assertSame( 0, $test->getActorId() );
+
                // Exceptional cases
                try {
                        User::newFromAnyId( null, null, null );
@@ -1216,6 +1225,13 @@ class UserTest extends MediaWikiTestCase {
                        $this->fail( 'Expected exception not thrown' );
                } catch ( InvalidArgumentException $ex ) {
                }
+
+               // Loading remote user by id from remote wiki should fail
+               try {
+                       User::newFromAnyId( 123456, null, 654321, 'foo' );
+                       $this->fail( 'Expected exception not thrown' );
+               } catch ( InvalidArgumentException $ex ) {
+               }
        }
 
        /**