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

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 657181b..2770bdc 100644 (file)
--- 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 9e9bda6..cc28a01 100644 (file)
--- 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 918a392..55215b3 100644 (file)
--- 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 7741c35..8a868a3 100644 (file)
--- 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 );
                }
        }
 }