* Cleanup ApiUpload
authorBryan Tong Minh <btongminh@users.mediawiki.org>
Wed, 26 Aug 2009 19:38:38 +0000 (19:38 +0000)
committerBryan Tong Minh <btongminh@users.mediawiki.org>
Wed, 26 Aug 2009 19:38:38 +0000 (19:38 +0000)
* UploadBase::verifyUpload now always returns a status array
* Disabled async upload by url because wfShellBackgroundExec is broken

includes/GlobalFunctions.php
includes/api/ApiBase.php
includes/api/ApiUpload.php
includes/specials/SpecialUpload.php
includes/upload/UploadBase.php
includes/upload/UploadFromChunks.php
includes/upload/UploadFromUrl.php

index d59c7fe..7b98ed6 100644 (file)
@@ -2348,6 +2348,8 @@ function wfShellExec( $cmd, &$retval=null ) {
 /**
  * Executes a shell command in the background. Passes back the PID of the operation 
  *
+ * FIXME: Does not work on Windows; does not work at all (See CodeReview r55575)
+ *
  * @param $cmd String
  */
 function wfShellBackgroundExec( $cmd ){        
index ad93c0b..466038d 100644 (file)
@@ -862,7 +862,8 @@ abstract class ApiBase {
                'undo-failure' => array('code' => 'undofailure', 'info' => 'Undo failed due to conflicting intermediate edits'),
 
                //uploadMsgs
-               'invalid-session-key' => array( 'code' => 'invalid-session-key', 'info'=>'Not a valid session key' ),
+               'invalid-session-key' => array( 'code' => 'invalid-session-key', 'info' => 'Not a valid session key' ),
+               'nouploadmodule' => array( 'code' => 'nouploadmodule', 'info' => 'No upload module set' ),
        );
 
        /**
index 7377040..589a6d9 100644 (file)
@@ -30,7 +30,8 @@ if ( !defined( 'MEDIAWIKI' ) ) {
  * @ingroup API
  */
 class ApiUpload extends ApiBase {
-       var $mUpload = null;
+       protected $mUpload = null;
+       protected $mParams;
 
        public function __construct( $main, $action ) {
                parent::__construct( $main, $action );
@@ -44,9 +45,9 @@ class ApiUpload extends ApiBase {
                $request = $this->getMain()->getRequest();
 
                // do token checks:
-               if( is_null( $this->mParams['token'] ) )
+               if ( is_null( $this->mParams['token'] ) )
                        $this->dieUsageMsg( array( 'missingparam', 'token' ) );
-               if( !$wgUser->matchEditToken( $this->mParams['token'] ) )
+               if ( !$wgUser->matchEditToken( $this->mParams['token'] ) )
                        $this->dieUsageMsg( array( 'sessionfailure' ) );
 
 
@@ -54,24 +55,23 @@ class ApiUpload extends ApiBase {
                $this->mParams['file'] = $request->getFileName( 'file' );
 
                // Check whether upload is enabled
-               if( !UploadBase::isEnabled() )
+               if ( !UploadBase::isEnabled() )
                        $this->dieUsageMsg( array( 'uploaddisabled' ) );
 
-               wfDebug( __METHOD__ . "running require param\n" );
                // One and only one of the following parameters is needed
                $this->requireOnlyOneParameter( $this->mParams,
                        'sessionkey', 'file', 'url', 'enablechunks' );
 
-               if( $this->mParams['enablechunks'] ){
+               if ( $this->mParams['enablechunks'] ) {
                        // chunks upload enabled
                        $this->mUpload = new UploadFromChunks();
-                       $this->mUpload->initializeFromParams( $this->mParams, $request );
+                       $status = $this->mUpload->initializeFromParams( $this->mParams, $request );
 
-                       //if getAPIresult did not exit report the status error:
-                       if( isset( $this->mUpload->status['error'] ) )
-                               $this->dieUsageMsg( $this->mUpload->status['error'] );
+                       if( isset( $status['error'] ) )
+                               $this->dieUsageMsg( $status['error'] );
 
-               } else if( $this->mParams['internalhttpsession'] ){
+               } elseif ( $this->mParams['internalhttpsession'] ) {
+                       // TODO: Move to subclass
                        $sd = & $_SESSION['wsDownload'][ $this->mParams['internalhttpsession'] ];
 
                        wfDebug("InternalHTTP:: " . print_r($this->mParams, true));
@@ -83,56 +83,60 @@ class ApiUpload extends ApiBase {
                                filesize( $sd['target_file_path'] )
                        );
 
-                       if( !isset( $this->mUpload ) )
-                               $this->dieUsage( 'No upload module set', 'nomodule' );
-
-               } else if( $this->mParams['httpstatus'] && $this->mParams['sessionkey'] ){
+               } elseif ( $this->mParams['httpstatus'] && $this->mParams['sessionkey'] ) {
                        // return the status of the given upload session_key:
-                       if( !isset( $_SESSION['wsDownload'][ $this->mParams['sessionkey'] ] ) ){
+                       
+                       // Check the session key
+                       if( !isset( $_SESSION['wsDownload'][$this->mParams['sessionkey']] ) )
                                        return $this->dieUsageMsg( array( 'invalid-session-key' ) );
-                       }
-                       $sd = & $_SESSION['wsDownload'][$this->mParams['sessionkey']];
+                       
+                       $sd =& $_SESSION['wsDownload'][$this->mParams['sessionkey']];
                        // keep passing down the upload sessionkey
                        $statusResult = array(
                                'upload_session_key' => $this->mParams['sessionkey']
                        );
 
                        // put values into the final apiResult if available
-                       if( isset( $sd['apiUploadResult'] ) ) $statusResult['apiUploadResult'] = $sd['apiUploadResult'];
-                       if( isset( $sd['loaded'] ) ) $statusResult['loaded'] = $sd['loaded'];
-                       if( isset( $sd['content_length'] ) ) $statusResult['content_length'] = $sd['content_length'];
-
-                       return $this->getResult()->addValue( null, $this->getModuleName(),
-                                               $statusResult
-                       );
+                       if( isset( $sd['apiUploadResult'] ) ) 
+                               $statusResult['apiUploadResult'] = $sd['apiUploadResult'];
+                       if( isset( $sd['loaded'] ) ) 
+                               $statusResult['loaded'] = $sd['loaded'];
+                       if( isset( $sd['content_length'] ) ) 
+                               $statusResult['content_length'] = $sd['content_length'];
+
+                       return $this->getResult()->addValue( null, 
+                                       $this->getModuleName(), $statusResult);
+                       
                } else if( $this->mParams['sessionkey'] ) {
                        // Stashed upload
                        $this->mUpload = new UploadFromStash();
-                       $this->mUpload->initialize( $this->mParams['filename'], $_SESSION['wsUploadData'][$this->mParams['sessionkey']] );
+                       $this->mUpload->initialize( $this->mParams['filename'], 
+                                       $_SESSION['wsUploadData'][$this->mParams['sessionkey']] );
                } else {
                        // Upload from url or file
                        // Parameter filename is required
-                       if( !isset( $this->mParams['filename'] ) )
+                       if ( !isset( $this->mParams['filename'] ) )
                                $this->dieUsageMsg( array( 'missingparam', 'filename' ) );
 
                        // Initialize $this->mUpload
-                       if( isset( $this->mParams['file'] ) ) {
+                       if ( isset( $this->mParams['file'] ) ) {
                                $this->mUpload = new UploadFromFile();
                                $this->mUpload->initialize(
                                        $request->getFileName( 'file' ),
                                        $request->getFileTempName( 'file' ),
                                        $request->getFileSize( 'file' )
                                );
-                       } elseif( isset( $this->mParams['url'] ) ) {
-
+                       } elseif ( isset( $this->mParams['url'] ) ) {
                                $this->mUpload = new UploadFromUrl();
-                               $this->mUpload->initialize( $this->mParams['filename'], $this->mParams['url'], $this->mParams['asyncdownload'] );
+                               $this->mUpload->initialize( $this->mParams['filename'], 
+                                               $this->mParams['url'], $this->mParams['asyncdownload'] );
 
                                $status = $this->mUpload->fetchFile();
                                if( !$status->isOK() ){
                                        return $this->dieUsage( 'fetchfileerror', $status->getWikiText() );
                                }
-                               //check if we doing a async request set session info and return the upload_session_key)
+                               
+                               // check if we doing a async request set session info and return the upload_session_key)
                                if( $this->mUpload->isAsync() ){
                                        $upload_session_key = $status->value;
                                        // update the session with anything with the params we will need to finish up the upload later on:
@@ -159,7 +163,7 @@ class ApiUpload extends ApiBase {
                $this->doExecUpload();
        }
 
-       function doExecUpload(){
+       protected function doExecUpload(){
                global $wgUser;
                // Check whether the user has the appropriate permissions to upload anyway
                $permission = $this->mUpload->isAllowed( $wgUser );
@@ -177,57 +181,55 @@ class ApiUpload extends ApiBase {
                $this->getResult()->addValue( null, $this->getModuleName(), $result );
        }
 
-       private function performUpload() {
+       protected function performUpload() {
                global $wgUser;
                $result = array();
-               $resultDetails = null;
                $permErrors = $this->mUpload->verifyPermissions( $wgUser );
                if( $permErrors !== true ) {
-                       $result['result'] = 'Failure';
-                       $result['error'] = 'permission-denied';
-                       return $result;
+                       $this->dieUsageMsg( array( 'baddaccess-groups' ) );
                }
-               $verification = $this->mUpload->verifyUpload( $resultDetails );
-               if( $verification != UploadBase::OK ) {
+               
+               // TODO: Move them to ApiBase's message map
+               $verification = $this->mUpload->verifyUpload();
+               if( $verification['status'] !== UploadBase::OK ) {
                        $result['result'] = 'Failure';
-                       switch( $verification ) {
+                       switch( $verification['status'] ) {
                                case UploadBase::EMPTY_FILE:
-                                       $result['error'] = 'empty-file';
+                                       $this->dieUsage( 'The file you submitted was empty', 'empty-file' );
                                        break;
                                case UploadBase::FILETYPE_MISSING:
-                                       $result['error'] = 'filetype-missing';
+                                       $this->dieUsage( 'The file is missing an extension', 'filetype-missing' );
                                        break;
                                case UploadBase::FILETYPE_BADTYPE:
                                        global $wgFileExtensions;
-                                       $result['error'] = 'filetype-banned';
-                                       $result['filetype'] = $resultDetails['finalExt'];
-                                       $result['allowed-filetypes'] = $wgFileExtensions;
+                                       $this->dieUsage( 'This type of file is banned', 'filetype-banned', 
+                                                       0, array( 
+                                                               'filetype' => $verification['finalExt'],
+                                                               'allowed' => $wgFileExtensions
+                                                       ) );
                                        break;
                                case UploadBase::MIN_LENGHT_PARTNAME:
-                                       $result['error'] = 'filename-tooshort';
+                                       $this->dieUsage( 'The filename is too short', 'filename-tooshort' );
                                        break;
                                case UploadBase::ILLEGAL_FILENAME:
-                                       $result['error'] = 'illegal-filename';
-                                       $result['filename'] = $resultDetails['filtered'];
+                                       $this->dieUsage( 'The filename is not allowed', 'illegal-filename',
+                                                       0, array( 'filename' => $verification['filtered'] ) );
                                        break;
                                case UploadBase::OVERWRITE_EXISTING_FILE:
-                                       $result['error'] = 'overwrite';
+                                       $this->dieUsage( 'Overwriting an existing file is not allowed', 'overwrite' );
                                        break;
                                case UploadBase::VERIFICATION_ERROR:
-                                       $result['error'] = 'verification-error';
-                                       $args = $resultDetails['veri'];
-                                       $code = array_shift( $args );
-                                       $result['verification-error'] = $code;
-                                       $result['args'] = $args;
-                                       $this->getResult()->setIndexedTagName( $result['args'], 'arg' );
+                                       $this->getResult()->setIndexedTagName( $verification['details'], 'detail' );                                    
+                                       $this->dieUsage( 'This file did not pass file verification', 'verification-error',
+                                                       0, array( 'details' => $verification['details'] ) );
                                        break;
                                case UploadBase::UPLOAD_VERIFICATION_ERROR:
-                                       $result['error'] = 'upload-verification-error';
-                                       $result['upload-verification-error'] = $resultDetails['error'];
+                                       $this->dieUsage( "The modification you tried to make was aborted by an extension hook", 
+                                                       'hookaborted', 0, array( 'error' => $verification['error'] ) );
                                        break;
                                default:
-                                       $result['error'] = 'unknown-error';
-                                       $result['code'] = $verification;
+                                       $this->dieUsage( 'An unknown error occurred', 'unknown-error',
+                                                       0, array( 'code' =>  $verification['status'] ) );
                                        break;
                        }
                        return $result;
@@ -236,36 +238,39 @@ class ApiUpload extends ApiBase {
                if( !$this->mParams['ignorewarnings'] ) {
                        $warnings = $this->mUpload->checkWarnings();
                        if( $warnings ) {
+                               
+                               // Add indices
                                $this->getResult()->setIndexedTagName( $warnings, 'warning' );
-                               //also add index to duplicate:
-                               if(isset($warnings['duplicate']))
+                               
+                               if( isset( $warnings['duplicate'] ) )
                                        $this->getResult()->setIndexedTagName( $warnings['duplicate'], 'duplicate');
 
-                               if(isset($warnings['exists']))
-                                       $this->getResult()->setIndexedTagName( $warnings['exists'], 'exists');
-
+                               if( isset( $warnings['exists'] ) )
+                                       $this->getResult()->setIndexedTagName( $warnings['exists'], 'exists' );
+                               
+                               if( isset( $warnings['filewasdeleted'] ) )
+                                       $warnings['filewasdeleted'] = $warnings['filewasdeleted']->getDBkey();
+                                       
                                $result['result'] = 'Warning';
                                $result['warnings'] = $warnings;
-                               if( isset( $result['filewasdeleted'] ) )
-                                       $result['filewasdeleted'] = $result['filewasdeleted']->getDBkey();
 
                                $sessionKey = $this->mUpload->stashSession();
-                               if( $sessionKey )
-                                       $result['sessionkey'] = $sessionKey;
+                               if ( !$sessionKey ) 
+                                       $this->dieUsage( 'Stashing temporary file failed', 'stashfailed' );
+                               $result['sessionkey'] = $sessionKey;
                                return $result;
                        }
                }
 
-               // do the upload
+               // No errors, no warnings: do the upload
                $status = $this->mUpload->performUpload( $this->mParams['comment'],
                        $this->mParams['comment'], $this->mParams['watch'], $wgUser );
 
                if( !$status->isGood() ) {
-                       $result['result'] = 'Failure';
-                       $result['error'] = 'internal-error';
-                       $result['details'] = $status->getErrorsArray();
+                       $error = $status->getErrorsArray();
                        $this->getResult()->setIndexedTagName( $result['details'], 'error' );
-                       return $result;
+                       
+                       $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error );                   
                }
 
                $file = $this->mUpload->getLocalFile();
@@ -277,9 +282,7 @@ class ApiUpload extends ApiBase {
                //get all the image properties:
                $imParam = ApiQueryImageInfo::getPropertyNames();
         $result['imageinfo'] = ApiQueryImageInfo::getInfo( $file,
-                       array_flip( $imParam ),
-            $this->getResult() );
-
+                               array_flip( $imParam ), $this->getResult() );
 
                return $result;
        }
@@ -307,7 +310,7 @@ class ApiUpload extends ApiBase {
                        'chunk' => null,
                        'done' => false,
                        'url' => null,
-                       'asyncdownload' => false,
+                       /*'asyncdownload' => false,*/ // btongminh: Disabled pending fixing wfShellBackgroundExec
                        'httpstatus' => false,
                        'sessionkey' => null,
                        'internalhttpsession' => null,
index bb734e2..81d6101 100644 (file)
@@ -226,8 +226,8 @@ class UploadForm extends SpecialPage {
 
                        case UploadBase::VERIFICATION_ERROR:
                                unset( $details['status'] );
-                               $code = array_shift( $details );
-                               $this->uploadError( wfMsgExt( $code, 'parseinline', $details ) );
+                               $code = array_shift( $details['details'] );
+                               $this->uploadError( wfMsgExt( $code, 'parseinline', $details['details'] ) );
                                break;
 
                        case UploadBase::UPLOAD_VERIFICATION_ERROR:
@@ -284,7 +284,7 @@ class UploadForm extends SpecialPage {
 
                // Check whether this is a sane upload
                $result = $this->mUpload->verifyUpload();
-               if( $result != UploadBase::OK )
+               if( $result['status'] != UploadBase::OK )
                        return $result;
 
                $this->mLocalFile = $this->mUpload->getLocalFile();
index aeca23d..ce188ab 100644 (file)
@@ -18,6 +18,7 @@ abstract class UploadBase {
        protected $mDesiredDestName, $mDestName, $mRemoveTempFile, $mSourceType;
        protected $mTitle = false, $mTitleError = 0;
        protected $mFilteredName, $mFinalExtension;
+       protected $mLocalFile;
 
        const SUCCESS = 0;
        const OK = 0;
@@ -175,17 +176,19 @@ abstract class UploadBase {
                if( $verification !== true ) {
                        if( !is_array( $verification ) )
                                $verification = array( $verification );
-                       $verification['status'] = self::VERIFICATION_ERROR;
-                       return $verification;
+                       return array( 'status' => self::VERIFICATION_ERROR,
+                                       'details' => $verification );
+
                }
 
                $error = '';
                if( !wfRunHooks( 'UploadVerification',
                                array( $this->mDestName, $this->mTempPath, &$error ) ) ) {
+                       // This status needs another name...
                        return array( 'status' => self::UPLOAD_VERIFICATION_ERROR, 'error' => $error );
                }
 
-               return self::OK;
+               return array( 'status' => self::OK );
        }
 
        /**
@@ -202,7 +205,7 @@ abstract class UploadBase {
 
                #magically determine mime type
                $magic = MimeMagic::singleton();
-               $mime = $magic->guessMimeType( $this->mTempFile, false );
+               $mime = $magic->guessMimeType( $this->mTempPath, false );
 
                #check mime type, if desired
                global $wgVerifyMimeType;
@@ -212,11 +215,11 @@ abstract class UploadBase {
                                return array( 'filetype-badmime', $mime );
 
                        # Check IE type
-                       $fp = fopen( $this->mTempFile, 'rb' );
+                       $fp = fopen( $this->mTempPath, 'rb' );
                        $chunk = fread( $fp, 256 );
                        fclose( $fp );
                        $extMime = $magic->guessTypesForExtension( $this->mFinalExtension );
-                       $ieTypes = $magic->getIEMimeTypes( $tmpfile, $chunk, $extMime );
+                       $ieTypes = $magic->getIEMimeTypes( $this->mTempPath, $chunk, $extMime );
                        foreach ( $ieTypes as $ieType ) {
                                if ( $this->checkFileExtension( $ieType, $wgMimeTypeBlacklist ) ) {
                                        return array( 'filetype-bad-ie-mime', $ieType );
@@ -225,11 +228,11 @@ abstract class UploadBase {
                }
 
                #check for htmlish code and javascript
-               if( self::detectScript( $tmpfile, $mime, $this->mFinalExtension ) ) {
+               if( self::detectScript( $this->mTempPath, $mime, $this->mFinalExtension ) ) {
                        return 'uploadscripted';
                }
                if( $this->mFinalExtension == 'svg' || $mime == 'image/svg+xml' ) {
-                       if( self::detectScriptInSvg( $tmpfile ) ) {
+                       if( self::detectScriptInSvg( $this->mTempPath ) ) {
                                return 'uploadscripted';
                        }
                }
@@ -237,7 +240,7 @@ abstract class UploadBase {
                /**
                * Scan the uploaded file for viruses
                */
-               $virus = $this->detectVirus( $tmpfile );
+               $virus = $this->detectVirus( $this->mTempPath );
                if ( $virus ) {
                        return array( 'uploadvirus', $virus );
                }
index 3ac5401..956a398 100644 (file)
@@ -79,7 +79,7 @@ class UploadFromChunks extends UploadBase {
        function verifyUpload() {
                // no checks on chunk upload mode:
                if( $this->chunk_mode ==  UploadFromChunks::INIT )
-                       return self::OK;
+                       return array( 'status' => self::OK );
 
                // verify on init and last chunk request
                if(     $this->chunk_mode == UploadFromChunks::CHUNK ||
index 1bc09dd..c5458da 100644 (file)
@@ -62,11 +62,6 @@ class UploadFromUrl extends UploadBase {
         * @param $request Object: WebRequest object
         */
        public function initializeFromRequest( &$request ) {
-
-               // set dl mode if not set:
-               if( !$this->dl_mode )
-                       $this->dl_mode = Http::SYNC_DOWNLOAD;
-
                $desiredDestName = $request->getText( 'wpDestFile' );
                if( !$desiredDestName )
                        $desiredDestName = $request->getText( 'wpUploadFile' );