From 3c95d3bdd935768b246d0688eaaa258feeecd4e6 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Wed, 7 Nov 2018 11:01:26 -0500
Subject: [PATCH] ApiComparePages: Don't try to find next/prev of a deleted
 revision

RevisionStore::getPreviousRevision() and ::getNextRevision() don't
handle being passed a RevisionArchiveRecord very well. Even if they
would, it's not clear whether the user wants to be comparing with the
next/previous deleted revision or the next/previous revision even if not
deleted. So let's just make it an error, at least for now.

Bug: T208929
Change-Id: I151019e336bda92aa4040ba4162eb2588c909652
---
 includes/api/ApiComparePages.php              |  6 +++++
 includes/api/i18n/en.json                     |  1 +
 includes/api/i18n/qqq.json                    |  1 +
 .../includes/api/ApiComparePagesTest.php      | 24 +++++++++++++++++++
 4 files changed, 32 insertions(+)

diff --git a/includes/api/ApiComparePages.php b/includes/api/ApiComparePages.php
index 76b7bce67b..97b21b9d91 100644
--- a/includes/api/ApiComparePages.php
+++ b/includes/api/ApiComparePages.php
@@ -22,6 +22,7 @@
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Revision\MutableRevisionRecord;
 use MediaWiki\Revision\RevisionRecord;
+use MediaWiki\Revision\RevisionArchiveRecord;
 use MediaWiki\Revision\RevisionStore;
 use MediaWiki\Revision\SlotRecord;
 
@@ -61,6 +62,11 @@ class ApiComparePages extends ApiBase {
 			if ( !$fromRelRev ) {
 				$this->dieWithError( 'apierror-compare-relative-to-nothing' );
 			}
+			if ( $params['torelative'] !== 'cur' && $fromRelRev instanceof RevisionArchiveRecord ) {
+				// RevisionStore's getPreviousRevision/getNextRevision blow up
+				// when passed an RevisionArchiveRecord for a deleted page
+				$this->dieWithError( [ 'apierror-compare-relative-to-deleted', $params['torelative'] ] );
+			}
 			switch ( $params['torelative'] ) {
 				case 'prev':
 					// Swap 'from' and 'to'
diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json
index 83bb6e65d5..0028dfca8c 100644
--- a/includes/api/i18n/en.json
+++ b/includes/api/i18n/en.json
@@ -1730,6 +1730,7 @@
 	"apierror-compare-nofromrevision": "No 'from' revision. Specify <var>fromrev</var>, <var>fromtitle</var>, or <var>fromid</var>.",
 	"apierror-compare-notext": "Parameter <var>$1</var> cannot be used without <var>$2</var>.",
 	"apierror-compare-notorevision": "No 'to' revision. Specify <var>torev</var>, <var>totitle</var>, or <var>toid</var>.",
+	"apierror-compare-relative-to-deleted": "Cannot use <kbd>torelative=$1</kbd> relative to a deleted revision.",
 	"apierror-compare-relative-to-nothing": "No 'from' revision for <var>torelative</var> to be relative to.",
 	"apierror-contentserializationexception": "Content serialization failed: $1",
 	"apierror-contenttoobig": "The content you supplied exceeds the article size limit of $1 {{PLURAL:$1|kilobyte|kilobytes}}.",
diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json
index 83427bafd8..660e0faf41 100644
--- a/includes/api/i18n/qqq.json
+++ b/includes/api/i18n/qqq.json
@@ -1618,6 +1618,7 @@
 	"apierror-compare-nofromrevision": "{{doc-apierror}}",
 	"apierror-compare-notext": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter that is not allowed without <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var>.\n* $2 - The specific <var>totext-&#x7B;role}</var>/<var>fromtext-&#x7B;role}</var> parameter that must be present.",
 	"apierror-compare-notorevision": "{{doc-apierror}}",
+	"apierror-compare-relative-to-deleted": "{{doc-apierror}}",
 	"apierror-compare-relative-to-nothing": "{{doc-apierror}}",
 	"apierror-contentserializationexception": "{{doc-apierror}}\n\nParameters:\n* $1 - Exception text, may end with punctuation. Currently this is probably English, hopefully we'll fix that in the future.",
 	"apierror-contenttoobig": "{{doc-apierror}}\n\nParameters:\n* $1 - Maximum article size in kilobytes.",
diff --git a/tests/phpunit/includes/api/ApiComparePagesTest.php b/tests/phpunit/includes/api/ApiComparePagesTest.php
index 3709c2195a..e8cd342155 100644
--- a/tests/phpunit/includes/api/ApiComparePagesTest.php
+++ b/tests/phpunit/includes/api/ApiComparePagesTest.php
@@ -801,6 +801,30 @@ class ApiComparePagesTest extends ApiTestCase {
 				[],
 				'nosuchrevid',
 			],
+			'Error, deleted revision ID and torelative=prev' => [
+				[
+					'fromrev' => '{{REPL:revC2}}',
+					'torelative' => 'prev',
+				],
+				[],
+				'compare-relative-to-deleted', true
+			],
+			'Error, deleted revision ID and torelative=next' => [
+				[
+					'fromrev' => '{{REPL:revC2}}',
+					'torelative' => 'next',
+				],
+				[],
+				'compare-relative-to-deleted', true
+			],
+			'Deleted revision ID and torelative=cur' => [
+				[
+					'fromrev' => '{{REPL:revC2}}',
+					'torelative' => 'cur',
+				],
+				[],
+				'nosuchrevid', true
+			],
 			'Error, revision-deleted content' => [
 				[
 					'fromrev' => '{{REPL:revA2}}',
-- 
2.20.1