diff options
Diffstat (limited to 'includes/upload')
| -rw-r--r-- | includes/upload/UploadBase.php | 462 | ||||
| -rw-r--r-- | includes/upload/UploadFromChunks.php | 110 | ||||
| -rw-r--r-- | includes/upload/UploadFromFile.php | 9 | ||||
| -rw-r--r-- | includes/upload/UploadFromStash.php | 46 | ||||
| -rw-r--r-- | includes/upload/UploadFromUrl.php | 72 | ||||
| -rw-r--r-- | includes/upload/UploadStash.php | 286 |
6 files changed, 625 insertions, 360 deletions
diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 8268f8e3..89ce2b3a 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -31,8 +31,6 @@ * UploadBase and subclasses are the backend of MediaWiki's file uploads. * The frontends are formed by ApiUpload and SpecialUpload. * - * See also includes/docs/upload.txt - * * @author Brion Vibber * @author Bryan Tong Minh * @author Michael Dale @@ -46,7 +44,13 @@ abstract class UploadBase { protected $mBlackListedExtensions; protected $mJavaDetected, $mSVGNSError; - protected static $safeXmlEncodings = array( 'UTF-8', 'ISO-8859-1', 'ISO-8859-2', 'UTF-16', 'UTF-32' ); + protected static $safeXmlEncodings = array( + 'UTF-8', + 'ISO-8859-1', + 'ISO-8859-2', + 'UTF-16', + 'UTF-32' + ); const SUCCESS = 0; const OK = 0; @@ -68,7 +72,7 @@ abstract class UploadBase { const SESSION_STATUS_KEY = 'wsUploadStatusData'; /** - * @param $error int + * @param int $error * @return string */ public function getVerificationErrorCode( $error ) { @@ -105,7 +109,7 @@ abstract class UploadBase { } # Check php's file_uploads setting - return wfIsHipHop() || wfIniGetBool( 'file_uploads' ); + return wfIsHHVM() || wfIniGetBool( 'file_uploads' ); } /** @@ -113,8 +117,8 @@ abstract class UploadBase { * identifying the missing permission. * Can be overridden by subclasses. * - * @param $user User - * @return bool + * @param User $user + * @return bool|string */ public static function isAllowed( $user ) { foreach ( array( 'upload', 'edit' ) as $permission ) { @@ -122,18 +126,19 @@ abstract class UploadBase { return $permission; } } + return true; } // Upload handlers. Should probably just be a global. - static $uploadHandlers = array( 'Stash', 'File', 'Url' ); + private static $uploadHandlers = array( 'Stash', 'File', 'Url' ); /** * Create a form of UploadBase depending on wpSourceType and initializes it * - * @param $request WebRequest - * @param $type - * @return null + * @param WebRequest $request + * @param string|null $type + * @return null|UploadBase */ public static function createFromRequest( &$request, $type = null ) { $type = $type ? $type : $request->getVal( 'wpSourceType', 'File' ); @@ -166,22 +171,25 @@ abstract class UploadBase { return null; } + /** @var UploadBase $handler */ $handler = new $className; $handler->initializeFromRequest( $request ); + return $handler; } /** * Check whether a request if valid for this handler - * @param $request + * @param WebRequest $request * @return bool */ public static function isValidRequest( $request ) { return false; } - public function __construct() {} + public function __construct() { + } /** * Returns the upload type. Should be overridden by child classes @@ -195,9 +203,9 @@ abstract class UploadBase { /** * Initialize the path information - * @param string $name the desired destination name - * @param string $tempPath the temporary path - * @param int $fileSize the file size + * @param string $name The desired destination name + * @param string $tempPath The temporary path + * @param int $fileSize The file size * @param bool $removeTempFile (false) remove the temporary file? * @throws MWException */ @@ -213,6 +221,8 @@ abstract class UploadBase { /** * Initialize from a WebRequest. Override this in a subclass. + * + * @param WebRequest $request */ abstract public function initializeFromRequest( &$request ); @@ -234,7 +244,7 @@ abstract class UploadBase { /** * Return the file size - * @return integer + * @return int */ public function getFileSize() { return $this->mFileSize; @@ -249,15 +259,16 @@ abstract class UploadBase { } /** - * @param string $srcPath the source path - * @return string|bool the real path if it was a virtual URL Returns false on failure + * @param string $srcPath The source path + * @return string|bool The real path if it was a virtual URL Returns false on failure */ function getRealPath( $srcPath ) { wfProfileIn( __METHOD__ ); $repo = RepoGroup::singleton()->getLocalRepo(); if ( $repo->isVirtualUrl( $srcPath ) ) { - // @todo just make uploads work with storage paths - // UploadFromStash loads files via virtual URLs + /** @todo Just make uploads work with storage paths UploadFromStash + * loads files via virtual URLs. + */ $tmpFile = $repo->getLocalCopy( $srcPath ); if ( $tmpFile ) { $tmpFile->bind( $this ); // keep alive with $this @@ -267,12 +278,13 @@ abstract class UploadBase { $path = $srcPath; } wfProfileOut( __METHOD__ ); + return $path; } /** * Verify whether the upload is sane. - * @return mixed self::OK or else an array with error information + * @return mixed Const self::OK or else an array with error information */ public function verifyUpload() { wfProfileIn( __METHOD__ ); @@ -282,6 +294,7 @@ abstract class UploadBase { */ if ( $this->isEmptyFile() ) { wfProfileOut( __METHOD__ ); + return array( 'status' => self::EMPTY_FILE ); } @@ -291,6 +304,7 @@ abstract class UploadBase { $maxSize = self::getMaxUploadSize( $this->getSourceType() ); if ( $this->mFileSize > $maxSize ) { wfProfileOut( __METHOD__ ); + return array( 'status' => self::FILE_TOO_LARGE, 'max' => $maxSize, @@ -305,6 +319,7 @@ abstract class UploadBase { $verification = $this->verifyFile(); if ( $verification !== true ) { wfProfileOut( __METHOD__ ); + return array( 'status' => self::VERIFICATION_ERROR, 'details' => $verification @@ -317,27 +332,30 @@ abstract class UploadBase { $result = $this->validateName(); if ( $result !== true ) { wfProfileOut( __METHOD__ ); + return $result; } $error = ''; if ( !wfRunHooks( 'UploadVerification', - array( $this->mDestName, $this->mTempPath, &$error ) ) ) - { + array( $this->mDestName, $this->mTempPath, &$error ) ) + ) { wfProfileOut( __METHOD__ ); + return array( 'status' => self::HOOK_ABORTED, 'error' => $error ); } wfProfileOut( __METHOD__ ); + 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' + * @return mixed True if valid, otherwise and array with 'status' * and other keys - **/ + */ public function validateName() { $nt = $this->getTitle(); if ( is_null( $nt ) ) { @@ -351,6 +369,7 @@ abstract class UploadBase { $result['blacklistedExt'] = $this->mBlackListedExtensions; } } + return $result; } $this->mDestName = $this->getLocalFile()->getName(); @@ -359,25 +378,27 @@ abstract class UploadBase { } /** - * Verify the mime type. + * Verify the MIME type. * - * @note Only checks that it is not an evil mime. The does it have - * correct extension given its mime type check is in verifyFile. - * @param string $mime representing the mime - * @return mixed true if the file is verified, an array otherwise + * @note Only checks that it is not an evil MIME. The "does it have + * correct extension given its MIME type?" check is in verifyFile. + * in `verifyFile()` that MIME type and file extension correlate. + * @param string $mime Representing the MIME + * @return mixed True if the file is verified, an array otherwise */ protected function verifyMimeType( $mime ) { global $wgVerifyMimeType; wfProfileIn( __METHOD__ ); if ( $wgVerifyMimeType ) { - wfDebug( "\n\nmime: <$mime> extension: <{$this->mFinalExtension}>\n\n" ); + wfDebug( "mime: <$mime> extension: <{$this->mFinalExtension}>\n" ); global $wgMimeTypeBlacklist; if ( $this->checkFileExtension( $mime, $wgMimeTypeBlacklist ) ) { wfProfileOut( __METHOD__ ); + return array( 'filetype-badmime', $mime ); } - # Check IE type + # Check what Internet Explorer would detect $fp = fopen( $this->mTempPath, 'rb' ); $chunk = fread( $fp, 256 ); fclose( $fp ); @@ -388,20 +409,21 @@ abstract class UploadBase { foreach ( $ieTypes as $ieType ) { if ( $this->checkFileExtension( $ieType, $wgMimeTypeBlacklist ) ) { wfProfileOut( __METHOD__ ); + return array( 'filetype-bad-ie-mime', $ieType ); } } } wfProfileOut( __METHOD__ ); + return true; } - /** * Verifies that it's ok to include the uploaded file * - * @return mixed true of the file is verified, array otherwise. + * @return mixed True of the file is verified, array otherwise. */ protected function verifyFile() { global $wgVerifyMimeType; @@ -410,27 +432,29 @@ abstract class UploadBase { $status = $this->verifyPartialFile(); if ( $status !== true ) { wfProfileOut( __METHOD__ ); + return $status; } $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); - $mime = $this->mFileProps['file-mime']; + $mime = $this->mFileProps['mime']; if ( $wgVerifyMimeType ) { # XXX: Missing extension will be caught by validateName() via getTitle() if ( $this->mFinalExtension != '' && !$this->verifyExtension( $mime, $this->mFinalExtension ) ) { wfProfileOut( __METHOD__ ); + return array( 'filetype-mime-mismatch', $this->mFinalExtension, $mime ); } } - $handler = MediaHandler::getHandler( $mime ); if ( $handler ) { $handlerStatus = $handler->verifyUpload( $this->mTempPath ); if ( !$handlerStatus->isOK() ) { $errors = $handlerStatus->getErrorsArray(); wfProfileOut( __METHOD__ ); + return reset( $errors ); } } @@ -438,11 +462,13 @@ abstract class UploadBase { wfRunHooks( 'UploadVerifyFile', array( $this, $mime, &$status ) ); if ( $status !== true ) { wfProfileOut( __METHOD__ ); + return $status; } wfDebug( __METHOD__ . ": all clear; passing.\n" ); wfProfileOut( __METHOD__ ); + return true; } @@ -452,7 +478,7 @@ abstract class UploadBase { * Runs the blacklist checks, but not any checks that may * assume the entire file is present. * - * @return Mixed true for valid or array with error message key. + * @return mixed True for valid or array with error message key. */ protected function verifyPartialFile() { global $wgAllowJavaUploads, $wgDisableUploadScriptChecks; @@ -463,11 +489,12 @@ abstract class UploadBase { $this->mFileProps = FSFile::getPropsFromPath( $this->mTempPath, $this->mFinalExtension ); - # check mime type, if desired + # check MIME type, if desired $mime = $this->mFileProps['file-mime']; $status = $this->verifyMimeType( $mime ); if ( $status !== true ) { wfProfileOut( __METHOD__ ); + return $status; } @@ -475,12 +502,14 @@ abstract class UploadBase { if ( !$wgDisableUploadScriptChecks ) { if ( self::detectScript( $this->mTempPath, $mime, $this->mFinalExtension ) ) { wfProfileOut( __METHOD__ ); + return array( 'uploadscripted' ); } if ( $this->mFinalExtension == 'svg' || $mime == 'image/svg+xml' ) { $svgStatus = $this->detectScriptInSvg( $this->mTempPath ); if ( $svgStatus !== false ) { wfProfileOut( __METHOD__ ); + return $svgStatus; } } @@ -497,11 +526,13 @@ abstract class UploadBase { $error = reset( $errors ); if ( $error[0] !== 'zip-wrong-format' ) { wfProfileOut( __METHOD__ ); + return $error; } } if ( $this->mJavaDetected ) { wfProfileOut( __METHOD__ ); + return array( 'uploadjava' ); } } @@ -510,15 +541,19 @@ abstract class UploadBase { $virus = $this->detectVirus( $this->mTempPath ); if ( $virus ) { wfProfileOut( __METHOD__ ); + return array( 'uploadvirus', $virus ); } wfProfileOut( __METHOD__ ); + return true; } /** * Callback for ZipDirectoryReader to detect Java class files. + * + * @param array $entry */ function zipEntryCallback( $entry ) { $names = array( $entry['name'] ); @@ -541,11 +576,13 @@ abstract class UploadBase { } /** - * Alias for verifyTitlePermissions. The function was originally 'verifyPermissions' - * but that suggests it's checking the user, when it's really checking the title + user combination. - * @param $user User object to verify the permissions against + * Alias for verifyTitlePermissions. The function was originally + * 'verifyPermissions', but that suggests it's checking the user, when it's + * really checking the title + user combination. + * + * @param User $user User object to verify the permissions against * @return mixed An array as returned by getUserPermissionsErrors or true - * in case the user has proper permissions. + * in case the user has proper permissions. */ public function verifyPermissions( $user ) { return $this->verifyTitlePermissions( $user ); @@ -558,9 +595,9 @@ abstract class UploadBase { * isAllowed() should be called as well for generic is-user-blocked or * can-user-upload checking. * - * @param $user User object to verify the permissions against + * @param User $user User object to verify the permissions against * @return mixed An array as returned by getUserPermissionsErrors or true - * in case the user has proper permissions. + * in case the user has proper permissions. */ public function verifyTitlePermissions( $user ) { /** @@ -581,6 +618,7 @@ abstract class UploadBase { if ( $permErrors || $permErrorsUpload || $permErrorsCreate ) { $permErrors = array_merge( $permErrors, wfArrayDiff2( $permErrorsUpload, $permErrors ) ); $permErrors = array_merge( $permErrors, wfArrayDiff2( $permErrorsCreate, $permErrors ) ); + return $permErrors; } @@ -597,7 +635,7 @@ abstract class UploadBase { * * This should not assume that mTempPath is set. * - * @return Array of warnings + * @return array Array of warnings */ public function checkWarnings() { global $wgLang; @@ -617,6 +655,9 @@ abstract class UploadBase { if ( $this->mDesiredDestName != $filename && $comparableName != $filename ) { $warnings['badfilename'] = $filename; + // Debugging for bug 62241 + wfDebugLog( 'upload', "Filename: '$filename', mDesiredDestName: " + . "'$this->mDesiredDestName', comparableName: '$comparableName'" ); } // Check whether the file extension is on the unwanted list @@ -660,10 +701,15 @@ abstract class UploadBase { // Check dupes against archives $archivedImage = new ArchivedFile( null, 0, "{$hash}.{$this->mFinalExtension}" ); if ( $archivedImage->getID() > 0 ) { - $warnings['duplicate-archive'] = $archivedImage->getName(); + if ( $archivedImage->userCan( File::DELETED_FILE ) ) { + $warnings['duplicate-archive'] = $archivedImage->getName(); + } else { + $warnings['duplicate-archive'] = ''; + } } wfProfileOut( __METHOD__ ); + return $warnings; } @@ -671,12 +717,12 @@ abstract class UploadBase { * Really perform the upload. Stores the file in the local repo, watches * if necessary and runs the UploadComplete hook. * - * @param $comment - * @param $pageText - * @param $watch - * @param $user User + * @param string $comment + * @param string $pageText + * @param bool $watch + * @param User $user * - * @return Status indicating the whether the upload succeeded. + * @return Status Indicating the whether the upload succeeded. */ public function performUpload( $comment, $pageText, $watch, $user ) { wfProfileIn( __METHOD__ ); @@ -693,12 +739,17 @@ abstract class UploadBase { if ( $status->isGood() ) { if ( $watch ) { - WatchAction::doWatch( $this->getLocalFile()->getTitle(), $user, WatchedItem::IGNORE_USER_RIGHTS ); + WatchAction::doWatch( + $this->getLocalFile()->getTitle(), + $user, + WatchedItem::IGNORE_USER_RIGHTS + ); } wfRunHooks( 'UploadComplete', array( &$this ) ); } wfProfileOut( __METHOD__ ); + return $status; } @@ -726,7 +777,9 @@ abstract class UploadBase { # exclamation mark, so restrict file name to 240 bytes. if ( strlen( $this->mFilteredName ) > 240 ) { $this->mTitleError = self::FILENAME_TOO_LONG; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } /** @@ -739,7 +792,9 @@ abstract class UploadBase { $nt = Title::makeTitleSafe( NS_FILE, $this->mFilteredName ); if ( is_null( $nt ) ) { $this->mTitleError = self::ILLEGAL_FILENAME; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } $this->mFilteredName = $nt->getDBkey(); @@ -780,61 +835,79 @@ abstract class UploadBase { if ( $this->mFinalExtension == '' ) { $this->mTitleError = self::FILETYPE_MISSING; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } elseif ( $blackListedExtensions || - ( $wgCheckFileExtensions && $wgStrictFileExtensions && - !$this->checkFileExtensionList( $ext, $wgFileExtensions ) ) ) { + ( $wgCheckFileExtensions && $wgStrictFileExtensions && + !$this->checkFileExtension( $this->mFinalExtension, $wgFileExtensions ) ) + ) { $this->mBlackListedExtensions = $blackListedExtensions; $this->mTitleError = self::FILETYPE_BADTYPE; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } - // Windows may be broken with special characters, see bug XXX - if ( wfIsWindows() && !preg_match( '/^[\x0-\x7f]*$/', $nt->getText() ) ) { + // Windows may be broken with special characters, see bug 1780 + if ( !preg_match( '/^[\x0-\x7f]*$/', $nt->getText() ) + && !RepoGroup::singleton()->getLocalRepo()->backendSupportsUnicodePaths() + ) { $this->mTitleError = self::WINDOWS_NONASCII_FILENAME; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } # If there was more than one "extension", reassemble the base # filename to prevent bogus complaints about length if ( count( $ext ) > 1 ) { - for ( $i = 0; $i < count( $ext ) - 1; $i++ ) { + $iterations = count( $ext ) - 1; + for ( $i = 0; $i < $iterations; $i++ ) { $partname .= '.' . $ext[$i]; } } if ( strlen( $partname ) < 1 ) { $this->mTitleError = self::MIN_LENGTH_PARTNAME; - return $this->mTitle = null; + $this->mTitle = null; + + return $this->mTitle; } - return $this->mTitle = $nt; + $this->mTitle = $nt; + + return $this->mTitle; } /** * Return the local file and initializes if necessary. * - * @return LocalFile|null + * @return LocalFile|UploadStashFile|null */ public function getLocalFile() { if ( is_null( $this->mLocalFile ) ) { $nt = $this->getTitle(); $this->mLocalFile = is_null( $nt ) ? null : wfLocalFile( $nt ); } + return $this->mLocalFile; } /** - * If the user does not supply all necessary information in the first upload form submission (either by accident or - * by design) then we may want to stash the file temporarily, get more information, and publish the file later. + * If the user does not supply all necessary information in the first upload + * form submission (either by accident or by design) then we may want to + * stash the file temporarily, get more information, and publish the file + * later. * - * This method will stash a file in a temporary directory for later processing, and save the necessary descriptive info - * into the database. - * This method returns the file object, which also has a 'fileKey' property which can be passed through a form or - * API request to find this stashed file again. + * This method will stash a file in a temporary directory for later + * processing, and save the necessary descriptive info into the database. + * This method returns the file object, which also has a 'fileKey' property + * which can be passed through a form or API request to find this stashed + * file again. * - * @param $user User - * @return UploadStashFile stashed file + * @param User $user + * @return UploadStashFile Stashed file */ public function stashFile( User $user = null ) { // was stashSessionFile @@ -845,13 +918,15 @@ abstract class UploadBase { $this->mLocalFile = $file; wfProfileOut( __METHOD__ ); + return $file; } /** - * Stash a file in a temporary directory, returning a key which can be used to find the file again. See stashFile(). + * Stash a file in a temporary directory, returning a key which can be used + * to find the file again. See stashFile(). * - * @return String: file key + * @return string File key */ public function stashFileGetKey() { return $this->stashFile()->getFileKey(); @@ -860,7 +935,7 @@ abstract class UploadBase { /** * alias for stashFileGetKey, for backwards compatibility * - * @return String: file key + * @return string File key */ public function stashSession() { return $this->stashFileGetKey(); @@ -887,12 +962,13 @@ abstract class UploadBase { * earlier pseudo-'extensions' to determine type and execute * scripts, so the blacklist needs to check them all. * - * @param $filename string + * @param string $filename * @return array */ public static function splitExtensions( $filename ) { $bits = explode( '.', $filename ); $basename = array_shift( $bits ); + return array( $basename, $bits ); } @@ -900,9 +976,9 @@ abstract class UploadBase { * Perform case-insensitive match against a list of file extensions. * Returns true if the extension is in the list. * - * @param $ext String - * @param $list Array - * @return Boolean + * @param string $ext + * @param array $list + * @return bool */ public static function checkFileExtension( $ext, $list ) { return in_array( strtolower( $ext ), $list ); @@ -912,20 +988,20 @@ abstract class UploadBase { * Perform case-insensitive match against a list of file extensions. * Returns an array of matching extensions. * - * @param $ext Array - * @param $list Array - * @return Boolean + * @param array $ext + * @param array $list + * @return bool */ public static function checkFileExtensionList( $ext, $list ) { return array_intersect( array_map( 'strtolower', $ext ), $list ); } /** - * Checks if the mime type of the uploaded file matches the file extension. + * Checks if the MIME type of the uploaded file matches the file extension. * - * @param string $mime the mime type of the uploaded file - * @param string $extension the filename extension that the file is to be served with - * @return Boolean + * @param string $mime The MIME type of the uploaded file + * @param string $extension The filename extension that the file is to be served with + * @return bool */ public static function verifyExtension( $mime, $extension ) { $magic = MimeMagic::singleton(); @@ -934,10 +1010,12 @@ abstract class UploadBase { if ( !$magic->isRecognizableExtension( $extension ) ) { wfDebug( __METHOD__ . ": passing file with unknown detected mime type; " . "unrecognized extension '$extension', can't verify\n" ); + return true; } else { wfDebug( __METHOD__ . ": rejecting file with unknown detected mime type; " . "recognized extension '$extension', so probably invalid file\n" ); + return false; } } @@ -947,19 +1025,22 @@ abstract class UploadBase { if ( $match === null ) { if ( $magic->getTypesForExtension( $extension ) !== null ) { wfDebug( __METHOD__ . ": No extension known for $mime, but we know a mime for $extension\n" ); + return false; } else { wfDebug( __METHOD__ . ": no file extension known for mime type $mime, passing file\n" ); + return true; } } elseif ( $match === true ) { wfDebug( __METHOD__ . ": mime type $mime matches extension $extension, passing file\n" ); - #TODO: if it's a bitmap, make sure PHP or ImageMagic resp. can handle it! + /** @todo If it's a bitmap, make sure PHP or ImageMagick resp. can handle it! */ return true; - } else { - wfDebug( __METHOD__ . ": mime type $mime mismatches file extension $extension, rejecting file\n" ); + wfDebug( __METHOD__ + . ": mime type $mime mismatches file extension $extension, rejecting file\n" ); + return false; } } @@ -970,10 +1051,10 @@ abstract class UploadBase { * potentially harmful. The present implementation will produce false * positives in some situations. * - * @param string $file pathname to the temporary upload file - * @param string $mime the mime type of the file - * @param string $extension the extension of the file - * @return Boolean: true if the file contains something looking like embedded scripts + * @param string $file Pathname to the temporary upload file + * @param string $mime The MIME type of the file + * @param string $extension The extension of the file + * @return bool True if the file contains something looking like embedded scripts */ public static function detectScript( $file, $mime, $extension ) { global $wgAllowTitlesInSVG; @@ -994,6 +1075,7 @@ abstract class UploadBase { if ( !$chunk ) { wfProfileOut( __METHOD__ ); + return false; } @@ -1012,12 +1094,13 @@ abstract class UploadBase { $chunk = trim( $chunk ); - # @todo FIXME: Convert from UTF-16 if necessary! + /** @todo FIXME: Convert from UTF-16 if necessary! */ wfDebug( __METHOD__ . ": checking for embedded scripts and HTML stuff\n" ); # check for HTML doctype if ( preg_match( "/<!DOCTYPE *X?HTML/i", $chunk ) ) { wfProfileOut( __METHOD__ ); + return true; } @@ -1026,6 +1109,7 @@ abstract class UploadBase { if ( $extension == 'svg' || strpos( $mime, 'image/svg' ) === 0 ) { if ( self::checkXMLEncodingMissmatch( $file ) ) { wfProfileOut( __METHOD__ ); + return true; } } @@ -1049,7 +1133,7 @@ abstract class UploadBase { '<a href', '<body', '<head', - '<html', #also in safari + '<html', #also in safari '<img', '<pre', '<script', #also in safari @@ -1064,6 +1148,7 @@ abstract class UploadBase { if ( false !== strpos( $chunk, $tag ) ) { wfDebug( __METHOD__ . ": found something that may make it be mistaken for html: $tag\n" ); wfProfileOut( __METHOD__ ); + return true; } } @@ -1079,6 +1164,7 @@ abstract class UploadBase { if ( preg_match( '!type\s*=\s*[\'"]?\s*(?:\w*/)?(?:ecma|java)!sim', $chunk ) ) { wfDebug( __METHOD__ . ": found script types\n" ); wfProfileOut( __METHOD__ ); + return true; } @@ -1086,6 +1172,7 @@ abstract class UploadBase { if ( preg_match( '!(?:href|src|data)\s*=\s*[\'"]?\s*(?:ecma|java)script:!sim', $chunk ) ) { wfDebug( __METHOD__ . ": found html-style script urls\n" ); wfProfileOut( __METHOD__ ); + return true; } @@ -1093,21 +1180,22 @@ abstract class UploadBase { if ( preg_match( '!url\s*\(\s*[\'"]?\s*(?:ecma|java)script:!sim', $chunk ) ) { wfDebug( __METHOD__ . ": found css-style script urls\n" ); wfProfileOut( __METHOD__ ); + return true; } wfDebug( __METHOD__ . ": no scripts found\n" ); wfProfileOut( __METHOD__ ); + return false; } - /** * Check a whitelist of xml encodings that are known not to be interpreted differently * by the server's xml parser (expat) and some common browsers. * - * @param string $file pathname to the temporary upload file - * @return Boolean: true if the file contains an encoding that could be misinterpreted + * @param string $file Pathname to the temporary upload file + * @return bool True if the file contains an encoding that could be misinterpreted */ public static function checkXMLEncodingMissmatch( $file ) { global $wgSVGMetadataCutoff; @@ -1119,16 +1207,19 @@ abstract class UploadBase { && !in_array( strtoupper( $encMatch[1] ), self::$safeXmlEncodings ) ) { wfDebug( __METHOD__ . ": Found unsafe XML encoding '{$encMatch[1]}'\n" ); + return true; } } elseif ( preg_match( "!<\?xml\b!si", $contents ) ) { // Start of XML declaration without an end in the first $wgSVGMetadataCutoff // bytes. There shouldn't be a legitimate reason for this to happen. wfDebug( __METHOD__ . ": Unmatched XML declaration start\n" ); + return true; } elseif ( substr( $contents, 0, 4 ) == "\x4C\x6F\xA7\x94" ) { // EBCDIC encoded XML wfDebug( __METHOD__ . ": EBCDIC Encoded XML\n" ); + return true; } @@ -1139,17 +1230,19 @@ abstract class UploadBase { wfSuppressWarnings(); $str = iconv( $encoding, 'UTF-8', $contents ); wfRestoreWarnings(); - if ( $str != '' && preg_match( "!<\?xml\b(.*?)\?>!si", $str, $matches ) ) { + if ( $str != '' && preg_match( "!<\?xml\b(.*?)\?>!si", $str, $matches ) ) { if ( preg_match( $encodingRegex, $matches[1], $encMatch ) && !in_array( strtoupper( $encMatch[1] ), self::$safeXmlEncodings ) ) { wfDebug( __METHOD__ . ": Found unsafe XML encoding '{$encMatch[1]}'\n" ); + return true; } } elseif ( $str != '' && preg_match( "!<\?xml\b!si", $str ) ) { // Start of XML declaration without an end in the first $wgSVGMetadataCutoff // bytes. There shouldn't be a legitimate reason for this to happen. wfDebug( __METHOD__ . ": Unmatched XML declaration start\n" ); + return true; } } @@ -1158,8 +1251,8 @@ abstract class UploadBase { } /** - * @param $filename string - * @return bool + * @param string $filename + * @return mixed False of the file is verified (does not contain scripts), array otherwise. */ protected function detectScriptInSvg( $filename ) { $this->mSVGNSError = false; @@ -1176,35 +1269,40 @@ abstract class UploadBase { if ( $this->mSVGNSError ) { return array( 'uploadscriptednamespace', $this->mSVGNSError ); } + return array( 'uploadscripted' ); } + return false; } /** * Callback to filter SVG Processing Instructions. - * @param $target string processing instruction name - * @param $data string processing instruction attribute and value + * @param string $target Processing instruction name + * @param string $data Processing instruction attribute and value * @return bool (true if the filter identified something bad) */ public static function checkSvgPICallback( $target, $data ) { // Don't allow external stylesheets (bug 57550) - if ( preg_match( '/xml-stylesheet/i', $target) ) { + if ( preg_match( '/xml-stylesheet/i', $target ) ) { return true; } + return false; } /** * @todo Replace this with a whitelist filter! - * @param $element string - * @param $attribs array + * @param string $element + * @param array $attribs * @return bool */ public function checkSvgScriptCallback( $element, $attribs, $data = null ) { list( $namespace, $strippedElement ) = $this->splitXmlNamespace( $element ); + // We specifically don't include: + // http://www.w3.org/1999/xhtml (bug 60771) static $validNamespaces = array( '', 'adobe:ns:meta/', @@ -1235,17 +1333,21 @@ abstract class UploadBase { 'http://purl.org/dc/elements/1.1', 'http://schemas.microsoft.com/visio/2003/svgextensions/', 'http://sodipodi.sourceforge.net/dtd/sodipodi-0.dtd', + 'http://taptrix.com/inkpad/svg_extensions', 'http://web.resource.org/cc/', 'http://www.freesoftware.fsf.org/bkchem/cdml', 'http://www.inkscape.org/namespaces/inkscape', + 'http://www.opengis.net/gml', 'http://www.w3.org/1999/02/22-rdf-syntax-ns#', 'http://www.w3.org/2000/svg', + 'http://www.w3.org/tr/rec-rdf-syntax/', ); if ( !in_array( $namespace, $validNamespaces ) ) { wfDebug( __METHOD__ . ": Non-svg namespace '$namespace' in uploaded file.\n" ); - // @TODO return a status object to a closure in XmlTypeCheck, for MW1.21+ + /** @todo Return a status object to a closure in XmlTypeCheck, for MW1.21+ */ $this->mSVGNSError = $namespace; + return true; } @@ -1254,24 +1356,29 @@ abstract class UploadBase { */ if ( $strippedElement == 'script' ) { wfDebug( __METHOD__ . ": Found script element '$element' in uploaded file.\n" ); + return true; } - # e.g., <svg xmlns="http://www.w3.org/2000/svg"> <handler xmlns:ev="http://www.w3.org/2001/xml-events" ev:event="load">alert(1)</handler> </svg> + # e.g., <svg xmlns="http://www.w3.org/2000/svg"> + # <handler xmlns:ev="http://www.w3.org/2001/xml-events" ev:event="load">alert(1)</handler> </svg> if ( $strippedElement == 'handler' ) { wfDebug( __METHOD__ . ": Found scriptable element '$element' in uploaded file.\n" ); + return true; } # SVG reported in Feb '12 that used xml:stylesheet to generate javascript block if ( $strippedElement == 'stylesheet' ) { wfDebug( __METHOD__ . ": Found scriptable element '$element' in uploaded file.\n" ); + return true; } # Block iframes, in case they pass the namespace check if ( $strippedElement == 'iframe' ) { wfDebug( __METHOD__ . ": iframe in uploaded file.\n" ); + return true; } @@ -1288,7 +1395,9 @@ abstract class UploadBase { $value = strtolower( $value ); if ( substr( $stripped, 0, 2 ) == 'on' ) { - wfDebug( __METHOD__ . ": Found event-handler attribute '$attrib'='$value' in uploaded file.\n" ); + wfDebug( __METHOD__ + . ": Found event-handler attribute '$attrib'='$value' in uploaded file.\n" ); + return true; } @@ -1309,13 +1418,17 @@ abstract class UploadBase { # href with embedded svg as target if ( $stripped == 'href' && preg_match( '!data:[^,]*image/svg[^,]*,!sim', $value ) ) { - wfDebug( __METHOD__ . ": Found href to embedded svg \"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); + wfDebug( __METHOD__ . ": Found href to embedded svg " + . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); + return true; } # href with embedded (text/xml) svg as target if ( $stripped == 'href' && preg_match( '!data:[^,]*text/xml[^,]*,!sim', $value ) ) { - wfDebug( __METHOD__ . ": Found href to embedded svg \"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); + wfDebug( __METHOD__ . ": Found href to embedded svg " + . "\"<$strippedElement '$attrib'='$value'...\" in uploaded file.\n" ); + return true; } @@ -1332,26 +1445,41 @@ abstract class UploadBase { } # use set/animate to add event-handler attribute to parent - if ( ( $strippedElement == 'set' || $strippedElement == 'animate' ) && $stripped == 'attributename' && substr( $value, 0, 2 ) == 'on' ) { - wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with \"<$strippedElement $stripped='$value'...\" in uploaded file.\n" ); + if ( ( $strippedElement == 'set' || $strippedElement == 'animate' ) + && $stripped == 'attributename' + && substr( $value, 0, 2 ) == 'on' + ) { + wfDebug( __METHOD__ . ": Found svg setting event-handler attribute with " + . "\"<$strippedElement $stripped='$value'...\" in uploaded file.\n" ); + return true; } # use set to add href attribute to parent element - if ( $strippedElement == 'set' && $stripped == 'attributename' && strpos( $value, 'href' ) !== false ) { + if ( $strippedElement == 'set' + && $stripped == 'attributename' + && strpos( $value, 'href' ) !== false + ) { wfDebug( __METHOD__ . ": Found svg setting href attribute '$value' in uploaded file.\n" ); + return true; } # use set to add a remote / data / script target to an element - if ( $strippedElement == 'set' && $stripped == 'to' && preg_match( '!(http|https|data|script):!sim', $value ) ) { + if ( $strippedElement == 'set' + && $stripped == 'to' + && preg_match( '!(http|https|data|script):!sim', $value ) + ) { wfDebug( __METHOD__ . ": Found svg setting attribute to '$value' in uploaded file.\n" ); + return true; } # use handler attribute with remote / data / script if ( $stripped == 'handler' && preg_match( '!(http|https|data|script):!sim', $value ) ) { - wfDebug( __METHOD__ . ": Found svg setting handler with remote/data/script '$attrib'='$value' in uploaded file.\n" ); + wfDebug( __METHOD__ . ": Found svg setting handler with remote/data/script " + . "'$attrib'='$value' in uploaded file.\n" ); + return true; } @@ -1376,11 +1504,15 @@ abstract class UploadBase { } # image filters can pull in url, which could be svg that executes scripts - if ( $strippedElement == 'image' && $stripped == 'filter' && preg_match( '!url\s*\(!sim', $value ) ) { - wfDebug( __METHOD__ . ": Found image filter with url: \"<$strippedElement $stripped='$value'...\" in uploaded file.\n" ); + if ( $strippedElement == 'image' + && $stripped == 'filter' + && preg_match( '!url\s*\(!sim', $value ) + ) { + wfDebug( __METHOD__ . ": Found image filter with url: " + . "\"<$strippedElement $stripped='$value'...\" in uploaded file.\n" ); + return true; } - } return false; //No scripts detected @@ -1440,24 +1572,26 @@ abstract class UploadBase { /** * Divide the element name passed by the xml parser to the callback into URI and prifix. - * @param $name string - * @return array containing the namespace URI and prefix + * @param string $element + * @return array Containing the namespace URI and prefix */ private static function splitXmlNamespace( $element ) { // 'http://www.w3.org/2000/svg:script' -> array( 'http://www.w3.org/2000/svg', 'script' ) $parts = explode( ':', strtolower( $element ) ); $name = array_pop( $parts ); $ns = implode( ':', $parts ); + return array( $ns, $name ); } /** - * @param $name string + * @param string $name * @return string */ private function stripXmlNamespace( $name ) { // 'http://www.w3.org/2000/svg:script' -> 'script' $parts = explode( ':', strtolower( $name ) ); + return array_pop( $parts ); } @@ -1466,10 +1600,10 @@ abstract class UploadBase { * This relies on the $wgAntivirus and $wgAntivirusSetup variables. * $wgAntivirusRequired may be used to deny upload if the scan fails. * - * @param string $file pathname to the temporary upload file - * @return mixed false if not virus is found, NULL if the scan fails or is disabled, - * 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. + * @param string $file Pathname to the temporary upload file + * @return mixed False if not virus is found, null if the scan fails or is disabled, + * 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. */ public static function detectVirus( $file ) { global $wgAntivirus, $wgAntivirusSetup, $wgAntivirusRequired, $wgOut; @@ -1478,6 +1612,7 @@ abstract class UploadBase { if ( !$wgAntivirus ) { wfDebug( __METHOD__ . ": virus scanner disabled\n" ); wfProfileOut( __METHOD__ ); + return null; } @@ -1486,6 +1621,7 @@ abstract class UploadBase { $wgOut->wrapWikiMsg( "<div class=\"error\">\n$1\n</div>", array( 'virus-badscanner', $wgAntivirus ) ); wfProfileOut( __METHOD__ ); + return wfMessage( 'virus-unknownscanner' )->text() . " $wgAntivirus"; } @@ -1530,7 +1666,9 @@ abstract class UploadBase { # scan failed (code was mapped to false by $exitCodeMap) wfDebug( __METHOD__ . ": failed to scan $file (code $exitCode).\n" ); - $output = $wgAntivirusRequired ? wfMessage( 'virus-scanfailed', array( $exitCode ) )->text() : null; + $output = $wgAntivirusRequired + ? wfMessage( 'virus-scanfailed', array( $exitCode ) )->text() + : null; } elseif ( $mappedCode === AV_SCAN_ABORTED ) { # scan failed because filetype is unknown (probably imune) wfDebug( __METHOD__ . ": unsupported file type $file (code $exitCode).\n" ); @@ -1557,6 +1695,7 @@ abstract class UploadBase { } wfProfileOut( __METHOD__ ); + return $output; } @@ -1564,9 +1703,9 @@ abstract class UploadBase { * Check if there's an overwrite conflict and, if so, if restrictions * forbid this user from performing the upload. * - * @param $user User + * @param User $user * - * @return mixed true on success, array on failure + * @return mixed True on success, array on failure */ private function checkOverwrite( $user ) { // First check whether the local file can be overwritten @@ -1593,9 +1732,9 @@ abstract class UploadBase { /** * Check if a user is the last uploader * - * @param $user User object - * @param string $img image name - * @return Boolean + * @param User $user + * @param string $img Image name + * @return bool */ public static function userCanReUpload( User $user, $img ) { if ( $user->isAllowed( 'reupload' ) ) { @@ -1622,7 +1761,7 @@ abstract class UploadBase { * - File exists with normalized extension * - The file looks like a thumbnail and the original exists * - * @param $file File The File object to check + * @param File $file The File object to check * @return mixed False if the file does not exists, else an array */ public static function getExistsWarning( $file ) { @@ -1668,7 +1807,7 @@ abstract class UploadBase { // Check for files with the same name but a different extension $similarFiles = RepoGroup::singleton()->getLocalRepo()->findFilesByPrefix( - "{$partname}.", 1 ); + "{$partname}.", 1 ); if ( count( $similarFiles ) ) { return array( 'warning' => 'exists-normalized', @@ -1679,7 +1818,10 @@ abstract class UploadBase { if ( self::isThumbName( $file->getName() ) ) { # Check for filenames like 50px- or 180px-, these are mostly thumbnails - $nt_thb = Title::newFromText( substr( $partname, strpos( $partname, '-' ) + 1 ) . '.' . $extension, NS_FILE ); + $nt_thb = Title::newFromText( + substr( $partname, strpos( $partname, '-' ) + 1 ) . '.' . $extension, + NS_FILE + ); $file_thb = wfLocalFile( $nt_thb ); if ( $file_thb->exists() ) { return array( @@ -1712,23 +1854,24 @@ abstract class UploadBase { /** * Helper function that checks whether the filename looks like a thumbnail - * @param $filename string + * @param string $filename * @return bool */ public static function isThumbName( $filename ) { $n = strrpos( $filename, '.' ); $partname = $n ? substr( $filename, 0, $n ) : $filename; + return ( - substr( $partname, 3, 3 ) == 'px-' || - substr( $partname, 2, 3 ) == 'px-' - ) && - preg_match( "/[0-9]{2}/", substr( $partname, 0, 2 ) ); + substr( $partname, 3, 3 ) == 'px-' || + substr( $partname, 2, 3 ) == 'px-' + ) && + preg_match( "/[0-9]{2}/", substr( $partname, 0, 2 ) ); } /** * Get a list of blacklisted filename prefixes from [[MediaWiki:Filename-prefix-blacklist]] * - * @return array list of prefixes + * @return array List of prefixes */ public static function getFilenamePrefixBlacklist() { $blacklist = array(); @@ -1749,23 +1892,28 @@ abstract class UploadBase { $blacklist[] = trim( $line ); } } + return $blacklist; } /** * Gets image info about the file just uploaded. * - * Also has the effect of setting metadata to be an 'indexed tag name' in returned API result if - * 'metadata' was requested. Oddly, we have to pass the "result" object down just so it can do that - * with the appropriate format, presumably. + * Also has the effect of setting metadata to be an 'indexed tag name' in + * returned API result if 'metadata' was requested. Oddly, we have to pass + * the "result" object down just so it can do that with the appropriate + * format, presumably. * - * @param $result ApiResult: - * @return Array: image info + * @param ApiResult $result + * @return array Image info */ public function getImageInfo( $result ) { $file = $this->getLocalFile(); - // TODO This cries out for refactoring. We really want to say $file->getAllInfo(); here. - // Perhaps "info" methods should be moved into files, and the API should just wrap them in queries. + /** @todo This cries out for refactoring. + * We really want to say $file->getAllInfo(); here. + * Perhaps "info" methods should be moved into files, and the API should + * just wrap them in queries. + */ if ( $file instanceof UploadStashFile ) { $imParam = ApiQueryStashImageInfo::getPropertyNames(); $info = ApiQueryStashImageInfo::getInfo( $file, array_flip( $imParam ), $result ); @@ -1773,21 +1921,23 @@ abstract class UploadBase { $imParam = ApiQueryImageInfo::getPropertyNames(); $info = ApiQueryImageInfo::getInfo( $file, array_flip( $imParam ), $result ); } + return $info; } /** - * @param $error array + * @param array $error * @return Status */ public function convertVerifyErrorToStatus( $error ) { $code = $error['status']; unset( $code['status'] ); + return Status::newFatal( $this->getVerificationErrorCode( $code ), $error ); } /** - * @param $forType null|string + * @param null|string $forType * @return int */ public static function getMaxUploadSize( $forType = null ) { @@ -1807,8 +1957,8 @@ abstract class UploadBase { /** * Get the current status of a chunked upload (used for polling). * The status will be read from the *current* user session. - * @param $statusKey string - * @return Array|bool + * @param string $statusKey + * @return Status[]|bool */ public static function getSessionStatus( $statusKey ) { return isset( $_SESSION[self::SESSION_STATUS_KEY][$statusKey] ) @@ -1819,8 +1969,8 @@ abstract class UploadBase { /** * Set the current status of a chunked upload (used for polling). * The status will be stored in the *current* user session. - * @param $statusKey string - * @param $value array|false + * @param string $statusKey + * @param array|bool $value * @return void */ public static function setSessionStatus( $statusKey, $value ) { diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php index 2e0b9444..14993023 100644 --- a/includes/upload/UploadFromChunks.php +++ b/includes/upload/UploadFromChunks.php @@ -28,14 +28,19 @@ * @author Michael Dale */ class UploadFromChunks extends UploadFromFile { - protected $mOffset, $mChunkIndex, $mFileKey, $mVirtualTempPath; + protected $mOffset; + protected $mChunkIndex; + protected $mFileKey; + protected $mVirtualTempPath; + /** @var LocalRepo */ + private $repo; /** * Setup local pointers to stash, repo and user (similar to UploadFromStash) * - * @param $user User - * @param $stash UploadStash - * @param $repo FileRepo + * @param User|null $user Default: null + * @param UploadStash|bool $stash Default: false + * @param FileRepo|bool $repo Default: false */ public function __construct( $user = null, $stash = false, $repo = false ) { // user object. sometimes this won't exist, as when running from cron. @@ -57,14 +62,13 @@ class UploadFromChunks extends UploadFromFile { } $this->stash = new UploadStash( $this->repo, $this->user ); } - - return true; } /** * Calls the parent stashFile and updates the uploadsession table to handle "chunks" * - * @return UploadStashFile stashed file + * @param User|null $user + * @return UploadStashFile Stashed file */ public function stashFile( User $user = null ) { // Stash file is the called on creating a new chunk session: @@ -83,11 +87,16 @@ class UploadFromChunks extends UploadFromFile { // Update db table to reflect initial "chunk" state $this->updateChunkStatus(); + return $this->mLocalFile; } /** * Continue chunk uploading + * + * @param string $name + * @param string $key + * @param WebRequestUpload $webRequestUpload */ public function continueChunks( $name, $key, $webRequestUpload ) { $this->mFileKey = $key; @@ -108,13 +117,14 @@ class UploadFromChunks extends UploadFromFile { * @return FileRepoStatus */ public function concatenateChunks() { + $chunkIndex = $this->getChunkIndex(); wfDebug( __METHOD__ . " concatenate {$this->mChunkIndex} chunks:" . - $this->getOffset() . ' inx:' . $this->getChunkIndex() . "\n" ); + $this->getOffset() . ' inx:' . $chunkIndex . "\n" ); // Concatenate all the chunks to mVirtualTempPath - $fileList = Array(); + $fileList = array(); // The first chunk is stored at the mVirtualTempPath path so we start on "chunk 1" - for ( $i = 0; $i <= $this->getChunkIndex(); $i++ ) { + for ( $i = 0; $i <= $chunkIndex; $i++ ) { $fileList[] = $this->getVirtualChunkLocation( $i ); } @@ -122,9 +132,12 @@ class UploadFromChunks extends UploadFromFile { $ext = FileBackend::extensionFromPath( $this->mVirtualTempPath ); // Get a 0-byte temp file to perform the concatenation at $tmpFile = TempFSFile::factory( 'chunkedupload_', $ext ); - $tmpPath = $tmpFile - ? $tmpFile->bind( $this )->getPath() // keep alive with $this - : false; // fail in concatenate() + $tmpPath = false; // fail in concatenate() + if ( $tmpFile ) { + // keep alive with $this + $tmpPath = $tmpFile->bind( $this )->getPath(); + } + // Concatenate the chunks at the temp file $tStart = microtime( true ); $status = $this->repo->concatenate( $fileList, $tmpPath, FileRepo::DELETE_SOURCE ); @@ -132,14 +145,17 @@ class UploadFromChunks extends UploadFromFile { if ( !$status->isOk() ) { return $status; } - wfDebugLog( 'fileconcatenate', "Combined $i chunks in $tAmount seconds.\n" ); + wfDebugLog( 'fileconcatenate', "Combined $i chunks in $tAmount seconds." ); - $this->mTempPath = $tmpPath; // file system path - $this->mFileSize = filesize( $this->mTempPath ); //Since this was set for the last chunk previously + // File system path + $this->mTempPath = $tmpPath; + // Since this was set for the last chunk previously + $this->mFileSize = filesize( $this->mTempPath ); $ret = $this->verifyUpload(); if ( $ret['status'] !== UploadBase::OK ) { wfDebugLog( 'fileconcatenate', "Verification failed for chunked upload" ); $status->fatal( $this->getVerificationErrorCode( $ret['status'] ) ); + return $status; } @@ -149,44 +165,45 @@ class UploadFromChunks extends UploadFromFile { $this->mLocalFile = parent::stashFile( $this->user ); $tAmount = microtime( true ) - $tStart; $this->mLocalFile->setLocalReference( $tmpFile ); // reuse (e.g. for getImageInfo()) - wfDebugLog( 'fileconcatenate', "Stashed combined file ($i chunks) in $tAmount seconds.\n" ); + wfDebugLog( 'fileconcatenate', "Stashed combined file ($i chunks) in $tAmount seconds." ); return $status; } /** * Perform the upload, then remove the temp copy afterward - * @param $comment string - * @param $pageText string - * @param $watch bool - * @param $user User + * @param string $comment + * @param string $pageText + * @param bool $watch + * @param User $user * @return Status */ public function performUpload( $comment, $pageText, $watch, $user ) { $rv = parent::performUpload( $comment, $pageText, $watch, $user ); + return $rv; } /** * Returns the virtual chunk location: - * @param $index + * @param int $index * @return string */ function getVirtualChunkLocation( $index ) { return $this->repo->getVirtualUrl( 'temp' ) . - '/' . - $this->repo->getHashPath( - $this->getChunkFileKey( $index ) - ) . - $this->getChunkFileKey( $index ); + '/' . + $this->repo->getHashPath( + $this->getChunkFileKey( $index ) + ) . + $this->getChunkFileKey( $index ); } /** * Add a chunk to the temporary directory * - * @param string $chunkPath path to temporary chunk file - * @param int $chunkSize size of the current chunk - * @param int $offset offset of current chunk ( mutch match database chunk offset ) + * @param string $chunkPath Path to temporary chunk file + * @param int $chunkSize Size of the current chunk + * @param int $offset Offset of current chunk ( mutch match database chunk offset ) * @return Status */ public function addChunk( $chunkPath, $chunkSize, $offset ) { @@ -220,6 +237,7 @@ class UploadFromChunks extends UploadFromFile { $status = Status::newFatal( 'invalid-chunk-offset' ); } } + return $status; } @@ -228,7 +246,7 @@ class UploadFromChunks extends UploadFromFile { */ private function updateChunkStatus() { wfDebug( __METHOD__ . " update chunk status for {$this->mFileKey} offset:" . - $this->getOffset() . ' inx:' . $this->getChunkIndex() . "\n" ); + $this->getOffset() . ' inx:' . $this->getChunkIndex() . "\n" ); $dbw = $this->repo->getMasterDb(); // Use a quick transaction since we will upload the full temp file into shared @@ -274,30 +292,32 @@ class UploadFromChunks extends UploadFromFile { /** * Get the current Chunk index - * @return Integer index of the current chunk + * @return int Index of the current chunk */ private function getChunkIndex() { if ( $this->mChunkIndex !== null ) { return $this->mChunkIndex; } + return 0; } /** * Gets the current offset in fromt the stashedupload table - * @return Integer current byte offset of the chunk file set + * @return int Current byte offset of the chunk file set */ private function getOffset() { if ( $this->mOffset !== null ) { return $this->mOffset; } + return 0; } /** * Output the chunk to disk * - * @param $chunkPath string + * @param string $chunkPath * @throws UploadChunkFileException * @return FileRepoStatus */ @@ -311,18 +331,20 @@ class UploadFromChunks extends UploadFromFile { $this->repo->getZonePath( 'temp' ) . "/{$hashPath}{$fileKey}" ); // Check for error in stashing the chunk: - if ( ! $storeStatus->isOK() ) { + if ( !$storeStatus->isOK() ) { $error = $storeStatus->getErrorsArray(); $error = reset( $error ); - if ( ! count( $error ) ) { + if ( !count( $error ) ) { $error = $storeStatus->getWarningsArray(); $error = reset( $error ); - if ( ! count( $error ) ) { + if ( !count( $error ) ) { $error = array( 'unknown', 'no error recorded' ); } } - throw new UploadChunkFileException( "error storing file in '$chunkPath': " . implode( '; ', $error ) ); + throw new UploadChunkFileException( "Error storing file in '$chunkPath': " . + implode( '; ', $error ) ); } + return $storeStatus; } @@ -330,6 +352,7 @@ class UploadFromChunks extends UploadFromFile { if ( $index === null ) { $index = $this->getChunkIndex(); } + return $this->mFileKey . '.' . $index; } @@ -352,6 +375,11 @@ class UploadFromChunks extends UploadFromFile { } } -class UploadChunkZeroLengthFileException extends MWException {}; -class UploadChunkFileException extends MWException {}; -class UploadChunkVerificationException extends MWException {}; +class UploadChunkZeroLengthFileException extends MWException { +} + +class UploadChunkFileException extends MWException { +} + +class UploadChunkVerificationException extends MWException { +} diff --git a/includes/upload/UploadFromFile.php b/includes/upload/UploadFromFile.php index a00ed327..3a1e8bdc 100644 --- a/includes/upload/UploadFromFile.php +++ b/includes/upload/UploadFromFile.php @@ -28,14 +28,13 @@ * @author Bryan Tong Minh */ class UploadFromFile extends UploadBase { - /** * @var WebRequestUpload */ protected $mUpload = null; /** - * @param $request WebRequest + * @param WebRequest $request */ function initializeFromRequest( &$request ) { $upload = $request->getUpload( 'wpUploadFile' ); @@ -49,8 +48,8 @@ class UploadFromFile extends UploadBase { /** * Initialize from a filename and a WebRequestUpload - * @param $name - * @param $webRequestUpload + * @param string $name + * @param WebRequestUpload $webRequestUpload */ function initialize( $name, $webRequestUpload ) { $this->mUpload = $webRequestUpload; @@ -59,7 +58,7 @@ class UploadFromFile extends UploadBase { } /** - * @param $request + * @param WebRequest $request * @return bool */ static function isValidRequest( $request ) { diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php index cb85fc63..b4e815fc 100644 --- a/includes/upload/UploadFromStash.php +++ b/includes/upload/UploadFromStash.php @@ -28,7 +28,10 @@ * @author Bryan Tong Minh */ class UploadFromStash extends UploadBase { - protected $mFileKey, $mVirtualTempPath, $mFileProps, $mSourceType; + protected $mFileKey; + protected $mVirtualTempPath; + protected $mFileProps; + protected $mSourceType; // an instance of UploadStash private $stash; @@ -37,9 +40,9 @@ class UploadFromStash extends UploadBase { private $repo; /** - * @param $user User - * @param $stash UploadStash - * @param $repo FileRepo + * @param User|bool $user Default: false + * @param UploadStash|bool $stash Default: false + * @param FileRepo|bool $repo Default: false */ public function __construct( $user = false, $stash = false, $repo = false ) { // user object. sometimes this won't exist, as when running from cron. @@ -65,7 +68,7 @@ class UploadFromStash extends UploadBase { } /** - * @param $key string + * @param string $key * @return bool */ public static function isValidKey( $key ) { @@ -74,9 +77,8 @@ class UploadFromStash extends UploadBase { } /** - * @param $request WebRequest - * - * @return Boolean + * @param WebRequest $request + * @return bool */ public static function isValidRequest( $request ) { // this passes wpSessionKey to getText() as a default when wpFileKey isn't set. @@ -86,8 +88,9 @@ class UploadFromStash extends UploadBase { } /** - * @param $key string - * @param $name string + * @param string $key + * @param string $name + * @param bool $initTempFile */ public function initialize( $key, $name = 'upload_file', $initTempFile = true ) { /** @@ -110,14 +113,17 @@ class UploadFromStash extends UploadBase { } /** - * @param $request WebRequest + * @param WebRequest $request */ public function initializeFromRequest( &$request ) { // sends wpSessionKey as a default when wpFileKey is missing $fileKey = $request->getText( 'wpFileKey', $request->getText( 'wpSessionKey' ) ); // chooses one of wpDestFile, wpUploadFile, filename in that order. - $desiredDestName = $request->getText( 'wpDestFile', $request->getText( 'wpUploadFile', $request->getText( 'filename' ) ) ); + $desiredDestName = $request->getText( + 'wpDestFile', + $request->getText( 'wpUploadFile', $request->getText( 'filename' ) ) + ); $this->initialize( $fileKey, $desiredDestName ); } @@ -144,19 +150,20 @@ class UploadFromStash extends UploadBase { /** * Stash the file. * - * @param $user User + * @param User $user * @return UploadStashFile */ public function stashFile( User $user = null ) { // replace mLocalFile with an instance of UploadStashFile, which adds some methods // that are useful for stashed files. $this->mLocalFile = parent::stashFile( $user ); + return $this->mLocalFile; } /** * This should return the key instead of the UploadStashFile instance, for backward compatibility. - * @return String + * @return string */ public function stashSession() { return $this->stashFile()->getFileKey(); @@ -164,7 +171,7 @@ class UploadFromStash extends UploadBase { /** * Remove a temporarily kept file stashed by saveTempUploadedFile(). - * @return bool success + * @return bool Success */ public function unsaveUploadedFile() { return $this->stash->removeFile( $this->mFileKey ); @@ -172,15 +179,16 @@ class UploadFromStash extends UploadBase { /** * Perform the upload, then remove the database record afterward. - * @param $comment string - * @param $pageText string - * @param $watch bool - * @param $user User + * @param string $comment + * @param string $pageText + * @param bool $watch + * @param User $user * @return Status */ public function performUpload( $comment, $pageText, $watch, $user ) { $rv = parent::performUpload( $comment, $pageText, $watch, $user ); $this->unsaveUploadedFile(); + return $rv; } } diff --git a/includes/upload/UploadFromUrl.php b/includes/upload/UploadFromUrl.php index 0201d5f4..b6056401 100644 --- a/includes/upload/UploadFromUrl.php +++ b/includes/upload/UploadFromUrl.php @@ -41,7 +41,7 @@ class UploadFromUrl extends UploadBase { * user is not allowed, return the name of the user right as a string. If * the user is allowed, have the parent do further permissions checking. * - * @param $user User + * @param User $user * * @return bool|string */ @@ -49,6 +49,7 @@ class UploadFromUrl extends UploadBase { if ( !$user->isAllowed( 'upload_by_url' ) ) { return 'upload_by_url'; } + return parent::isAllowed( $user ); } @@ -58,6 +59,7 @@ class UploadFromUrl extends UploadBase { */ public static function isEnabled() { global $wgAllowCopyUploads; + return $wgAllowCopyUploads && parent::isEnabled(); } @@ -66,7 +68,7 @@ class UploadFromUrl extends UploadBase { * The domains in the whitelist can include wildcard characters (*) in place * of any of the domain levels, e.g. '*.flickr.com' or 'upload.*.gov.uk'. * - * @param $url string + * @param string $url * @return bool */ public static function isAllowedHost( $url ) { @@ -103,13 +105,14 @@ class UploadFromUrl extends UploadBase { } */ } + return $valid; } /** * Checks whether the URL is not allowed. * - * @param $url string + * @param string $url * @return bool */ public static function isAllowedUrl( $url ) { @@ -118,15 +121,16 @@ class UploadFromUrl extends UploadBase { wfRunHooks( 'IsUploadAllowedFromUrl', array( $url, &$allowed ) ); self::$allowedUrls[$url] = $allowed; } + return self::$allowedUrls[$url]; } /** * Entry point for API upload * - * @param $name string - * @param $url string - * @param $async mixed Whether the download should be performed + * @param string $name + * @param string $url + * @param bool|string $async Whether the download should be performed * asynchronous. False for synchronous, async or async-leavemessage for * asynchronous download. * @throws MWException @@ -147,7 +151,7 @@ class UploadFromUrl extends UploadBase { /** * Entry point for SpecialUpload - * @param $request WebRequest object + * @param WebRequest $request */ public function initializeFromRequest( &$request ) { $desiredDestName = $request->getText( 'wpDestFile' ); @@ -162,13 +166,14 @@ class UploadFromUrl extends UploadBase { } /** - * @param $request WebRequest object + * @param WebRequest $request * @return bool */ public static function isValidRequest( $request ) { global $wgUser; $url = $request->getVal( 'wpUploadFileURL' ); + return !empty( $url ) && Http::isValidURI( $url ) && $wgUser->isAllowed( 'upload_by_url' ); @@ -184,7 +189,7 @@ class UploadFromUrl extends UploadBase { /** * Download the file (if not async) * - * @param Array $httpOptions Array of options for MWHttpRequest. Ignored if async. + * @param array $httpOptions Array of options for MWHttpRequest. Ignored if async. * This could be used to override the timeout on the http request. * @return Status */ @@ -202,23 +207,28 @@ class UploadFromUrl extends UploadBase { if ( !$this->mAsync ) { return $this->reallyFetchFile( $httpOptions ); } + return Status::newGood(); } + /** * Create a new temporary file in the URL subdirectory of wfTempDir(). * * @return string Path to the file */ protected function makeTemporaryFile() { - return tempnam( wfTempDir(), 'URL' ); + $tmpFile = TempFSFile::factory( 'URL' ); + $tmpFile->bind( $this ); + + return $tmpFile->getPath(); } /** * Callback: save a chunk of the result of a HTTP request to the temporary file * - * @param $req mixed - * @param $buffer string - * @return int number of bytes handled + * @param mixed $req + * @param string $buffer + * @return int Number of bytes handled */ public function saveTempFileChunk( $req, $buffer ) { $nbytes = fwrite( $this->mTmpHandle, $buffer ); @@ -238,7 +248,7 @@ class UploadFromUrl extends UploadBase { * Download the file, save it to the temporary file and update the file * size and set $mRemoveTempFile to true. * - * @param Array $httpOptions Array of options for MWHttpRequest + * @param array $httpOptions Array of options for MWHttpRequest * @return Status */ protected function reallyFetchFile( $httpOptions = array() ) { @@ -256,12 +266,12 @@ class UploadFromUrl extends UploadBase { $this->mRemoveTempFile = true; $this->mFileSize = 0; - $options = $httpOptions + array( - 'followRedirects' => true, - ); + $options = $httpOptions + array( 'followRedirects' => true ); + if ( $wgCopyUploadProxy !== false ) { $options['proxy'] = $wgCopyUploadProxy; } + if ( $wgCopyUploadTimeout && !isset( $options['timeout'] ) ) { $options['timeout'] = $wgCopyUploadTimeout; } @@ -294,42 +304,46 @@ class UploadFromUrl extends UploadBase { if ( $this->mAsync ) { return array( 'status' => UploadBase::OK ); } + return parent::verifyUpload(); } /** * Wrapper around the parent function in order to defer checking warnings * until the file really has been fetched. - * @return Array + * @return array */ public function checkWarnings() { if ( $this->mAsync ) { $this->mIgnoreWarnings = false; + return array(); } + return parent::checkWarnings(); } /** * Wrapper around the parent function in order to defer checking protection * until we are sure that the file can actually be uploaded - * @param $user User + * @param User $user * @return bool|mixed */ public function verifyTitlePermissions( $user ) { if ( $this->mAsync ) { return true; } + return parent::verifyTitlePermissions( $user ); } /** * Wrapper around the parent function in order to defer uploading to the * job queue for asynchronous uploads - * @param $comment string - * @param $pageText string - * @param $watch bool - * @param $user User + * @param string $comment + * @param string $pageText + * @param bool $watch + * @param User $user * @return Status */ public function performUpload( $comment, $pageText, $watch, $user ) { @@ -343,11 +357,11 @@ class UploadFromUrl extends UploadBase { } /** - * @param $comment - * @param $pageText - * @param $watch - * @param $user User - * @return String + * @param string $comment + * @param string $pageText + * @param bool $watch + * @param User $user + * @return string */ protected function insertJob( $comment, $pageText, $watch, $user ) { $sessionKey = $this->stashSession(); @@ -364,7 +378,7 @@ class UploadFromUrl extends UploadBase { ) ); $job->initializeSessionData(); JobQueueGroup::singleton()->push( $job ); + return $sessionKey; } - } diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php index ea117378..7d80b448 100644 --- a/includes/upload/UploadStash.php +++ b/includes/upload/UploadStash.php @@ -23,26 +23,35 @@ /** * UploadStash is intended to accomplish a few things: - * - enable applications to temporarily stash files without publishing them to the wiki. - * - Several parts of MediaWiki do this in similar ways: UploadBase, UploadWizard, and FirefoggChunkedExtension - * And there are several that reimplement stashing from scratch, in idiosyncratic ways. The idea is to unify them all here. - * Mostly all of them are the same except for storing some custom fields, which we subsume into the data array. - * - enable applications to find said files later, as long as the db table or temp files haven't been purged. - * - enable the uploading user (and *ONLY* the uploading user) to access said files, and thumbnails of said files, via a URL. - * We accomplish this using a database table, with ownership checking as you might expect. See SpecialUploadStash, which - * implements a web interface to some files stored this way. + * - Enable applications to temporarily stash files without publishing them to + * the wiki. + * - Several parts of MediaWiki do this in similar ways: UploadBase, + * UploadWizard, and FirefoggChunkedExtension. + * And there are several that reimplement stashing from scratch, in + * idiosyncratic ways. The idea is to unify them all here. + * Mostly all of them are the same except for storing some custom fields, + * which we subsume into the data array. + * - Enable applications to find said files later, as long as the db table or + * temp files haven't been purged. + * - Enable the uploading user (and *ONLY* the uploading user) to access said + * files, and thumbnails of said files, via a URL. We accomplish this using + * a database table, with ownership checking as you might expect. See + * SpecialUploadStash, which implements a web interface to some files stored + * this way. * - * UploadStash right now is *mostly* intended to show you one user's slice of the entire stash. The user parameter is only optional - * because there are few cases where we clean out the stash from an automated script. In the future we might refactor this. + * UploadStash right now is *mostly* intended to show you one user's slice of + * the entire stash. The user parameter is only optional because there are few + * cases where we clean out the stash from an automated script. In the future we + * might refactor this. * * UploadStash represents the entire stash of temporary files. * UploadStashFile is a filestore for the actual physical disk files. - * UploadFromStash extends UploadBase, and represents a single stashed file as it is moved from the stash to the regular file repository + * UploadFromStash extends UploadBase, and represents a single stashed file as + * it is moved from the stash to the regular file repository * * @ingroup Upload */ class UploadStash { - // Format of the key for files -- has to be suitable as a filename itself (e.g. ab12cd34ef.jpg) const KEY_FORMAT_REGEX = '/^[\w-\.]+\.\w*$/'; @@ -71,8 +80,8 @@ class UploadStash { * Designed to be compatible with the session stashing code in UploadBase * (should replace it eventually). * - * @param $repo FileRepo - * @param $user User (default null) + * @param FileRepo $repo + * @param User $user (default null) */ public function __construct( FileRepo $repo, $user = null ) { // this might change based on wiki's configuration. @@ -95,10 +104,11 @@ class UploadStash { /** * Get a file and its metadata from the stash. - * The noAuth param is a bit janky but is required for automated scripts which clean out the stash. + * The noAuth param is a bit janky but is required for automated scripts + * which clean out the stash. * - * @param string $key key under which file information is stored - * @param $noAuth Boolean (optional) Don't check authentication. Used by maintenance scripts. + * @param string $key Key under which file information is stored + * @param bool $noAuth (optional) Don't check authentication. Used by maintenance scripts. * @throws UploadStashFileNotFoundException * @throws UploadStashNotLoggedInException * @throws UploadStashWrongOwnerException @@ -106,7 +116,7 @@ class UploadStash { * @return UploadStashFile */ public function getFile( $key, $noAuth = false ) { - if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) { + if ( !preg_match( self::KEY_FORMAT_REGEX, $key ) ) { throw new UploadStashBadPathException( "key '$key' is not in a proper format" ); } @@ -117,7 +127,8 @@ class UploadStash { if ( !isset( $this->fileMetadata[$key] ) ) { if ( !$this->fetchFileMetadata( $key ) ) { - // If nothing was received, it's likely due to replication lag. Check the master to see if the record is there. + // If nothing was received, it's likely due to replication lag. + // Check the master to see if the record is there. $this->fetchFileMetadata( $key, DB_MASTER ); } @@ -138,14 +149,15 @@ class UploadStash { } } - if ( ! $this->files[$key]->exists() ) { + if ( !$this->files[$key]->exists() ) { wfDebug( __METHOD__ . " tried to get file at $key, but it doesn't exist\n" ); throw new UploadStashBadPathException( "path doesn't exist" ); } if ( !$noAuth ) { if ( $this->fileMetadata[$key]['us_user'] != $this->userId ) { - throw new UploadStashWrongOwnerException( "This file ($key) doesn't belong to the current user." ); + throw new UploadStashWrongOwnerException( "This file ($key) doesn't " + . "belong to the current user." ); } } @@ -155,34 +167,38 @@ class UploadStash { /** * Getter for file metadata. * - * @param string $key key under which file information is stored - * @return Array + * @param string $key Key under which file information is stored + * @return array */ public function getMetadata( $key ) { $this->getFile( $key ); + return $this->fileMetadata[$key]; } /** * Getter for fileProps * - * @param string $key key under which file information is stored - * @return Array + * @param string $key Key under which file information is stored + * @return array */ public function getFileProps( $key ) { $this->getFile( $key ); + return $this->fileProps[$key]; } /** - * Stash a file in a temp directory and record that we did this in the database, along with other metadata. + * Stash a file in a temp directory and record that we did this in the + * database, along with other metadata. * - * @param string $path path to file you want stashed - * @param string $sourceType the type of upload that generated this file (currently, I believe, 'file' or null) + * @param string $path Path to file you want stashed + * @param string $sourceType The type of upload that generated this file + * (currently, I believe, 'file' or null) * @throws UploadStashBadPathException * @throws UploadStashFileException * @throws UploadStashNotLoggedInException - * @return UploadStashFile: file, or null on failure + * @return UploadStashFile|null File, or null on failure */ public function stashFile( $path, $sourceType = null ) { if ( !is_file( $path ) ) { @@ -201,10 +217,11 @@ class UploadStash { $pathWithGoodExtension = $path; } - // If no key was supplied, make one. a mysql insertid would be totally reasonable here, except - // that for historical reasons, the key is this random thing instead. At least it's not guessable. + // If no key was supplied, make one. a mysql insertid would be totally + // reasonable here, except that for historical reasons, the key is this + // random thing instead. At least it's not guessable. // - // some things that when combined will make a suitably unique key. + // Some things that when combined will make a suitably unique key. // see: http://www.jwz.org/doc/mid.html list( $usec, $sec ) = explode( ' ', microtime() ); $usec = substr( $usec, 2 ); @@ -215,7 +232,7 @@ class UploadStash { $this->fileProps[$key] = $fileProps; - if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) { + if ( !preg_match( self::KEY_FORMAT_REGEX, $key ) ) { throw new UploadStashBadPathException( "key '$key' is not in a proper format" ); } @@ -224,30 +241,36 @@ class UploadStash { // if not already in a temporary area, put it there $storeStatus = $this->repo->storeTemp( basename( $pathWithGoodExtension ), $path ); - if ( ! $storeStatus->isOK() ) { - // It is a convention in MediaWiki to only return one error per API exception, even if multiple errors - // are available. We use reset() to pick the "first" thing that was wrong, preferring errors to warnings. - // This is a bit lame, as we may have more info in the $storeStatus and we're throwing it away, but to fix it means + if ( !$storeStatus->isOK() ) { + // It is a convention in MediaWiki to only return one error per API + // exception, even if multiple errors are available. We use reset() + // to pick the "first" thing that was wrong, preferring errors to + // warnings. This is a bit lame, as we may have more info in the + // $storeStatus and we're throwing it away, but to fix it means // redesigning API errors significantly. - // $storeStatus->value just contains the virtual URL (if anything) which is probably useless to the caller + // $storeStatus->value just contains the virtual URL (if anything) + // which is probably useless to the caller. $error = $storeStatus->getErrorsArray(); $error = reset( $error ); - if ( ! count( $error ) ) { + if ( !count( $error ) ) { $error = $storeStatus->getWarningsArray(); $error = reset( $error ); - if ( ! count( $error ) ) { + if ( !count( $error ) ) { $error = array( 'unknown', 'no error recorded' ); } } - // at this point, $error should contain the single "most important" error, plus any parameters. + // At this point, $error should contain the single "most important" + // error, plus any parameters. $errorMsg = array_shift( $error ); - throw new UploadStashFileException( "Error storing file in '$path': " . wfMessage( $errorMsg, $error )->text() ); + throw new UploadStashFileException( "Error storing file in '$path': " + . wfMessage( $errorMsg, $error )->text() ); } $stashPath = $storeStatus->value; // fetch the current user ID if ( !$this->isLoggedIn ) { - throw new UploadStashNotLoggedInException( __METHOD__ . ' No user is logged in, files must belong to users' ); + throw new UploadStashNotLoggedInException( __METHOD__ + . ' No user is logged in, files must belong to users' ); } // insert the file metadata into the db. @@ -279,7 +302,8 @@ class UploadStash { __METHOD__ ); - // store the insertid in the class variable so immediate retrieval (possibly laggy) isn't necesary. + // store the insertid in the class variable so immediate retrieval + // (possibly laggy) isn't necesary. $this->fileMetadata[$key]['us_id'] = $dbw->insertId(); # create the UploadStashFile object for this file. @@ -293,11 +317,12 @@ class UploadStash { * Does not clean up files in the repo, just the record of them. * * @throws UploadStashNotLoggedInException - * @return boolean: success + * @return bool Success */ public function clear() { if ( !$this->isLoggedIn ) { - throw new UploadStashNotLoggedInException( __METHOD__ . ' No user is logged in, files must belong to users' ); + throw new UploadStashNotLoggedInException( __METHOD__ + . ' No user is logged in, files must belong to users' ); } wfDebug( __METHOD__ . ' clearing all rows for user ' . $this->userId . "\n" ); @@ -318,19 +343,21 @@ class UploadStash { /** * Remove a particular file from the stash. Also removes it from the repo. * - * @param $key - * @throws UploadStashNoSuchKeyException|UploadStashNotLoggedInException|UploadStashWrongOwnerException - * @return boolean: success + * @param string $key + * @throws UploadStashNoSuchKeyException|UploadStashNotLoggedInException + * @throws UploadStashWrongOwnerException + * @return bool Success */ public function removeFile( $key ) { if ( !$this->isLoggedIn ) { - throw new UploadStashNotLoggedInException( __METHOD__ . ' No user is logged in, files must belong to users' ); + throw new UploadStashNotLoggedInException( __METHOD__ + . ' No user is logged in, files must belong to users' ); } $dbw = $this->repo->getMasterDb(); - // this is a cheap query. it runs on the master so that this function still works when there's lag. - // it won't be called all that often. + // this is a cheap query. it runs on the master so that this function + // still works when there's lag. It won't be called all that often. $row = $dbw->selectRow( 'uploadstash', 'us_user', @@ -343,7 +370,8 @@ class UploadStash { } if ( $row->us_user != $this->userId ) { - throw new UploadStashWrongOwnerException( "Can't delete: the file ($key) doesn't belong to this user." ); + throw new UploadStashWrongOwnerException( "Can't delete: " + . "the file ($key) doesn't belong to this user." ); } return $this->removeFileNoAuth( $key ); @@ -352,7 +380,8 @@ class UploadStash { /** * Remove a file (see removeFile), but doesn't check ownership first. * - * @return boolean: success + * @param string $key + * @return bool Success */ public function removeFileNoAuth( $key ) { wfDebug( __METHOD__ . " clearing row $key\n" ); @@ -368,8 +397,9 @@ class UploadStash { __METHOD__ ); - // TODO: look into UnregisteredLocalFile and find out why the rv here is sometimes wrong (false when file was removed) - // for now, ignore. + /** @todo Look into UnregisteredLocalFile and find out why the rv here is + * sometimes wrong (false when file was removed). For now, ignore. + */ $this->files[$key]->remove(); unset( $this->files[$key] ); @@ -382,11 +412,12 @@ class UploadStash { * List all files in the stash. * * @throws UploadStashNotLoggedInException - * @return Array + * @return array */ public function listFiles() { if ( !$this->isLoggedIn ) { - throw new UploadStashNotLoggedInException( __METHOD__ . ' No user is logged in, files must belong to users' ); + throw new UploadStashNotLoggedInException( __METHOD__ + . ' No user is logged in, files must belong to users' ); } $dbr = $this->repo->getSlaveDb(); @@ -412,12 +443,12 @@ class UploadStash { } /** - * Find or guess extension -- ensuring that our extension matches our mime type. + * Find or guess extension -- ensuring that our extension matches our MIME type. * Since these files are constructed from php tempnames they may not start off * with an extension. * XXX this is somewhat redundant with the checks that ApiUpload.php does with incoming * uploads versus the desired filename. Maybe we can get that passed to us... - * @param $path + * @param string $path * @throws UploadStashFileException * @return string */ @@ -429,7 +460,7 @@ class UploadStash { if ( $n !== false ) { $extension = $n ? substr( $path, $n + 1 ) : ''; } else { - // If not, assume that it should be related to the mime type of the original file. + // If not, assume that it should be related to the MIME type of the original file. $magic = MimeMagic::singleton(); $mimeType = $magic->guessMimeType( $path ); $extensions = explode( ' ', MimeMagic::singleton()->getExtensionsForType( $mimeType ) ); @@ -450,15 +481,16 @@ class UploadStash { // put it in a web accesible directory. return ''; } + return $extension; } /** * Helper function: do the actual database query to fetch file metadata. * - * @param string $key key - * @param $readFromDB: constant (default: DB_SLAVE) - * @return boolean + * @param string $key + * @param int $readFromDB Constant (default: DB_SLAVE) + * @return bool */ protected function fetchFileMetadata( $key, $readFromDB = DB_SLAVE ) { // populate $fileMetadata[$key] @@ -483,6 +515,7 @@ class UploadStash { } $this->fileMetadata[$key] = (array)$row; + $this->fileMetadata[$key]['us_props'] = $dbr->decodeBlob( $row->us_props ); return true; } @@ -490,7 +523,7 @@ class UploadStash { /** * Helper function: Initialize the UploadStashFile for a given file. * - * @param string $key key under which to store the object + * @param string $key Key under which to store the object * @throws UploadStashZeroLengthFileException * @return bool */ @@ -500,6 +533,7 @@ class UploadStash { throw new UploadStashZeroLengthFileException( "File is zero length" ); } $this->files[$key] = $file; + return true; } } @@ -510,12 +544,14 @@ class UploadStashFile extends UnregisteredLocalFile { protected $url; /** - * A LocalFile wrapper around a file that has been temporarily stashed, so we can do things like create thumbnails for it - * Arguably UnregisteredLocalFile should be handling its own file repo but that class is a bit retarded currently + * A LocalFile wrapper around a file that has been temporarily stashed, + * so we can do things like create thumbnails for it. Arguably + * UnregisteredLocalFile should be handling its own file repo but that + * class is a bit retarded currently. * - * @param $repo FileRepo: repository where we should find the path - * @param string $path path to file - * @param string $key key to store the path and any stashed data under + * @param FileRepo $repo Repository where we should find the path + * @param string $path Path to file + * @param string $key Key to store the path and any stashed data under * @throws UploadStashBadPathException * @throws UploadStashFileNotFoundException */ @@ -526,18 +562,21 @@ class UploadStashFile extends UnregisteredLocalFile { if ( $repo->isVirtualUrl( $path ) ) { $path = $repo->resolveVirtualUrl( $path ); } else { - - // check if path appears to be sane, no parent traversals, and is in this repo's temp zone. + // check if path appears to be sane, no parent traversals, + // and is in this repo's temp zone. $repoTempPath = $repo->getZonePath( 'temp' ); - if ( ( ! $repo->validateFilename( $path ) ) || - ( strpos( $path, $repoTempPath ) !== 0 ) ) { - wfDebug( "UploadStash: tried to construct an UploadStashFile from a file that should already exist at '$path', but path is not valid\n" ); + if ( ( !$repo->validateFilename( $path ) ) || + ( strpos( $path, $repoTempPath ) !== 0 ) + ) { + wfDebug( "UploadStash: tried to construct an UploadStashFile " + . "from a file that should already exist at '$path', but path is not valid\n" ); throw new UploadStashBadPathException( 'path is not valid' ); } // check if path exists! and is a plain file. - if ( ! $repo->fileExists( $path ) ) { - wfDebug( "UploadStash: tried to construct an UploadStashFile from a file that should already exist at '$path', but path is not found\n" ); + if ( !$repo->fileExists( $path ) ) { + wfDebug( "UploadStash: tried to construct an UploadStashFile from " + . "a file that should already exist at '$path', but path is not found\n" ); throw new UploadStashFileNotFoundException( 'cannot find path, or not a plain file' ); } } @@ -550,10 +589,10 @@ class UploadStashFile extends UnregisteredLocalFile { /** * A method needed by the file transforming and scaling routines in File.php * We do not necessarily care about doing the description at this point - * However, we also can't return the empty string, as the rest of MediaWiki demands this (and calls to imagemagick - * convert require it to be there) + * However, we also can't return the empty string, as the rest of MediaWiki + * demands this (and calls to imagemagick convert require it to be there) * - * @return String: dummy value + * @return string Dummy value */ public function getDescriptionUrl() { return $this->getUrl(); @@ -564,14 +603,17 @@ class UploadStashFile extends UnregisteredLocalFile { * The actual argument is the result of thumbName although we seem to have * buggy code elsewhere that expects a boolean 'suffix' * - * @param string $thumbName name of thumbnail (e.g. "120px-123456.jpg" ), or false to just get the path - * @return String: path thumbnail should take on filesystem, or containing directory if thumbname is false + * @param string $thumbName Name of thumbnail (e.g. "120px-123456.jpg" ), + * or false to just get the path + * @return string Path thumbnail should take on filesystem, or containing + * directory if thumbname is false */ public function getThumbPath( $thumbName = false ) { $path = dirname( $this->path ); if ( $thumbName !== false ) { $path .= "/$thumbName"; } + return $path; } @@ -580,18 +622,19 @@ class UploadStashFile extends UnregisteredLocalFile { * We override this because we want to use the pretty url name instead of the * ugly file name. * - * @param array $params handler-specific parameters - * @param $flags integer Bitfield that supports THUMB_* constants - * @return String: base name for URL, like '120px-12345.jpg', or null if there is no handler + * @param array $params Handler-specific parameters + * @param int $flags Bitfield that supports THUMB_* constants + * @return string Base name for URL, like '120px-12345.jpg', or null if there is no handler */ function thumbName( $params, $flags = 0 ) { return $this->generateThumbName( $this->getUrlName(), $params ); } /** - * Helper function -- given a 'subpage', return the local URL e.g. /wiki/Special:UploadStash/subpage - * @param $subPage String - * @return String: local URL for this subpage in the Special:UploadStash space. + * Helper function -- given a 'subpage', return the local URL, + * e.g. /wiki/Special:UploadStash/subpage + * @param string $subPage + * @return string Local URL for this subpage in the Special:UploadStash space. */ private function getSpecialUrl( $subPage ) { return SpecialPage::getTitleFor( 'UploadStash', $subPage )->getLocalURL(); @@ -600,14 +643,16 @@ class UploadStashFile extends UnregisteredLocalFile { /** * Get a URL to access the thumbnail * This is required because the model of how files work requires that - * the thumbnail urls be predictable. However, in our model the URL is not based on the filename - * (that's hidden in the db) + * the thumbnail urls be predictable. However, in our model the URL is + * not based on the filename (that's hidden in the db) * - * @param string $thumbName basename of thumbnail file -- however, we don't want to use the file exactly - * @return String: URL to access thumbnail, or URL with partial path + * @param string $thumbName Basename of thumbnail file -- however, we don't + * want to use the file exactly + * @return string URL to access thumbnail, or URL with partial path */ public function getThumbUrl( $thumbName = false ) { wfDebug( __METHOD__ . " getting for $thumbName \n" ); + return $this->getSpecialUrl( 'thumb/' . $this->getUrlName() . '/' . $thumbName ); } @@ -615,12 +660,13 @@ class UploadStashFile extends UnregisteredLocalFile { * The basename for the URL, which we want to not be related to the filename. * Will also be used as the lookup key for a thumbnail file. * - * @return String: base url name, like '120px-123456.jpg' + * @return string Base url name, like '120px-123456.jpg' */ public function getUrlName() { - if ( ! $this->urlName ) { + if ( !$this->urlName ) { $this->urlName = $this->fileKey; } + return $this->urlName; } @@ -628,29 +674,32 @@ class UploadStashFile extends UnregisteredLocalFile { * Return the URL of the file, if for some reason we wanted to download it * We tend not to do this for the original file, but we do want thumb icons * - * @return String: url + * @return string Url */ public function getUrl() { if ( !isset( $this->url ) ) { $this->url = $this->getSpecialUrl( 'file/' . $this->getUrlName() ); } + return $this->url; } /** - * Parent classes use this method, for no obvious reason, to return the path (relative to wiki root, I assume). - * But with this class, the URL is unrelated to the path. + * Parent classes use this method, for no obvious reason, to return the path + * (relative to wiki root, I assume). But with this class, the URL is + * unrelated to the path. * - * @return String: url + * @return string Url */ public function getFullUrl() { return $this->getUrl(); } /** - * Getter for file key (the unique id by which this file's location & metadata is stored in the db) + * Getter for file key (the unique id by which this file's location & + * metadata is stored in the db) * - * @return String: file key + * @return string File key */ public function getFileKey() { return $this->fileKey; @@ -658,7 +707,7 @@ class UploadStashFile extends UnregisteredLocalFile { /** * Remove the associated temporary file - * @return Status: success + * @return status Success */ public function remove() { if ( !$this->repo->fileExists( $this->path ) ) { @@ -672,15 +721,32 @@ class UploadStashFile extends UnregisteredLocalFile { public function exists() { return $this->repo->fileExists( $this->path ); } +} + +class UploadStashException extends MWException { +} + +class UploadStashNotAvailableException extends UploadStashException { +} + +class UploadStashFileNotFoundException extends UploadStashException { +} + +class UploadStashBadPathException extends UploadStashException { +} + +class UploadStashFileException extends UploadStashException { +} + +class UploadStashZeroLengthFileException extends UploadStashException { +} + +class UploadStashNotLoggedInException extends UploadStashException { +} + +class UploadStashWrongOwnerException extends UploadStashException { +} +class UploadStashNoSuchKeyException extends UploadStashException { } -class UploadStashException extends MWException {}; -class UploadStashNotAvailableException extends UploadStashException {}; -class UploadStashFileNotFoundException extends UploadStashException {}; -class UploadStashBadPathException extends UploadStashException {}; -class UploadStashFileException extends UploadStashException {}; -class UploadStashZeroLengthFileException extends UploadStashException {}; -class UploadStashNotLoggedInException extends UploadStashException {}; -class UploadStashWrongOwnerException extends UploadStashException {}; -class UploadStashNoSuchKeyException extends UploadStashException {}; |