From 234f54a88ef53c0a7771d86258888193ad7f9d8f Mon Sep 17 00:00:00 2001 From: "Mark A. Hershberger" Date: Tue, 30 Mar 2010 19:10:10 +0000 Subject: [PATCH] * Set $titleObj to null by default on getWatchlistValue since it often isn't needed & check that it is set when it is needed. (follow up r64197). * Refactoring ApiUpload & UploadBase to make it easier to extend & read. * Use a class constant for the upload session key instead of a hard-coded-across-several-files value. * Add UploadBase::appendToUploadFile() method to enable protocols that do incremental upload. --- includes/api/ApiBase.php | 7 +- includes/api/ApiUpload.php | 143 ++++++++++++++++++++------------- includes/upload/UploadBase.php | 72 +++++++++++++---- 3 files changed, 149 insertions(+), 73 deletions(-) diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php index c8ecd5be91..6beef9e693 100644 --- a/includes/api/ApiBase.php +++ b/includes/api/ApiBase.php @@ -535,7 +535,7 @@ abstract class ApiBase { /** * Handle watchlist settings */ - protected function getWatchlistValue ( $watchlist, $titleObj ) { + protected function getWatchlistValue ( $watchlist, $titleObj = null ) { switch ( $watchlist ) { case 'watch': return true; @@ -543,7 +543,10 @@ abstract class ApiBase { return false; case 'preferences': global $wgUser; - if ( $titleObj->exists() && $wgUser->getOption( 'watchdefault' ) && !$titleObj->userIsWatching() ) { + if ( isset($titleObj) + && $titleObj->exists() + && $wgUser->getOption( 'watchdefault' ) + && !$titleObj->userIsWatching() ) { return true; } return null; diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 0fb17ccbd9..f81ea24cff 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -60,14 +60,14 @@ class ApiUpload extends ApiBase { * Upload stashed in a previous request */ // Check the session key - if ( !isset( $_SESSION['wsUploadData'][$this->mParams['sessionkey']] ) ) { + if ( !isset( $_SESSION[UploadBase::getSessionKey()][$this->mParams['sessionkey']] ) ) { $this->dieUsageMsg( array( 'invalid-session-key' ) ); } $this->mUpload = new UploadFromStash(); $this->mUpload->initialize( $this->mParams['filename'], $this->mParams['sessionkey'], - $_SESSION['wsUploadData'][$this->mParams['sessionkey']] ); + $_SESSION[UploadBase::getSessionKey()][$this->mParams['sessionkey']] ); } elseif ( isset( $this->mParams['filename'] ) ) { /** * Upload from URL, etc. @@ -127,59 +127,73 @@ class ApiUpload extends ApiBase { $this->getResult()->addValue( null, $this->getModuleName(), $result ); } - protected function performUpload() { - global $wgUser; - $result = array(); - $permErrors = $this->mUpload->verifyPermissions( $wgUser ); - if ( $permErrors !== true ) { - $this->dieUsageMsg( array( 'badaccess-groups' ) ); + /** + * Performs file verification, dies on error. + * + * @param $flag integer passed to UploadBase::verifyUpload, set to + * UploadBase::EMPTY_FILE to skip the empty file check. + */ + public function verifyUpload( $flag ) { + $verification = $this->mUpload->verifyUpload( $flag ); + if ( $verification['status'] === UploadBase::OK ) { + return $verification; } + $this->getVerificationError( $verification ); + } + + /** + * Produce the usage error + * + * @param $verification array an associative array with the status + * key + */ + public function getVerificationError( $verification ) { // TODO: Move them to ApiBase's message map - $verification = $this->mUpload->verifyUpload(); - if ( $verification['status'] !== UploadBase::OK ) { - $result['result'] = 'Failure'; - switch( $verification['status'] ) { - case UploadBase::EMPTY_FILE: - $this->dieUsage( 'The file you submitted was empty', 'empty-file' ); - break; - case UploadBase::FILETYPE_MISSING: - $this->dieUsage( 'The file is missing an extension', 'filetype-missing' ); - break; - case UploadBase::FILETYPE_BADTYPE: - global $wgFileExtensions; - $this->dieUsage( 'This type of file is banned', 'filetype-banned', - 0, array( - 'filetype' => $verification['finalExt'], - 'allowed' => $wgFileExtensions - ) ); - break; - case UploadBase::MIN_LENGTH_PARTNAME: - $this->dieUsage( 'The filename is too short', 'filename-tooshort' ); - break; - case UploadBase::ILLEGAL_FILENAME: - $this->dieUsage( 'The filename is not allowed', 'illegal-filename', - 0, array( 'filename' => $verification['filtered'] ) ); - break; - case UploadBase::OVERWRITE_EXISTING_FILE: - $this->dieUsage( 'Overwriting an existing file is not allowed', 'overwrite' ); - break; - case UploadBase::VERIFICATION_ERROR: - $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::HOOK_ABORTED: - $this->dieUsage( "The modification you tried to make was aborted by an extension hook", - 'hookaborted', 0, array( 'error' => $verification['error'] ) ); - break; - default: - $this->dieUsage( 'An unknown error occurred', 'unknown-error', - 0, array( 'code' => $verification['status'] ) ); - break; - } - return $result; + switch( $verification['status'] ) { + case UploadBase::EMPTY_FILE: + $this->dieUsage( 'The file you submitted was empty', 'empty-file' ); + break; + case UploadBase::FILETYPE_MISSING: + $this->dieUsage( 'The file is missing an extension', 'filetype-missing' ); + break; + case UploadBase::FILETYPE_BADTYPE: + global $wgFileExtensions; + $this->dieUsage( 'This type of file is banned', 'filetype-banned', + 0, array( + 'filetype' => $verification['finalExt'], + 'allowed' => $wgFileExtensions + ) ); + break; + case UploadBase::MIN_LENGTH_PARTNAME: + $this->dieUsage( 'The filename is too short', 'filename-tooshort' ); + break; + case UploadBase::ILLEGAL_FILENAME: + $this->dieUsage( 'The filename is not allowed', 'illegal-filename', + 0, array( 'filename' => $verification['filtered'] ) ); + break; + case UploadBase::OVERWRITE_EXISTING_FILE: + $this->dieUsage( 'Overwriting an existing file is not allowed', 'overwrite' ); + break; + case UploadBase::VERIFICATION_ERROR: + $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::HOOK_ABORTED: + $this->dieUsage( "The modification you tried to make was aborted by an extension hook", + 'hookaborted', 0, array( 'error' => $verification['error'] ) ); + break; + default: + $this->dieUsage( 'An unknown error occurred', 'unknown-error', + 0, array( 'code' => $verification['status'] ) ); + break; } + } + + protected function checkForWarnings() { + $result = array(); + if ( !$this->mParams['ignorewarnings'] ) { $warnings = $this->mUpload->checkWarnings(); if ( $warnings ) { @@ -213,19 +227,34 @@ class ApiUpload extends ApiBase { return $result; } } + return; + } + + protected function performUpload() { + global $wgUser; + $permErrors = $this->mUpload->verifyPermissions( $wgUser ); + if ( $permErrors !== true ) { + $this->dieUsageMsg( array( 'badaccess-groups' ) ); + } + + $this->verifyUpload(); + + $warnings = $this->checkForWarnings(); + if( isset($warnings) ) return $warnings; // Use comment as initial page text by default if ( is_null( $this->mParams['text'] ) ) { $this->mParams['text'] = $this->mParams['comment']; } - - $watch = $this->getWatchlistValue( $params['watchlist'] ); - + + $file = $this->mUpload->getLocalFile(); + $watch = $this->getWatchlistValue( $params['watchlist'], $file->getTitle() ); + // Deprecated parameters if ( $this->mParams['watch'] ) { $watch = true; } - + // No errors, no warnings: do the upload $status = $this->mUpload->performUpload( $this->mParams['comment'], $this->mParams['text'], $watch, $wgUser ); @@ -236,9 +265,9 @@ class ApiUpload extends ApiBase { $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error ); } - + $file = $this->mUpload->getLocalFile(); - + $result['result'] = 'Success'; $result['filename'] = $file->getName(); $result['imageinfo'] = $this->mUpload->getImageInfo( $this->getResult() ); diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 94fda9b075..4f0522ef5d 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -33,6 +33,11 @@ abstract class UploadBase { const HOOK_ABORTED = 11; const SESSION_VERSION = 2; + const SESSION_KEYNAME = 'wsUploadData'; + + static public function getSessionKeyname() { + return self::SESSION_KEYNAME; + } /** * Returns true if uploads are enabled. @@ -143,12 +148,34 @@ abstract class UploadBase { } /** - * Return the file size + * Return true if the file is empty + * @return bool */ public function isEmptyFile() { return empty( $this->mFileSize ); } + /** + * Return the file size + * @return integer + */ + public function getFileSize() { + return $this->mFileSize; + } + + /** + * Append a file to the Repo file + * + * @param string $srcPath Path to source file + * @param string $toAppendPath Path to the Repo file that will be appended to. + * @return Status Status + */ + protected function appendToUploadFile( $srcPath, $toAppendPath ) { + $repo = RepoGroup::singleton()->getLocalRepo(); + $status = $repo->append( $srcPath, $toAppendPath ); + return $status; + } + /** * @param $srcPath String: the source path * @return the real path if it was a virtual URL @@ -163,9 +190,9 @@ abstract class UploadBase { /** * Verify whether the upload is sane. - * Returns self::OK or else an array with error information + * @return mixed self::OK or else an array with error information */ - public function verifyUpload() { + public function verifyUpload( ) { /** * If there was no filename or a zero size given, give up quick. */ @@ -189,6 +216,31 @@ abstract class UploadBase { ); } + /** + * Make sure this file can be created + */ + $result = $this->validateNameAndOverwrite(); + if( $result !== true ) { + return $result; + } + + $error = ''; + if( !wfRunHooks( 'UploadVerification', + array( $this->mDestName, $this->mTempPath, &$error ) ) ) { + // @fixme This status needs another name... + return array( 'status' => self::HOOK_ABORTED, 'error' => $error ); + } + + return array( 'status' => self::OK ); + } + + /** + * Verify that the name is valid and, if necessary, that we can overwrite + * + * @return mixed true if valid, otherwise and array with 'status' + * and other keys + **/ + public function validateNameAndOverwrite() { $nt = $this->getTitle(); if( is_null( $nt ) ) { $result = array( 'status' => $this->mTitleError ); @@ -212,15 +264,7 @@ abstract class UploadBase { 'overwrite' => $overwrite ); } - - $error = ''; - if( !wfRunHooks( 'UploadVerification', - array( $this->mDestName, $this->mTempPath, &$error ) ) ) { - // This status needs another name... - return array( 'status' => self::HOOK_ABORTED, 'error' => $error ); - } - - return array( 'status' => self::OK ); + return true; } /** @@ -511,7 +555,7 @@ abstract class UploadBase { } $key = $this->getSessionKey(); - $_SESSION['wsUploadData'][$key] = array( + $_SESSION[self::SESSION_KEYNAME][$key] = array( 'mTempPath' => $status->value, 'mFileSize' => $this->mFileSize, 'mFileProps' => $this->mFileProps, @@ -525,7 +569,7 @@ abstract class UploadBase { */ protected function getSessionKey() { $key = mt_rand( 0, 0x7fffffff ); - $_SESSION['wsUploadData'][$key] = array(); + $_SESSION[self::SESSION_KEYNAME][$key] = array(); return $key; } -- 2.20.1