From 4589f4d78183fcfde2009e217ea8524102c95a31 Mon Sep 17 00:00:00 2001
From: daniel <daniel.kinzler@wikimedia.de>
Date: Thu, 11 Jan 2018 16:43:18 +0100
Subject: [PATCH] Make Revision::__construct work with bad page ID

For backwards-copatibility, we need to be able to construct a Revision
object even for bad page IDs.

Bug: T184689
Change-Id: I18c823d7b72504447982364d581b34e98924b67f
---
 includes/Revision.php                   | 49 ++++++++++++++++++++++++-
 includes/Storage/RevisionStore.php      |  4 +-
 tests/phpunit/includes/RevisionTest.php | 22 +++++++++++
 3 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/includes/Revision.php b/includes/Revision.php
index 0844e89286..8849697e0f 100644
--- a/includes/Revision.php
+++ b/includes/Revision.php
@@ -495,13 +495,13 @@ class Revision implements IDBAccessObject {
 			$this->mRecord = self::getRevisionStore()->newMutableRevisionFromArray(
 				$row,
 				$queryFlags,
-				$title
+				$this->ensureTitle( $row, $queryFlags, $title )
 			);
 		} elseif ( is_object( $row ) ) {
 			$this->mRecord = self::getRevisionStore()->newRevisionFromRow(
 				$row,
 				$queryFlags,
-				$title
+				$this->ensureTitle( $row, $queryFlags, $title )
 			);
 		} else {
 			throw new InvalidArgumentException(
@@ -510,6 +510,51 @@ class Revision implements IDBAccessObject {
 		}
 	}
 
+	/**
+	 * Make sure we have *some* Title object for use by the constructor.
+	 * For B/C, the constructor shouldn't fail even for a bad page ID or bad revision ID.
+	 *
+	 * @param array|object $row
+	 * @param int $queryFlags
+	 * @param Title|null $title
+	 *
+	 * @return Title $title if not null, or a Title constructed from information in $row.
+	 */
+	private function ensureTitle( $row, $queryFlags, $title = null ) {
+		if ( $title ) {
+			return $title;
+		}
+
+		if ( is_array( $row ) ) {
+			if ( isset( $row['title'] ) ) {
+				if ( !( $row['title'] instanceof Title ) ) {
+					throw new MWException( 'title field must contain a Title object.' );
+				}
+
+				return $row['title'];
+			}
+
+			$pageId = isset( $row['page'] ) ? $row['page'] : 0;
+			$revId = isset( $row['id'] ) ? $row['id'] : 0;
+		} else {
+			$pageId = isset( $row->rev_page ) ? $row->rev_page : 0;
+			$revId = isset( $row->rev_id ) ? $row->rev_id : 0;
+		}
+
+		try {
+			$title = self::getRevisionStore()->getTitle( $pageId, $revId, $queryFlags );
+		} catch ( RevisionAccessException $ex ) {
+			// construct a dummy title!
+			wfLogWarning( __METHOD__ . ': ' . $ex->getMessage() );
+
+			// NOTE: this Title will only be used inside RevisionRecord
+			$title = Title::makeTitleSafe( NS_SPECIAL, "Badtitle/ID=$pageId" );
+			$title->resetArticleID( $pageId );
+		}
+
+		return $title;
+	}
+
 	/**
 	 * @return RevisionRecord
 	 */
diff --git a/includes/Storage/RevisionStore.php b/includes/Storage/RevisionStore.php
index 78e789e5f4..6c8cac9423 100644
--- a/includes/Storage/RevisionStore.php
+++ b/includes/Storage/RevisionStore.php
@@ -164,6 +164,8 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
 	 *
 	 * MCR migration note: this corresponds to Revision::getTitle
 	 *
+	 * @note this method should be private, external use should be avoided!
+	 *
 	 * @param int|null $pageId
 	 * @param int|null $revId
 	 * @param int $queryFlags
@@ -171,7 +173,7 @@ class RevisionStore implements IDBAccessObject, RevisionFactory, RevisionLookup
 	 * @return Title
 	 * @throws RevisionAccessException
 	 */
-	private function getTitle( $pageId, $revId, $queryFlags = 0 ) {
+	public function getTitle( $pageId, $revId, $queryFlags = 0 ) {
 		if ( !$pageId && !$revId ) {
 			throw new InvalidArgumentException( '$pageId and $revId cannot both be 0 or null' );
 		}
diff --git a/tests/phpunit/includes/RevisionTest.php b/tests/phpunit/includes/RevisionTest.php
index 0db76ff193..7ce43ed7ea 100644
--- a/tests/phpunit/includes/RevisionTest.php
+++ b/tests/phpunit/includes/RevisionTest.php
@@ -76,6 +76,17 @@ class RevisionTest extends MediaWikiTestCase {
 		$this->assertNull( $rev->getContent(), 'no content object should be available' );
 	}
 
+	/**
+	 * @covers Revision::__construct
+	 * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+	 */
+	public function testConstructFromArrayWithBadPageId() {
+		MediaWiki\suppressWarnings();
+		$rev = new Revision( [ 'page' => 77777777 ] );
+		$this->assertSame( 77777777, $rev->getPage() );
+		MediaWiki\restoreWarnings();
+	}
+
 	public function provideConstructFromArray_userSetAsExpected() {
 		yield 'no user defaults to wgUser' => [
 			[
@@ -297,6 +308,17 @@ class RevisionTest extends MediaWikiTestCase {
 		$assertions( $this, $rev );
 	}
 
+	/**
+	 * @covers Revision::__construct
+	 * @covers \MediaWiki\Storage\RevisionStore::newMutableRevisionFromArray
+	 */
+	public function testConstructFromRowWithBadPageId() {
+		MediaWiki\suppressWarnings();
+		$rev = new Revision( (object)[ 'rev_page' => 77777777 ] );
+		$this->assertSame( 77777777, $rev->getPage() );
+		MediaWiki\restoreWarnings();
+	}
+
 	public function provideGetRevisionText() {
 		yield 'Generic test' => [
 			'This is a goat of revision text.',
-- 
2.20.1