From e72256a6d970ab9d03d0278ed69f73d189fc0160 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Fri, 21 Apr 2017 11:00:14 -0400 Subject: [PATCH] API: Convert rvstartid/rvendid to timestamps for query We tried something like this once before, but reverted it because it was an unintended behavior change (see T98467). This time it's intended, we need it for query optimization. The behavior changes here are: * rvstartid/rvendid is exactly equivalent to specifying rvstart/rvend with the corresponding revisions' timestamps. * If the revision for rvstartid/rvendid is not found in the database, an error will be thrown. This will pull timestamps from deleted revisions, i.e. the `archive` table. While this is technically an information leak (that some revision ID exists as a deleted revision and the time the revision was made), it's minor and in line with the information revealed in Tool Labs thanks to T51088. Bug: T163532 Change-Id: Ida64a377c38b3553aa82ac754d80e8f898caf6c5 --- includes/api/ApiQueryRevisions.php | 47 ++++++++++++++++++++++++++++-- includes/api/i18n/en.json | 5 ++-- includes/api/i18n/qqq.json | 1 + 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/includes/api/ApiQueryRevisions.php b/includes/api/ApiQueryRevisions.php index 7b8394f7fd..3e077c3316 100644 --- a/includes/api/ApiQueryRevisions.php +++ b/includes/api/ApiQueryRevisions.php @@ -218,10 +218,53 @@ class ApiQueryRevisions extends ApiQueryRevisionsBase { ); } + // Convert startid/endid to timestamps (T163532) + if ( $params['startid'] !== null || $params['endid'] !== null ) { + $ids = [ + (int)$params['startid'] => true, + (int)$params['endid'] => true, + ]; + unset( $ids[0] ); // null + $ids = array_keys( $ids ); + + $db = $this->getDB(); + $sql = $db->unionQueries( [ + $db->selectSQLText( + 'revision', + [ 'id' => 'rev_id', 'ts' => 'rev_timestamp' ], + [ 'rev_id' => $ids ], + __METHOD__ + ), + $db->selectSQLText( + 'archive', + [ 'id' => 'ar_rev_id', 'ts' => 'ar_timestamp' ], + [ 'ar_rev_id' => $ids ], + __METHOD__ + ), + ], false ); + $res = $db->query( $sql, __METHOD__ ); + foreach ( $res as $row ) { + if ( (int)$row->id === (int)$params['startid'] ) { + $params['start'] = $row->ts; + } + if ( (int)$row->id === (int)$params['endid'] ) { + $params['end'] = $row->ts; + } + } + if ( $params['startid'] !== null && $params['start'] === null ) { + $p = $this->encodeParamName( 'startid' ); + $this->dieWithError( [ 'apierror-revisions-badid', $p ], "badid_$p" ); + } + if ( $params['endid'] !== null && $params['end'] === null ) { + $p = $this->encodeParamName( 'endid' ); + $this->dieWithError( [ 'apierror-revisions-badid', $p ], "badid_$p" ); + } + } + $this->addTimestampWhereRange( 'rev_timestamp', $params['dir'], $params['start'], $params['end'] ); - $this->addWhereRange( 'rev_id', $params['dir'], - $params['startid'], $params['endid'] ); + // Dummy to add rev_id to ORDER BY + $this->addWhereRange( 'rev_id', $params['dir'], null, null ); // There is only one ID, use it $ids = array_keys( $pageSet->getGoodTitles() ); diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index 387e4b6d7a..c3c7bd4571 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1057,8 +1057,8 @@ "apihelp-query+revisions-description": "Get revision information.\n\nMay be used in several ways:\n# Get data about a set of pages (last revision), by setting titles or pageids.\n# Get revisions for one given page, by using titles or pageids with start, end, or limit.\n# Get data about a set of revisions by setting their IDs with revids.", "apihelp-query+revisions-paraminfo-singlepageonly": "May only be used with a single page (mode #2).", - "apihelp-query+revisions-param-startid": "From which revision ID to start enumeration.", - "apihelp-query+revisions-param-endid": "Stop revision enumeration on this revision ID.", + "apihelp-query+revisions-param-startid": "Start enumeration from this revision's timestamp. The revision must exist, but need not belong to this page.", + "apihelp-query+revisions-param-endid": "Stop enumeration at this revision's timestamp. The revision must exist, but need not belong to this page.", "apihelp-query+revisions-param-start": "From which revision timestamp to start enumeration.", "apihelp-query+revisions-param-end": "Enumerate up to this timestamp.", "apihelp-query+revisions-param-user": "Only include revisions made by user.", @@ -1721,6 +1721,7 @@ "apierror-revdel-mutuallyexclusive": "The same field cannot be used in both hide and show.", "apierror-revdel-needtarget": "A target title is required for this RevDel type.", "apierror-revdel-paramneeded": "At least one value is required for hide and/or show.", + "apierror-revisions-badid": "No revision was found for parameter $1.", "apierror-revisions-norevids": "The revids parameter may not be used with the list options ($1limit, $1startid, $1endid, $1dir=newer, $1user, $1excludeuser, $1start, and $1end).", "apierror-revisions-singlepage": "titles, pageids or a generator was used to supply multiple pages, but the $1limit, $1startid, $1endid, $1dir=newer, $1user, $1excludeuser, $1start, and $1end parameters may only be used on a single page.", "apierror-revwrongpage": "r$1 is not a revision of $2.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 81adeec437..da0b22d892 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -1613,6 +1613,7 @@ "apierror-revdel-mutuallyexclusive": "{{doc-apierror}}", "apierror-revdel-needtarget": "{{doc-apierror}}", "apierror-revdel-paramneeded": "{{doc-apierror}}", + "apierror-revisions-badid": "{{doc-apierror}}\n\nParameters:\n* $1 - Parameter in question, e.g. \"rvstartid\".", "apierror-revisions-norevids": "{{doc-apierror}}\n\nParameters:\n* $1 - Module parameter prefix, e.g. \"bl\".", "apierror-revisions-singlepage": "{{doc-apierror}}\n\nParameters:\n* $1 - Module parameter prefix, e.g. \"bl\".", "apierror-revwrongpage": "{{doc-apierror}}\n\nParameters:\n* $1 - Revision ID number.\n* $2 - Page title.", -- 2.20.1