From c2033042ee59ac9cbcc62fda1cd8aa0123ed65ae Mon Sep 17 00:00:00 2001 From: Bryan Tong Minh Date: Wed, 26 Aug 2009 17:05:24 +0000 Subject: [PATCH] * Code style & commenting on upload functions. * Move isValidURI from UploadFromUrl to HttpFunctions. * Made some functions in UploadBase static. --- docs/upload.txt | 43 +------ includes/HttpFunctions.php | 16 ++- includes/upload/UploadBase.php | 175 ++++++++++++++++++--------- includes/upload/UploadFromChunks.php | 5 + includes/upload/UploadFromFile.php | 10 +- includes/upload/UploadFromStash.php | 21 ++-- includes/upload/UploadFromUrl.php | 55 ++++----- 7 files changed, 187 insertions(+), 138 deletions(-) diff --git a/docs/upload.txt b/docs/upload.txt index 7c20c6931a..a0f0a594ce 100644 --- a/docs/upload.txt +++ b/docs/upload.txt @@ -1,41 +1,2 @@ -Special:Upload: - -wfSpecialUpload - new UploadForm - mUpload = new UploadFrom... - execute() - $wgEnableUploads - isAllowed(upload) - isBlocked() - wfReadOnly() - processUpload() - internalProcessUpload() - wfRunHooks(UploadForm:BeforeProcessing) - mUpload->getTitle() - wfStripIllegalFilenameChars - splitExtensions() - checkFileExtension() - Title::makeTitleSafe - getUserPermissionsErrors(edit; upload; create) - mUpload->verifyUpload() - empty(mFileSize) - getTitle() - checkOverwrite() - fetchFile() - verifyFile() - checkMacBinary() - wfRunHooks(UploadVerification) - if(!ignoreWarning) mUpload->checkWarnings() - getInitialPageText() - mUpload->performUpload() - mLocalFile->upload() - if(isGood() && $watch) addWatch() - if(isGood()) wfRunHooks(UploadComplete) - wfRunHooks(SpecialUploadComplete) - -Changes: - * "Your file will be renamed to $1" check now done on the result of - Title::makeTitleSafe instead of filteredName - * getExistWarning only really does existence checks - * Other stuff forgotten to be documented - \ No newline at end of file +This document describes how the current uploading system is build up and how +custom backends can be built. (At least someday it will). diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php index af2a049782..8f51883fb7 100644 --- a/includes/HttpFunctions.php +++ b/includes/HttpFunctions.php @@ -54,7 +54,7 @@ class Http { // check for redirects: if( isset( $head['Location'] ) && strrpos( $head[0], '302' ) !== false ){ if( $redirectCount < $wgMaxRedirects ){ - if( UploadFromUrl::isValidURI( $head['Location'] ) ){ + if( self::isValidURI( $head['Location'] ) ){ return self::doDownload( $head['Location'], $target_file_path, $dl_mode, $redirectCount++ ); } else { return Status::newFatal( 'upload-proto-error' ); @@ -272,6 +272,18 @@ class Http { global $wgVersion; return "MediaWiki/$wgVersion"; } + + /** + * Checks that the given URI is a valid one + * @param $uri Mixed: URI to check for validity + */ + public static function isValidURI( $uri ){ + return preg_match( + '/(ftp|http|https):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/', + $uri, + $matches + ); + } } class HttpRequest { @@ -322,7 +334,7 @@ class HttpRequest { */ public function doRequest() { # Make sure we have a valid url - if( !UploadFromUrl::isValidURI( $this->url ) ) + if( !Http::isValidURI( $this->url ) ) return Status::newFatal('bad-url'); # Use curl if available diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 640126202f..52bb2487dd 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -1,10 +1,23 @@ isAllowed( 'upload' ) ) return 'upload'; return true; @@ -58,21 +71,24 @@ class UploadBase { /** * Create a form of UploadBase depending on wpSourceType and initializes it */ - static function createFromRequest( &$request, $type = null ) { + public static function createFromRequest( &$request, $type = null ) { $type = $type ? $type : $request->getVal( 'wpSourceType' ); if( !$type ) return null; + // Get the upload class $type = ucfirst( $type ); $className = 'UploadFrom' . $type; wfDebug( __METHOD__ . ": class name: $className\n" ); if( !in_array( $type, self::$uploadHandlers ) ) return null; + // Check whether this upload class is enabled if( !call_user_func( array( $className, 'isEnabled' ) ) ) return null; + // Check whether the request is valid if( !call_user_func( array( $className, 'isValidRequest' ), $request ) ) return null; @@ -85,33 +101,38 @@ class UploadBase { /** * Check whether a request if valid for this handler */ - static function isValidRequest( $request ) { + public static function isValidRequest( $request ) { return false; } - function __construct() {} + public function __construct() {} /** * Do the real variable initialization */ - function initialize( $name, $tempPath, $fileSize, $removeTempFile = false ) { + public function initialize( $name, $tempPath, $fileSize, $removeTempFile = false ) { $this->mDesiredDestName = $name; $this->mTempPath = $tempPath; $this->mFileSize = $fileSize; $this->mRemoveTempFile = $removeTempFile; } + + /** + * Initialize from a WebRequest. Override this in a subclass. + */ + public abstract function initializeFromRequest( &$request ); /** * Fetch the file. Usually a no-op */ - function fetchFile() { + public function fetchFile() { return Status::newGood(); } /** * Return the file size */ - function isEmptyFile(){ + public function isEmptyFile(){ return empty( $this->mFileSize ); } @@ -119,7 +140,7 @@ class UploadBase { * Verify whether the upload is sane. * Returns self::OK or else an array with error information */ - function verifyUpload() { + public function verifyUpload() { /** * If there was no filename or a zero size given, give up quick. */ @@ -135,8 +156,7 @@ class UploadBase { $result['finalExt'] = $this->mFinalExtension; return $result; } - $this->mLocalFile = wfLocalFile( $nt ); - $this->mDestName = $this->mLocalFile->getName(); + $this->mDestName = $this->getLocalFile()->getName(); /** * In some cases we may forbid overwriting of existing files. @@ -171,7 +191,7 @@ class UploadBase { /** * Verifies that it's ok to include the uploaded file * - * this function seems to intermixes tmpfile and $this->mTempPath .. no idea why this is + * FIXME: this function seems to intermixes tmpfile and $this->mTempPath .. no idea why this is * * @param string $tmpfile the full path of the temporary file to verify * @return mixed true of the file is verified, a string or array otherwise. @@ -186,7 +206,7 @@ class UploadBase { #check mime type, if desired global $wgVerifyMimeType; - if ($wgVerifyMimeType) { + if ( $wgVerifyMimeType ) { global $wgMimeTypeBlacklist; if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) return array( 'filetype-badmime', $mime ); @@ -205,11 +225,11 @@ class UploadBase { } #check for htmlish code and javascript - if( $this->detectScript( $tmpfile, $mime, $this->mFinalExtension ) ) { + if( self::detectScript( $tmpfile, $mime, $this->mFinalExtension ) ) { return 'uploadscripted'; } if( $this->mFinalExtension == 'svg' || $mime == 'image/svg+xml' ) { - if( $this->detectScriptInSvg( $tmpfile ) ) { + if( self::detectScriptInSvg( $tmpfile ) ) { return 'uploadscripted'; } } @@ -226,9 +246,13 @@ class UploadBase { } /** - * Check whether the user can edit, upload and create the image + * Check whether the user can edit, upload and create the image. + * + * @param User $user the user to verify the permissions against + * @return mixed An array as returned by getUserPermissionsErrors or true + * in case the user has proper permissions. */ - function verifyPermissions( $user ) { + public function verifyPermissions( $user ) { /** * If the image is protected, non-sysop users won't be able * to modify it by uploading a new revision. @@ -249,11 +273,14 @@ class UploadBase { /** * Check for non fatal problems with the file + * + * @return array Array of warnings */ - function checkWarnings() { + public function checkWarnings() { $warning = array(); - $filename = $this->mLocalFile->getName(); + $localFile = $this->getLocalFile(); + $filename = $localFile->getName(); $n = strrpos( $filename, '.' ); $partname = $n ? substr( $filename, 0, $n ) : $filename; @@ -284,14 +311,14 @@ class UploadBase { $warning['emptyfile'] = true; - $exists = self::getExistsWarning( $this->mLocalFile ); + $exists = self::getExistsWarning( $localFile ); if( $exists !== false ) $warning['exists'] = $exists; // Check whether this may be a thumbnail if( $exists !== false && $exists[0] != 'thumb' - && self::isThumbName( $this->mLocalFile->getName() ) ){ - //make the title: + && self::isThumbName( $filename ) ){ + // Make the title $nt = $this->getTitle(); $warning['file-thumbnail-no'] = substr( $filename, 0, strpos( $nt->getText() , '-' ) +1 ); @@ -324,22 +351,25 @@ class UploadBase { # If the file existed before and was deleted, warn the user of this # Don't bother doing so if the file exists now, however - if( $this->mLocalFile->wasDeleted() && !$this->mLocalFile->exists() ) - $warning['filewasdeleted'] = $this->mLocalFile->getTitle(); + if( $localFile->wasDeleted() && !$localFile->exists() ) + $warning['filewasdeleted'] = $localFile->getTitle(); return $warning; } /** - * Really perform the upload. + * Really perform the upload. Stores the file in the local repo, watches + * if necessary and runs the UploadComplete hook. + * + * @return mixed Status indicating the whether the upload succeeded. */ - function performUpload( $comment, $pageText, $watch, $user ) { + public function performUpload( $comment, $pageText, $watch, $user ) { wfDebug( "\n\n\performUpload: sum:" . $comment . ' c: ' . $pageText . ' w:' . $watch ); - $status = $this->mLocalFile->upload( $this->mTempPath, $comment, $pageText, + $status = $this->getLocalFile()->upload( $this->mTempPath, $comment, $pageText, File::DELETE_SOURCE, $this->mFileProps, false, $user ); if( $status->isGood() && $watch ) - $user->addWatch( $this->mLocalFile->getTitle() ); + $user->addWatch( $this->getLocalFile()->getTitle() ); if( $status->isGood() ) wfRunHooks( 'UploadComplete', array( &$this ) ); @@ -348,9 +378,12 @@ class UploadBase { } /** - * Returns a title or null + * Returns the title of the file to be uploaded. Sets mTitleError in case + * the name was illegal. + * + * @return Title The title of the file or null in case the name was illegal */ - function getTitle() { + public function getTitle() { if ( $this->mTitle !== false ) return $this->mTitle; @@ -415,7 +448,10 @@ class UploadBase { return $this->mTitle = $nt; } - function getLocalFile() { + /** + * Return the local file and initializes if necessary. + */ + public function getLocalFile() { if( is_null( $this->mLocalFile ) ) { $nt = $this->getTitle(); $this->mLocalFile = is_null( $nt ) ? null : wfLocalFile( $nt ); @@ -435,14 +471,20 @@ class UploadBase { * @return string - full path the stashed file, or false on failure * @access private */ - function saveTempUploadedFile( $saveName, $tempName ) { + protected function saveTempUploadedFile( $saveName, $tempName ) { $repo = RepoGroup::singleton()->getLocalRepo(); $status = $repo->storeTemp( $saveName, $tempName ); return $status; } - /* append to a stashed file */ - function appendToUploadFile( $srcPath, $toAppendPath ){ + /** + * Append a file to a stashed file. + * + * @param string $srcPath Path to file to append from + * @param string $toAppendPath Path to file to append to + * @return Status Status + */ + public function appendToUploadFile( $srcPath, $toAppendPath ){ $repo = RepoGroup::singleton()->getLocalRepo(); $status = $repo->append( $srcPath, $toAppendPath ); return $status; @@ -454,21 +496,19 @@ class UploadBase { * Returns a key value which will be passed through a form * to pick up the path info on a later invocation. * - * @return int - * @access private + * @return int Session key */ - function stashSession() { + public function stashSession() { $status = $this->saveTempUploadedFile( $this->mDestName, $this->mTempPath ); if( !$status->isOK() ) { # Couldn't save the file. return false; } - $mTempPath = $status->value; if(!isset($_SESSION)) session_start(); // start up the session (might have been previously closed to prevent php session locking) $key = $this->getSessionKey(); $_SESSION['wsUploadData'][$key] = array( - 'mTempPath' => $mTempPath, + 'mTempPath' => $status->value, 'mFileSize' => $this->mFileSize, 'mFileProps' => $this->mFileProps, 'version' => self::SESSION_VERSION, @@ -477,9 +517,9 @@ class UploadBase { } /** - * Pull session key gen from stash in cases where we want to start an upload without much information + * Generate a random session key from stash in cases where we want to start an upload without much information */ - function getSessionKey(){ + protected function getSessionKey(){ $key = mt_rand( 0, 0x7fffffff ); $_SESSION['wsUploadData'][$key] = array(); return $key; @@ -489,7 +529,7 @@ class UploadBase { * Remove a temporarily kept file stashed by saveTempUploadedFile(). * @return success */ - function unsaveUploadedFile() { + public function unsaveUploadedFile() { $repo = RepoGroup::singleton()->getLocalRepo(); $success = $repo->freeTemp( $this->mTempPath ); return $success; @@ -500,14 +540,14 @@ class UploadBase { * on exit to clean up. * @access private */ - function cleanupTempFile() { + public function cleanupTempFile() { if ( $this->mRemoveTempFile && $this->mTempPath && file_exists( $this->mTempPath ) ) { wfDebug( __METHOD__ . ": Removing temporary file {$this->mTempPath}\n" ); unlink( $this->mTempPath ); } } - function getTempPath() { + public function getTempPath() { return $this->mTempPath; } @@ -602,7 +642,7 @@ class UploadBase { * @param string $extension The extension of the file * @return bool true if the file contains something looking like embedded scripts */ - function detectScript( $file, $mime, $extension ) { + public static function detectScript( $file, $mime, $extension ) { global $wgAllowTitlesInSVG; #ugly hack: for text files, always look at the entire file. @@ -700,7 +740,7 @@ class UploadBase { return false; } - function detectScriptInSvg( $filename ) { + protected function detectScriptInSvg( $filename ) { $check = new XmlTypeCheck( $filename, array( $this, 'checkSvgScriptCallback' ) ); return $check->filterMatch; } @@ -708,7 +748,7 @@ class UploadBase { /** * @todo Replace this with a whitelist filter! */ - function checkSvgScriptCallback( $element, $attribs ) { + public function checkSvgScriptCallback( $element, $attribs ) { $stripped = $this->stripXmlNamespace( $element ); if( $stripped == 'script' ) { @@ -745,7 +785,7 @@ class UploadBase { * or a string containing feedback from the virus scanner if a virus was found. * If textual feedback is missing but a virus was found, this function returns true. */ - function detectVirus( $file ) { + public static function detectVirus( $file ) { global $wgAntivirus, $wgAntivirusSetup, $wgAntivirusRequired, $wgOut; if ( !$wgAntivirus ) { @@ -843,7 +883,7 @@ class UploadBase { * * @access private */ - function checkMacBinary() { + private function checkMacBinary() { $macbin = new MacBinary( $this->mTempPath ); if( $macbin->isValid() ) { $dataFile = tempnam( wfTempDir(), 'WikiMacBinary' ); @@ -865,18 +905,19 @@ class UploadBase { * Check if there's an overwrite conflict and, if so, if restrictions * forbid this user from performing the upload. * - * @return mixed true on success, WikiError on failure + * @return mixed true on success, error string on failure * @access private */ - function checkOverwrite() { + private function checkOverwrite() { global $wgUser; // First check whether the local file can be overwritten - if( $this->mLocalFile->exists() ) - if( !self::userCanReUpload( $wgUser, $this->mLocalFile ) ) + $file = $this->getLocalFile(); + if( $file->exists() ) + if( !self::userCanReUpload( $wgUser, $file ) ) return 'fileexists-forbidden'; // Check shared conflicts - $file = wfFindFile( $this->mLocalFile->getName() ); + $file = wfFindFile( $file->getName() ); if ( $file && ( !$wgUser->isAllowed( 'reupload' ) || !$wgUser->isAllowed( 'reupload-shared' ) ) ) return 'fileexists-shared-forbidden'; @@ -904,6 +945,17 @@ class UploadBase { return $user->getId() == $img->getUser( 'id' ); } + /** + * Helper function that does various existence checks for a file. + * The following checks are performed: + * - The file exists + * - Article with the same name as the file exists + * - File exists with normalized extension + * - The file looks like a thumbnail and the original exists + * + * @param File $file The file to check + * @return mixed False if the file does not exists, else an array + */ public static function getExistsWarning( $file ) { if( $file->exists() ) return array( 'exists', $file ); @@ -944,6 +996,9 @@ class UploadBase { return false; } + /** + * Helper function that checks whether the filename looks like a thumbnail + */ public static function isThumbName( $filename ) { $n = strrpos( $filename, '.' ); $partname = $n ? substr( $filename, 0, $n ) : $filename; diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index ce22b6549b..a77ac7f45d 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -1,5 +1,10 @@ getText( 'wpDestFile' ); if( !$desiredDestName ) diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php index 22f8e9b26b..06eb163e8f 100644 --- a/includes/upload/UploadFromStash.php +++ b/includes/upload/UploadFromStash.php @@ -1,8 +1,15 @@ getSessionData( 'wsUploadData' ); return self::isValidSessionKey( $request->getInt( 'wpSessionKey' ), @@ -20,7 +27,7 @@ class UploadFromStash extends UploadBase { /* * some $na vars for uploadBase method compatibility. */ - function initialize( $name, $sessionData, $na, $na2=false ) { + public function initialize( $name, $sessionData, $na, $na2=false ) { /** * Confirming a temporarily stashed upload. * We don't want path names to be forged, so we keep @@ -36,7 +43,7 @@ class UploadFromStash extends UploadBase { $this->mFileProps = $sessionData['mFileProps']; } - function initializeFromRequest( &$request ) { + public function initializeFromRequest( &$request ) { $sessionKey = $request->getInt( 'wpSessionKey' ); $sessionData = $request->getSessionData('wsUploadData'); @@ -56,7 +63,7 @@ class UploadFromStash extends UploadBase { /** * We're here from "ignore warnings anyway" so return just OK */ - function checkWarnings() { + public function checkWarnings() { return array(); } diff --git a/includes/upload/UploadFromUrl.php b/includes/upload/UploadFromUrl.php index b543070acd..1bc09dd072 100644 --- a/includes/upload/UploadFromUrl.php +++ b/includes/upload/UploadFromUrl.php @@ -1,5 +1,13 @@ isAllowed( 'upload_by_url' ) ) return 'upload_by_url'; return parent::isAllowed( $user ); @@ -18,13 +27,15 @@ class UploadFromUrl extends UploadBase { /** * Checks if the upload from URL feature is enabled */ - static function isEnabled() { + public static function isEnabled() { global $wgAllowCopyUploads; return $wgAllowCopyUploads && parent::isEnabled(); } - /* entry point for API upload:: ASYNC_DOWNLOAD (if possible) */ - function initialize( $name, $url, $asyncdownload, $na = false ) { + /** + * Entry point for API upload:: ASYNC_DOWNLOAD (if possible) + */ + public function initialize( $name, $url, $asyncdownload, $na = false ) { global $wgTmpDirectory, $wgPhpCli; // check for $asyncdownload request: @@ -36,8 +47,8 @@ class UploadFromUrl extends UploadBase { } } - $local_file = tempnam( $wgTmpDirectory, 'WEBUPLOAD' ); - parent::initialize( $name, $local_file, 0, true ); + $localFile = tempnam( $wgTmpDirectory, 'WEBUPLOAD' ); + parent::initialize( $name, $localFile, 0, true ); $this->mUrl = trim( $url ); } @@ -50,7 +61,7 @@ class UploadFromUrl extends UploadBase { * Entry point for SpecialUpload no ASYNC_DOWNLOAD possible * @param $request Object: WebRequest object */ - function initializeFromRequest( &$request ) { + public function initializeFromRequest( &$request ) { // set dl mode if not set: if( !$this->dl_mode ) @@ -69,16 +80,16 @@ class UploadFromUrl extends UploadBase { /** * Do the real fetching stuff */ - function fetchFile() { - // entry point for SpecialUplaod - if( self::isValidURI( $this->mUrl ) === false ) { + public function fetchFile() { + // Entry point for SpecialUpload + if( Http::isValidURI( $this->mUrl ) === false ) { return Status::newFatal( 'upload-proto-error' ); } - // now do the actual download to the target file: + // Now do the actual download to the target file: $status = Http::doDownload( $this->mUrl, $this->mTempPath, $this->dl_mode ); - // update the local filesize var: + // Update the local filesize var: $this->mFileSize = filesize( $this->mTempPath ); return $status; @@ -87,23 +98,13 @@ class UploadFromUrl extends UploadBase { /** * @param $request Object: WebRequest object */ - static function isValidRequest( $request ){ + public static function isValidRequest( $request ){ if( !$request->getVal( 'wpUploadFileURL' ) ) return false; // check that is a valid url: - return self::isValidURI( $request->getVal( 'wpUploadFileURL' ) ); + return Http::isValidURI( $request->getVal( 'wpUploadFileURL' ) ); } - /** - * Checks that the given URI is a valid one - * @param $uri Mixed: URI to check for validity - */ - static function isValidURI( $uri ){ - return preg_match( - '/(ftp|http|https):\/\/(\w+:{0,1}\w*@)?(\S+)(:[0-9]+)?(\/|\/([\w#!:.?+=&%@!\-\/]))?/', - $uri, - $matches - ); - } + } \ No newline at end of file -- 2.20.1