Do not return invalid hash from ApiStashEdit

[lhc/web/wiklou.git] / includes / api / ApiStashEdit.php

diff --git a/includes/api/ApiStashEdit.php b/includes/api/ApiStashEdit.php
index 8a9de06..e8318c2 100644 (file)
--- a/includes/api/ApiStashEdit.php
+++ b/includes/api/ApiStashEdit.php
@@ -67,11 +67,11 @@ class ApiStashEdit extends ApiBase {
                        );
                }
 
-               $this->requireAtLeastOneParameter( $params, 'stashedtexthash', 'text' );
+               $this->requireOnlyOneParameter( $params, 'stashedtexthash', 'text' );
 
                $text = null;
                $textHash = null;
-               if ( strlen( $params['stashedtexthash'] ) ) {
+               if ( $params['stashedtexthash'] !== null ) {
                        // Load from cache since the client indicates the text is the same as last stash
                        $textHash = $params['stashedtexthash'];
                        if ( !preg_match( '/^[0-9a-f]{40}$/', $textHash ) ) {
@@ -82,16 +82,11 @@ class ApiStashEdit extends ApiBase {
                        if ( !is_string( $text ) ) {
                                $this->dieWithError( 'apierror-stashedit-missingtext', 'missingtext' );
                        }
-               } elseif ( $params['text'] !== null ) {
-                       // Trim and fix newlines so the key SHA1's match (see WebRequest::getText())
+               } else {
+                       // 'text' was passed.  Trim and fix newlines so the key SHA1's
+                       // match (see WebRequest::getText())
                        $text = rtrim( str_replace( "\r\n", "\n", $params['text'] ) );
                        $textHash = sha1( $text );
-               } else {
-                       $this->dieWithError( [
-                               'apierror-missingparam-at-least-one-of',
-                               Message::listParam( [ '<var>stashedtexthash</var>', '<var>text</var>' ] ),
-                               2,
-                       ], 'missingparam' );
                }
 
                $textContent = ContentHandler::makeContent(
@@ -156,14 +151,13 @@ class ApiStashEdit extends ApiBase {
                $stats = MediaWikiServices::getInstance()->getStatsdDataFactory();
                $stats->increment( "editstash.cache_stores.$status" );
 
-               $this->getResult()->addValue(
-                       null,
-                       $this->getModuleName(),
-                       [
-                               'status' => $status,
-                               'texthash' => $textHash
-                       ]
-               );
+               $ret = [ 'status' => $status ];
+               // If we were rate-limited, we still return the pre-existing valid hash if one was passed
+               if ( $status !== 'ratelimited' || $params['stashedtexthash'] !== null ) {
+                       $ret['texthash'] = $textHash;
+               }
+
+               $this->getResult()->addValue( null, $this->getModuleName(), $ret );
        }
 
        /**
@@ -181,9 +175,14 @@ class ApiStashEdit extends ApiBase {
                $title = $page->getTitle();
                $key = self::getStashKey( $title, self::getContentHash( $content ), $user );
 
-               // Use the master DB for fast blocking locks
+               // Use the master DB to allow for fast blocking locks on the "save path" where this
+               // value might actually be used to complete a page edit. If the edit submission request
+               // happens before this edit stash requests finishes, then the submission will block until
+               // the stash request finishes parsing. For the lock acquisition below, there is not much
+               // need to duplicate parsing of the same content/user/summary bundle, so try to avoid
+               // blocking at all here.
                $dbw = wfGetDB( DB_MASTER );
-               if ( !$dbw->lock( $key, __METHOD__, 1 ) ) {
+               if ( !$dbw->lock( $key, __METHOD__, 0 ) ) {
                        // De-duplicate requests on the same key
                        return self::ERROR_BUSY;
                }
@@ -209,8 +208,10 @@ class ApiStashEdit extends ApiBase {
                        Hooks::run( 'ParserOutputStashForEdit',
                                [ $page, $content, $editInfo->output, $summary, $user ] );
 
+                       $titleStr = (string)$title;
                        if ( $alreadyCached ) {
-                               $logger->debug( "Already cached parser output for key '$key' ('$title')." );
+                               $logger->debug( "Already cached parser output for key '{cachekey}' ('{title}').",
+                                       [ 'cachekey' => $key, 'title' => $titleStr ] );
                                return self::ERROR_NONE;
                        }
 
@@ -224,14 +225,17 @@ class ApiStashEdit extends ApiBase {
                        if ( $stashInfo ) {
                                $ok = $cache->set( $key, $stashInfo, $ttl );
                                if ( $ok ) {
-                                       $logger->debug( "Cached parser output for key '$key' ('$title')." );
+                                       $logger->debug( "Cached parser output for key '{cachekey}' ('{title}').",
+                                               [ 'cachekey' => $key, 'title' => $titleStr ] );
                                        return self::ERROR_NONE;
                                } else {
-                                       $logger->error( "Failed to cache parser output for key '$key' ('$title')." );
+                                       $logger->error( "Failed to cache parser output for key '{cachekey}' ('{title}').",
+                                               [ 'cachekey' => $key, 'title' => $titleStr ] );
                                        return self::ERROR_CACHE;
                                }
                        } else {
-                               $logger->info( "Uncacheable parser output for key '$key' ('$title') [$code]." );
+                               $logger->info( "Uncacheable parser output for key '{cachekey}' ('{title}') [{code}].",
+                                       [ 'cachekey' => $key, 'title' => $titleStr, 'code' => $code ] );
                                return self::ERROR_UNCACHEABLE;
                        }
                }
@@ -330,11 +334,15 @@ class ApiStashEdit extends ApiBase {
         * @return string|null TS_MW timestamp or null
         */
        private static function lastEditTime( User $user ) {
-               $time = wfGetDB( DB_REPLICA )->selectField(
-                       'recentchanges',
+               $db = wfGetDB( DB_REPLICA );
+               $actorQuery = ActorMigration::newMigration()->getWhere( $db, 'rc_user', $user, false );
+               $time = $db->selectField(
+                       [ 'recentchanges' ] + $actorQuery['tables'],
                        'MAX(rc_timestamp)',
-                       [ 'rc_user_text' => $user->getName() ],
-                       __METHOD__
+                       [ $actorQuery['conds'] ],
+                       __METHOD__,
+                       [],
+                       $actorQuery['joins']
                );
 
                return wfTimestampOrNull( TS_MW, $time );