From 6a9497769c9f1d24dfe922255d77691e270728b3 Mon Sep 17 00:00:00 2001
From: daniel <dkinzler@wikimedia.org>
Date: Fri, 23 Nov 2018 13:30:35 +0100
Subject: [PATCH] RevisionStore: Avoid exception on prev/next of deleted
 revision

This makes RevisionStore a bit more robust against use with deleted
revisions.

Note that this does not make I151019e336bda redundant, since detecting
this situation early and providing meaningful error message is useful.

Bug: T208929
Change-Id: I7635ddc0176e21d873eacadebe02603b1fe51c38
---
 includes/Revision/RevisionStore.php           | 49 ++++++++++++++++---
 .../Revision/RevisionStoreDbTestBase.php      | 40 +++++++++++++++
 2 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/includes/Revision/RevisionStore.php b/includes/Revision/RevisionStore.php
index 6d3b72cdf9..73881cdfe7 100644
--- a/includes/Revision/RevisionStore.php
+++ b/includes/Revision/RevisionStore.php
@@ -2517,45 +2517,78 @@ class RevisionStore
 	}
 
 	/**
-	 * Get previous revision for this title
+	 * Get the revision before $rev in the page's history, if any.
+	 * Will return null for the first revision but also for deleted or unsaved revisions.
 	 *
 	 * MCR migration note: this replaces Revision::getPrevious
 	 *
+	 * @see Title::getPreviousRevisionID
+	 * @see PageArchive::getPreviousRevision
+	 *
 	 * @param RevisionRecord $rev
 	 * @param Title|null $title if known (optional)
 	 *
 	 * @return RevisionRecord|null
 	 */
 	public function getPreviousRevision( RevisionRecord $rev, Title $title = null ) {
+		if ( !$rev->getId() || !$rev->getPageId() ) {
+			// revision is unsaved or otherwise incomplete
+			return null;
+		}
+
+		if ( $rev instanceof RevisionArchiveRecord ) {
+			// revision is deleted, so it's not part of the page history
+			return null;
+		}
+
 		if ( $title === null ) {
+			// this would fail for deleted revisions
 			$title = $this->getTitle( $rev->getPageId(), $rev->getId() );
 		}
+
 		$prev = $title->getPreviousRevisionID( $rev->getId() );
-		if ( $prev ) {
-			return $this->getRevisionByTitle( $title, $prev );
+		if ( !$prev ) {
+			return null;
 		}
-		return null;
+
+		return $this->getRevisionByTitle( $title, $prev );
 	}
 
 	/**
-	 * Get next revision for this title
+	 * Get the revision after $rev in the page's history, if any.
+	 * Will return null for the latest revision but also for deleted or unsaved revisions.
 	 *
 	 * MCR migration note: this replaces Revision::getNext
 	 *
+	 * @see Title::getNextRevisionID
+	 *
 	 * @param RevisionRecord $rev
 	 * @param Title|null $title if known (optional)
 	 *
 	 * @return RevisionRecord|null
 	 */
 	public function getNextRevision( RevisionRecord $rev, Title $title = null ) {
+		if ( !$rev->getId() || !$rev->getPageId() ) {
+			// revision is unsaved or otherwise incomplete
+			return null;
+		}
+
+		if ( $rev instanceof RevisionArchiveRecord ) {
+			// revision is deleted, so it's not part of the page history
+			return null;
+		}
+
 		if ( $title === null ) {
+			// this would fail for deleted revisions
 			$title = $this->getTitle( $rev->getPageId(), $rev->getId() );
 		}
+
 		$next = $title->getNextRevisionID( $rev->getId() );
-		if ( $next ) {
-			return $this->getRevisionByTitle( $title, $next );
+		if ( !$next ) {
+			return null;
 		}
-		return null;
+
+		return $this->getRevisionByTitle( $title, $next );
 	}
 
 	/**
diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
index 0d6a439dc3..4ece51bead 100644
--- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
+++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
@@ -12,7 +12,9 @@ use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\IncompleteRevisionException;
 use MediaWiki\Revision\MutableRevisionRecord;
+use MediaWiki\Revision\RevisionArchiveRecord;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionSlots;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Revision\SlotRecord;
 use MediaWiki\Storage\BlobStoreFactory;
@@ -1354,6 +1356,44 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
 		);
 	}
 
+	public function provideNonHistoryRevision() {
+		$title = Title::newFromText( __METHOD__ );
+		$rev = new MutableRevisionRecord( $title );
+		yield [ $rev ];
+
+		$user = new UserIdentityValue( 7, 'Frank', 0 );
+		$comment = CommentStoreComment::newUnsavedComment( 'Test' );
+		$row = (object)[
+			'ar_id' => 3,
+			'ar_rev_id' => 34567,
+			'ar_page_id' => 5,
+			'ar_deleted' => 0,
+			'ar_minor_edit' => 0,
+			'ar_timestamp' => '20180101020202',
+		];
+		$slots = new RevisionSlots( [] );
+		$rev = new RevisionArchiveRecord( $title, $user, $comment, $row, $slots );
+		yield [ $rev ];
+	}
+
+	/**
+	 * @dataProvider provideNonHistoryRevision
+	 * @covers \MediaWiki\Revision\RevisionStore::getPreviousRevision
+	 */
+	public function testGetPreviousRevision_bad( RevisionRecord $rev ) {
+		$store = MediaWikiServices::getInstance()->getRevisionStore();
+		$this->assertNull( $store->getPreviousRevision( $rev ) );
+	}
+
+	/**
+	 * @dataProvider provideNonHistoryRevision
+	 * @covers \MediaWiki\Revision\RevisionStore::getNextRevision
+	 */
+	public function testGetNextRevision_bad( RevisionRecord $rev ) {
+		$store = MediaWikiServices::getInstance()->getRevisionStore();
+		$this->assertNull( $store->getNextRevision( $rev ) );
+	}
+
 	/**
 	 * @covers \MediaWiki\Revision\RevisionStore::getTimestampFromId
 	 */
-- 
2.20.1