From c7844017c040885512b83e0e70b7bdfbcd0e9ab7 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 29 Sep 2014 16:20:57 -0700 Subject: [PATCH] Made upload jobs avoid using the user session * This causes problems with some session handlers and it is also trickier to deal with in non CLI script without leaking cookie headers. Change-Id: Iaf2a57f9299e42a5f68bf85115e62e88fa0f8ed6 --- includes/api/ApiUpload.php | 8 +++-- .../jobqueue/jobs/AssembleUploadChunksJob.php | 17 +++------- .../jobqueue/jobs/PublishStashedFileJob.php | 18 ++++------ includes/upload/UploadBase.php | 33 +++++++++++-------- 4 files changed, 36 insertions(+), 40 deletions(-) diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 657181b77a..2770bdcf68 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -223,11 +223,12 @@ class ApiUpload extends ApiBase { // Check we added the last chunk: if ( $this->mParams['offset'] + $chunkSize == $this->mParams['filesize'] ) { if ( $this->mParams['async'] ) { - $progress = UploadBase::getSessionStatus( $filekey ); + $progress = UploadBase::getSessionStatus( $this->getUser(), $filekey ); if ( $progress && $progress['result'] === 'Poll' ) { $this->dieUsage( "Chunk assembly already in progress.", 'stashfailed' ); } UploadBase::setSessionStatus( + $this->getUser(), $filekey, array( 'result' => 'Poll', 'stage' => 'queued', 'status' => Status::newGood() ) @@ -327,7 +328,7 @@ class ApiUpload extends ApiBase { // Status report for "upload to stash"/"upload from stash" if ( $this->mParams['filekey'] && $this->mParams['checkstatus'] ) { - $progress = UploadBase::getSessionStatus( $this->mParams['filekey'] ); + $progress = UploadBase::getSessionStatus( $this->getUser(), $this->mParams['filekey'] ); if ( !$progress ) { $this->dieUsage( 'No result in status data', 'missingresult' ); } elseif ( !$progress['status']->isGood() ) { @@ -612,11 +613,12 @@ class ApiUpload extends ApiBase { // No errors, no warnings: do the upload if ( $this->mParams['async'] ) { - $progress = UploadBase::getSessionStatus( $this->mParams['filekey'] ); + $progress = UploadBase::getSessionStatus( $this->getUser(), $this->mParams['filekey'] ); if ( $progress && $progress['result'] === 'Poll' ) { $this->dieUsage( "Upload from stash already in progress.", 'publishfailed' ); } UploadBase::setSessionStatus( + $this->getUser(), $this->mParams['filekey'], array( 'result' => 'Poll', 'stage' => 'queued', 'status' => Status::newGood() ) ); diff --git a/includes/jobqueue/jobs/AssembleUploadChunksJob.php b/includes/jobqueue/jobs/AssembleUploadChunksJob.php index 9e9bda6ff0..cc28a019c0 100644 --- a/includes/jobqueue/jobs/AssembleUploadChunksJob.php +++ b/includes/jobqueue/jobs/AssembleUploadChunksJob.php @@ -35,26 +35,16 @@ class AssembleUploadChunksJob extends Job { public function run() { $scope = RequestContext::importScopedSession( $this->params['session'] ); $context = RequestContext::getMain(); + $user = $context->getUser(); try { - $user = $context->getUser(); if ( !$user->isLoggedIn() ) { $this->setLastError( "Could not load the author user from session." ); return false; } - if ( count( $_SESSION ) === 0 ) { - // Empty session probably indicates that we didn't associate - // with the session correctly. Note that being able to load - // the user does not necessarily mean the session was loaded. - // Most likely cause by suhosin.session.encrypt = On. - $this->setLastError( "Error associating with user session. " . - "Try setting suhosin.session.encrypt = Off" ); - - return false; - } - UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Poll', 'stage' => 'assembling', 'status' => Status::newGood() ) ); @@ -70,6 +60,7 @@ class AssembleUploadChunksJob extends Job { $status = $upload->concatenateChunks(); if ( !$status->isGood() ) { UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Failure', 'stage' => 'assembling', 'status' => $status ) ); @@ -93,6 +84,7 @@ class AssembleUploadChunksJob extends Job { // Cache the info so the user doesn't have to wait forever to get the final info UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Success', @@ -104,6 +96,7 @@ class AssembleUploadChunksJob extends Job { ); } catch ( MWException $e ) { UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Failure', diff --git a/includes/jobqueue/jobs/PublishStashedFileJob.php b/includes/jobqueue/jobs/PublishStashedFileJob.php index 918a392db4..55215b3e36 100644 --- a/includes/jobqueue/jobs/PublishStashedFileJob.php +++ b/includes/jobqueue/jobs/PublishStashedFileJob.php @@ -35,26 +35,16 @@ class PublishStashedFileJob extends Job { public function run() { $scope = RequestContext::importScopedSession( $this->params['session'] ); $context = RequestContext::getMain(); + $user = $context->getUser(); try { - $user = $context->getUser(); if ( !$user->isLoggedIn() ) { $this->setLastError( "Could not load the author user from session." ); return false; } - if ( count( $_SESSION ) === 0 ) { - // Empty session probably indicates that we didn't associate - // with the session correctly. Note that being able to load - // the user does not necessarily mean the session was loaded. - // Most likely cause by suhosin.session.encrypt = On. - $this->setLastError( "Error associating with user session. " . - "Try setting suhosin.session.encrypt = Off" ); - - return false; - } - UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Poll', 'stage' => 'publish', 'status' => Status::newGood() ) ); @@ -72,6 +62,7 @@ class PublishStashedFileJob extends Job { $status = Status::newFatal( 'verification-error' ); $status->value = array( 'verification' => $verification ); UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Failure', 'stage' => 'publish', 'status' => $status ) ); @@ -89,6 +80,7 @@ class PublishStashedFileJob extends Job { ); if ( !$status->isGood() ) { UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Failure', 'stage' => 'publish', 'status' => $status ) ); @@ -106,6 +98,7 @@ class PublishStashedFileJob extends Job { // Cache the info so the user doesn't have to wait forever to get the final info UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Success', @@ -117,6 +110,7 @@ class PublishStashedFileJob extends Job { ); } catch ( MWException $e ) { UploadBase::setSessionStatus( + $user, $this->params['filekey'], array( 'result' => 'Failure', diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 7741c350e7..8a868a3b1f 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -69,8 +69,6 @@ abstract class UploadBase { const WINDOWS_NONASCII_FILENAME = 13; const FILENAME_TOO_LONG = 14; - const SESSION_STATUS_KEY = 'wsUploadStatusData'; - /** * @param int $error * @return string @@ -1977,29 +1975,38 @@ abstract class UploadBase { } /** - * Get the current status of a chunked upload (used for polling). - * The status will be read from the *current* user session. + * Get the current status of a chunked upload (used for polling) + * + * The value will be read from cache. + * + * @param User $user * @param string $statusKey * @return Status[]|bool */ - public static function getSessionStatus( $statusKey ) { - return isset( $_SESSION[self::SESSION_STATUS_KEY][$statusKey] ) - ? $_SESSION[self::SESSION_STATUS_KEY][$statusKey] - : false; + public static function getSessionStatus( User $user, $statusKey ) { + $key = wfMemcKey( 'uploadstatus', $user->getId() ?: md5( $user->getName() ), $statusKey ); + + return wfGetCache( CACHE_ANYTHING )->get( $key ); } /** - * Set the current status of a chunked upload (used for polling). - * The status will be stored in the *current* user session. + * Set the current status of a chunked upload (used for polling) + * + * The value will be set in cache for 1 day + * + * @param User $user * @param string $statusKey * @param array|bool $value * @return void */ - public static function setSessionStatus( $statusKey, $value ) { + public static function setSessionStatus( User $user, $statusKey, $value ) { + $key = wfMemcKey( 'uploadstatus', $user->getId() ?: md5( $user->getName() ), $statusKey ); + + $cache = wfGetCache( CACHE_ANYTHING ); if ( $value === false ) { - unset( $_SESSION[self::SESSION_STATUS_KEY][$statusKey] ); + $cache->delete( $key ); } else { - $_SESSION[self::SESSION_STATUS_KEY][$statusKey] = $value; + $cache->set( $key, $value, 86400 ); } } } -- 2.20.1