From 96528b1f889506dbc21e31a9416787dc221d10a2 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 12 Mar 2014 12:19:27 -0700 Subject: [PATCH] Avoid header notice log spam from RunJobs API * Moved ApiRunJobs to a special page instead of going through ApiMain and having to fight the logic there. As a separate internal API, this does not show up on the API help page and is no longer effected by $wgEnableAPI. bug: 62233 Change-Id: I1db6f526d02e130a66ee03289858a734d89e6c00 --- includes/AutoLoader.php | 2 +- includes/Wiki.php | 18 ++- includes/api/ApiMain.php | 1 - includes/specialpage/SpecialPageFactory.php | 1 + .../SpecialRunJobs.php} | 109 ++++++++---------- languages/messages/MessagesEn.php | 1 + 6 files changed, 57 insertions(+), 75 deletions(-) rename includes/{api/ApiRunJobs.php => specials/SpecialRunJobs.php} (63%) diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index c627da16fe..d5c28ba7d4 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -361,7 +361,6 @@ $wgAutoloadLocalClasses = array( 'ApiRevisionDelete' => 'includes/api/ApiRevisionDelete.php', 'ApiRollback' => 'includes/api/ApiRollback.php', 'ApiRsd' => 'includes/api/ApiRsd.php', - 'ApiRunJobs' => 'includes/api/ApiRunJobs.php', 'ApiSetNotificationTimestamp' => 'includes/api/ApiSetNotificationTimestamp.php', 'ApiTokens' => 'includes/api/ApiTokens.php', 'ApiUnblock' => 'includes/api/ApiUnblock.php', @@ -1018,6 +1017,7 @@ $wgAutoloadLocalClasses = array( 'SpecialRedirect' => 'includes/specials/SpecialRedirect.php', 'SpecialResetTokens' => 'includes/specials/SpecialResetTokens.php', 'SpecialRevisionDelete' => 'includes/specials/SpecialRevisiondelete.php', + 'SpecialRunJobs' => 'includes/specials/SpecialRunJobs.php', 'SpecialSearch' => 'includes/specials/SpecialSearch.php', 'SpecialSpecialpages' => 'includes/specials/SpecialSpecialpages.php', 'SpecialStatistics' => 'includes/specials/SpecialStatistics.php', diff --git a/includes/Wiki.php b/includes/Wiki.php index 69cfe9dd5a..4bf8fd3a59 100644 --- a/includes/Wiki.php +++ b/includes/Wiki.php @@ -624,10 +624,12 @@ class MediaWiki { * the socket once it's done. */ protected function triggerJobs() { - global $wgJobRunRate, $wgServer, $wgScriptPath, $wgScriptExtension, $wgEnableAPI; + global $wgJobRunRate, $wgServer; if ( $wgJobRunRate <= 0 || wfReadOnly() ) { return; + } elseif ( $this->getTitle()->isSpecial( 'RunJobs' ) ) { + return; // recursion guard } $section = new ProfileSection( __METHOD__ ); @@ -642,15 +644,9 @@ class MediaWiki { $n = intval( $wgJobRunRate ); } - $query = array( 'action' => 'runjobs', + $query = array( 'title' => 'Special:RunJobs', 'tasks' => 'jobs', 'maxjobs' => $n, 'sigexpiry' => time() + 5 ); - $query['signature'] = ApiRunJobs::getQuerySignature( $query ); - - if ( !$wgEnableAPI ) { - // Fall back to running the job here while the user waits - ApiRunJobs::executeJobs( $n ); - return; - } + $query['signature'] = SpecialRunJobs::getQuerySignature( $query ); $errno = $errstr = null; $info = wfParseUrl( $wgServer ); @@ -665,11 +661,11 @@ class MediaWiki { if ( !$sock ) { wfDebugLog( 'runJobs', "Failed to start cron API (socket error $errno): $errstr\n" ); // Fall back to running the job here while the user waits - ApiRunJobs::executeJobs( $n ); + SpecialRunJobs::executeJobs( $n ); return; } - $url = wfAppendQuery( "{$wgScriptPath}/api{$wgScriptExtension}", $query ); + $url = wfAppendQuery( wfScript( 'index' ), $query ); $req = "POST $url HTTP/1.1\r\nHost: {$info['host']}\r\nConnection: Close\r\n\r\n"; wfDebugLog( 'runJobs', "Running $n job(s) via '$url'\n" ); diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 8f270dcb18..e1c087474f 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -68,7 +68,6 @@ class ApiMain extends ApiBase { 'purge' => 'ApiPurge', 'setnotificationtimestamp' => 'ApiSetNotificationTimestamp', 'rollback' => 'ApiRollback', - 'runjobs' => 'ApiRunJobs', 'delete' => 'ApiDelete', 'undelete' => 'ApiUndelete', 'protect' => 'ApiProtect', diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php index dea65f3534..c6735e695a 100644 --- a/includes/specialpage/SpecialPageFactory.php +++ b/includes/specialpage/SpecialPageFactory.php @@ -165,6 +165,7 @@ class SpecialPageFactory { 'PermanentLink' => 'SpecialPermanentLink', 'Redirect' => 'SpecialRedirect', 'Revisiondelete' => 'SpecialRevisionDelete', + 'RunJobs' => 'SpecialRunJobs', 'Specialpages' => 'SpecialSpecialpages', 'Userlogout' => 'SpecialUserlogout', ); diff --git a/includes/api/ApiRunJobs.php b/includes/specials/SpecialRunJobs.php similarity index 63% rename from includes/api/ApiRunJobs.php rename to includes/specials/SpecialRunJobs.php index 05327c8ab1..8a4026d62f 100644 --- a/includes/api/ApiRunJobs.php +++ b/includes/specials/SpecialRunJobs.php @@ -1,5 +1,6 @@ getOutput()->disable(); + if ( wfReadOnly() ) { - $this->dieUsage( 'Wiki is in read-only mode', 'read_only', 400 ); + header( "HTTP/1.0 423 Locked" ); + print 'Wiki is in read-only mode'; + return; + } elseif ( !$this->getRequest()->wasPosted() ) { + header( "HTTP/1.0 400 Bad Request" ); + print 'Request must be POSTed'; + return; } - $params = $this->extractRequestParams(); - $squery = $this->getRequest()->getValues(); + $optional = array( 'maxjobs' => 0 ); + $required = array_flip( array( 'title', 'tasks', 'signature', 'sigexpiry' ) ); + + $params = array_intersect_key( $this->getRequest()->getValues(), $required + $optional ); + $missing = array_diff_key( $required, $params ); + if ( count( $missing ) ) { + header( "HTTP/1.0 400 Bad Request" ); + print 'Missing parameters: ' . implode( ', ', array_keys( $missing ) ); + return; + } + + $squery = $params; unset( $squery['signature'] ); - $cSig = self::getQuerySignature( $squery ); - $rSig = $params['signature']; + $cSig = self::getQuerySignature( $squery ); // correct signature + $rSig = $params['signature']; // provided signature - // Time-insensitive signature verification - if ( strlen( $rSig ) !== strlen( $cSig ) ) { + // Constant-time signature verification + // http://www.emerose.com/timing-attacks-explained + // @todo: make a common method for this + if ( !is_string( $rSig ) || strlen( $rSig ) !== strlen( $cSig ) ) { $verified = false; } else { $result = 0; @@ -49,23 +73,27 @@ class ApiRunJobs extends ApiBase { } $verified = ( $result == 0 ); } - if ( !$verified || $params['sigexpiry'] < time() ) { - $this->dieUsage( 'Invalid or stale signature provided', 'bad_signature', 400 ); + header( "HTTP/1.0 400 Bad Request" ); + print 'Invalid or stale signature provided'; + return; } + // Apply any default parameter values + $params += $optional; + // Client will usually disconnect before checking the response, // but it needs to know when it is safe to disconnect. Until this // reaches ignore_user_abort(), it is not safe as the jobs won't run. ignore_user_abort( true ); // jobs may take a bit of time header( "HTTP/1.0 202 Accepted" ); ob_flush(); - flush(); + flush(); // Once the client receives this response, it can disconnect // Do all of the specified tasks... - if ( in_array( 'jobs', $params['tasks'] ) ) { - self::executeJobs( $params['maxjobs'] ); + if ( in_array( 'jobs', explode( '|', $params['tasks'] ) ) ) { + self::executeJobs( (int)$params['maxjobs'] ); } } @@ -101,7 +129,7 @@ class ApiRunJobs extends ApiBase { } do { - $job = $group->pop( JobQueueGroup::TYPE_DEFAULT, JobQueueGroup::USE_CACHE ); // job from any queue + $job = $group->pop( JobQueueGroup::TYPE_DEFAULT, JobQueueGroup::USE_CACHE ); if ( $job ) { $output = $job->toString() . "\n"; $t = - microtime( true ); @@ -126,47 +154,4 @@ class ApiRunJobs extends ApiBase { MWExceptionHandler::logException( $e ); } } - - public function mustBePosted() { - return true; - } - - public function getAllowedParams() { - return array( - 'tasks' => array( - ApiBase::PARAM_ISMULTI => true, - ApiBase::PARAM_TYPE => array( 'jobs' ) - ), - 'maxjobs' => array( - ApiBase::PARAM_TYPE => 'integer', - ApiBase::PARAM_DFLT => 0 - ), - 'signature' => array( - ApiBase::PROP_TYPE => 'string', - ), - 'sigexpiry' => array( - ApiBase::PARAM_TYPE => 'integer', - ApiBase::PARAM_DFLT => 0 // ~epoch - ), - ); - } - - public function getParamDescription() { - return array( - 'tasks' => 'List of task types to perform', - 'maxjobs' => 'Maximum number of jobs to run', - 'signature' => 'HMAC Signature that signs the request', - 'sigexpiry' => 'HMAC signature expiry as a UNIX timestamp' - ); - } - - public function getDescription() { - return 'Perform periodic tasks or run jobs from the queue.'; - } - - public function getExamples() { - return array( - 'api.php?action=runjobs&tasks=jobs&maxjobs=3' => 'Run up to 3 jobs from the queue', - ); - } } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index f1725a3a6d..113bb24377 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -460,6 +460,7 @@ $specialPageAliases = array( 'Redirect' => array( 'Redirect' ), 'ResetTokens' => array( 'ResetTokens' ), 'Revisiondelete' => array( 'RevisionDelete' ), + 'RunJobs' => array( 'RunJobs' ), 'Search' => array( 'Search' ), 'Shortpages' => array( 'ShortPages' ), 'Specialpages' => array( 'SpecialPages' ), -- 2.20.1