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

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.

Bug: T222381
Change-Id: Ic585f972d61da136744d080df13d8eb1ecd04cf5

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index 0329bd1..e07277d 100644 (file)
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -1739,7 +1739,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() );
@@ -1793,7 +1794,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() );
@@ -1931,14 +1933,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 cdbbcc5..99f4022 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 481da75..a8989fb 100644 (file)
--- a/tests/phpunit/includes/user/UserTest.php
+++ b/tests/phpunit/includes/user/UserTest.php
@@ -1201,6 +1201,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 );
@@ -1212,6 +1221,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 ) {
+               }
        }
 
        /**