From: Ian Baker Date: Tue, 12 Jul 2011 21:11:43 +0000 (+0000) Subject: Refactored UploadStash and related classes to use the database for file metadata... X-Git-Tag: 1.31.0-rc.0~28894 X-Git-Url: http://git.cyclocoop.org/%28?a=commitdiff_plain;h=9d4fd0c5674bdd01f8f34c5f00e848632ab77747;p=lhc%2Fweb%2Fwiklou.git Refactored UploadStash and related classes to use the database for file metadata storage instead of the session, see bug 26179 Tweaked the UploadWizard to work properly with the new backend code, updated tests --- diff --git a/includes/api/ApiQueryStashImageInfo.php b/includes/api/ApiQueryStashImageInfo.php index e8dea83487..43f925a963 100644 --- a/includes/api/ApiQueryStashImageInfo.php +++ b/includes/api/ApiQueryStashImageInfo.php @@ -51,7 +51,7 @@ class ApiQueryStashImageInfo extends ApiQueryImageInfo { $result->addValue( array( 'query', $this->getModuleName() ), null, $imageInfo ); $result->setIndexedTagName_internal( array( 'query', $this->getModuleName() ), $modulePrefix ); } - + //TODO: update exception handling here to understand current getFile exceptions } catch ( UploadStashNotAvailableException $e ) { $this->dieUsage( "Session not available: " . $e->getMessage(), "nosession" ); } catch ( UploadStashFileNotFoundException $e ) { diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 8b302e4eff..6a5a1ac781 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -58,6 +58,11 @@ class ApiUpload extends ApiBase { $request = $this->getMain()->getRequest(); // Add the uploaded file to the params array $this->mParams['file'] = $request->getFileName( 'file' ); + + // Copy the session key to the file key, for backward compatibility. + if( !$this->mParams['filekey'] && $this->mParams['sessionkey'] ) { + $this->mParams['filekey'] = $this->mParams['sessionkey']; + } // Select an upload module if ( !$this->selectUploadModule() ) { @@ -103,7 +108,8 @@ class ApiUpload extends ApiBase { // in case the warnings can be fixed with some further user action, let's stash this upload // and return a key they can use to restart it try { - $result['sessionkey'] = $this->performStash(); + $result['filekey'] = $this->performStash(); + $result['sessionkey'] = $result['filekey']; // backwards compatibility } catch ( MWException $e ) { $result['warnings']['stashfailed'] = $e->getMessage(); } @@ -112,7 +118,8 @@ class ApiUpload extends ApiBase { // In this case, a failure to stash ought to be fatal try { $result['result'] = 'Success'; - $result['sessionkey'] = $this->performStash(); + $result['filekey'] = $this->performStash(); + $result['sessionkey'] = $result['filekey']; // backwards compatibility } catch ( MWException $e ) { $this->dieUsage( $e->getMessage(), 'stashfailed' ); } @@ -133,18 +140,20 @@ class ApiUpload extends ApiBase { } /** - * Stash the file and return the session key + * Stash the file and return the file key * Also re-raises exceptions with slightly more informative message strings (useful for API) * @throws MWException - * @return String session key + * @return String file key */ function performStash() { try { - $sessionKey = $this->mUpload->stashSessionFile()->getSessionKey(); + $fileKey = $this->mUpload->stashFile()->getFileKey(); } catch ( MWException $e ) { - throw new MWException( 'Stashing temporary file failed: ' . get_class( $e ) . ' ' . $e->getMessage() ); + $message = 'Stashing temporary file failed: ' . get_class( $e ) . ' ' . $e->getMessage(); + wfDebug( __METHOD__ . ' ' . $message . "\n"); + throw new MWException( $message ); } - return $sessionKey; + return $fileKey; } /** @@ -158,7 +167,8 @@ class ApiUpload extends ApiBase { */ function dieRecoverableError( $error, $parameter, $data = array() ) { try { - $data['sessionkey'] = $this->performStash(); + $data['filekey'] = $this->performStash(); + $data['sessionkey'] = $data['filekey']; } catch ( MWException $e ) { $data['stashfailed'] = $e->getMessage(); } @@ -180,7 +190,7 @@ class ApiUpload extends ApiBase { // One and only one of the following parameters is needed $this->requireOnlyOneParameter( $this->mParams, - 'sessionkey', 'file', 'url', 'statuskey' ); + 'filekey', 'file', 'url', 'statuskey' ); if ( $this->mParams['statuskey'] ) { $this->checkAsyncDownloadEnabled(); @@ -204,17 +214,14 @@ class ApiUpload extends ApiBase { $this->dieUsageMsg( array( 'missingparam', 'filename' ) ); } - if ( $this->mParams['sessionkey'] ) { + if ( $this->mParams['filekey'] ) { // Upload stashed in a previous request - $sessionData = $request->getSessionData( UploadBase::getSessionKeyName() ); - if ( !UploadFromStash::isValidSessionKey( $this->mParams['sessionkey'], $sessionData ) ) { - $this->dieUsageMsg( 'invalid-session-key' ); + if ( !UploadFromStash::isValidKey( $this->mParams['filekey'] ) ) { + $this->dieUsageMsg( 'invalid-file-key' ); } $this->mUpload = new UploadFromStash(); - $this->mUpload->initialize( $this->mParams['filename'], - $this->mParams['sessionkey'], - $sessionData[$this->mParams['sessionkey']] ); + $this->mUpload->initialize( $this->mParams['filekey'], $this->mParams['filename'] ); } elseif ( isset( $this->mParams['file'] ) ) { $this->mUpload = new UploadFromFile(); @@ -463,7 +470,11 @@ class ApiUpload extends ApiBase { 'ignorewarnings' => false, 'file' => null, 'url' => null, - 'sessionkey' => null, + 'filekey' => null, + 'sessionkey' => array( + ApiBase::PARAM_DFLT => null, + ApiBase::PARAM_DEPRECATED => true, + ), 'stash' => false, 'asyncdownload' => false, @@ -485,12 +496,13 @@ class ApiUpload extends ApiBase { 'ignorewarnings' => 'Ignore any warnings', 'file' => 'File contents', 'url' => 'Url to fetch the file from', - 'sessionkey' => 'Session key that identifies a previous upload that was stashed temporarily.', + 'filekey' => 'Key that identifies a previous upload that was stashed temporarily.', + 'sessionkey' => 'Same as filekey, maintained for backward compatibility.', 'stash' => 'If set, the server will not add the file to the repository and stash it temporarily.', 'asyncdownload' => 'Make fetching a URL asynchronous', 'leavemessage' => 'If asyncdownload is used, leave a message on the user talk page if finished', - 'statuskey' => 'Fetch the upload status for this session key', + 'statuskey' => 'Fetch the upload status for this file key', ); return $params; @@ -502,20 +514,18 @@ class ApiUpload extends ApiBase { 'Upload a file, or get the status of pending uploads. Several methods are available:', ' * Upload file contents directly, using the "file" parameter', ' * Have the MediaWiki server fetch a file from a URL, using the "url" parameter', - ' * Complete an earlier upload that failed due to warnings, using the "sessionkey" parameter', + ' * Complete an earlier upload that failed due to warnings, using the "filekey" parameter', 'Note that the HTTP POST must be done as a file upload (i.e. using multipart/form-data) when', - 'sending the "file". Note also that queries using session keys must be', - 'done in the same login session as the query that originally returned the key (i.e. do not', - 'log out and then log back in). Also you must get and send an edit token before doing any upload stuff' + 'sending the "file". Also you must get and send an edit token before doing any upload stuff' ); } public function getPossibleErrors() { return array_merge( parent::getPossibleErrors(), - $this->getRequireOnlyOneParameterErrorMessages( array( 'sessionkey', 'file', 'url', 'statuskey' ) ), + $this->getRequireOnlyOneParameterErrorMessages( array( 'filekey', 'file', 'url', 'statuskey' ) ), array( array( 'uploaddisabled' ), - array( 'invalid-session-key' ), + array( 'invalid-file-key' ), array( 'uploaddisabled' ), array( 'mustbeloggedin', 'upload' ), array( 'badaccess-groups' ), @@ -545,7 +555,7 @@ class ApiUpload extends ApiBase { 'Upload from a URL:', ' api.php?action=upload&filename=Wiki.png&url=http%3A//upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png', 'Complete an upload that failed due to warnings:', - ' api.php?action=upload&filename=Wiki.png&sessionkey=sessionkey&ignorewarnings=1', + ' api.php?action=upload&filename=Wiki.png&filekey=filekey&ignorewarnings=1', ); } diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index eb8a73d152..32adcf7c39 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -184,6 +184,7 @@ class MysqlUpdater extends DatabaseUpdater { // 1.19 array( 'addTable', 'config', 'patch-config.sql' ), + array( 'addTable', 'uploadstash', 'patch-uploadstash.sql' ), ); } diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index b3a8a212cf..4031293ae9 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -61,6 +61,7 @@ class SqliteUpdater extends DatabaseUpdater { // 1.19 array( 'addTable', 'config', 'patch-config.sql' ), + array( 'addTable', 'uploadstash', 'patch-uploadstash.sql' ), ); } diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 889c759fda..67d275ff79 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -38,13 +38,6 @@ abstract class UploadBase { const FILE_TOO_LARGE = 12; const WINDOWS_NONASCII_FILENAME = 13; - const SESSION_VERSION = 2; - const SESSION_KEYNAME = 'wsUploadData'; - - public static function getSessionKeyname() { - return self::SESSION_KEYNAME; - } - public function getVerificationErrorCode( $error ) { $code_to_status = array(self::EMPTY_FILE => 'empty-file', self::FILE_TOO_LARGE => 'file-too-large', @@ -745,32 +738,40 @@ abstract class UploadBase { * 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 user's session. - * This method returns the file object, which also has a 'sessionKey' property which can be passed through a form or + * 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 $key String: (optional) the session key used to find the file info again. If not supplied, a key will be autogenerated. + * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated. * @return UploadStashFile stashed file */ - public function stashSessionFile( $key = null ) { + public function stashFile( $key = null ) { + // was stashSessionFile $stash = RepoGroup::singleton()->getLocalRepo()->getUploadStash(); - $data = array( - 'mFileProps' => $this->mFileProps, - 'mSourceType' => $this->getSourceType(), - ); - $file = $stash->stashFile( $this->mTempPath, $data, $key ); + + $file = $stash->stashFile( $this->mTempPath, $this->getSourceType(), $key ); $this->mLocalFile = $file; return $file; } /** - * Stash a file in a temporary directory, returning a key which can be used to find the file again. See stashSessionFile(). + * Stash a file in a temporary directory, returning a key which can be used to find the file again. See stashFile(). + * + * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated. + * @return String: file key + */ + public function stashFileGetKey( $key = null ) { + return $this->stashFile( $key )->getFileKey(); + } + + /** + * alias for stashFileGetKey, for backwards compatibility * - * @param $key String: (optional) the session key used to find the file info again. If not supplied, a key will be autogenerated. - * @return String: session key + * @param $key String: (optional) the file key used to find the file info again. If not supplied, a key will be autogenerated. + * @return String: file key */ public function stashSession( $key = null ) { - return $this->stashSessionFile( $key )->getSessionKey(); + return $this->stashFileGetKey( $key ); } /** diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php index 7077bcb4d0..7f3789c17c 100644 --- a/includes/upload/UploadFromStash.php +++ b/includes/upload/UploadFromStash.php @@ -8,16 +8,29 @@ */ class UploadFromStash extends UploadBase { + protected $mFileKey, $mVirtualTempPath, $mFileProps, $mSourceType; + + // an instance of UploadStash + private $stash; + + //LocalFile repo + private $repo; + + public function __construct( $stash = false, $repo = false ) { + if( !$this->repo ) { + $this->repo = RepoGroup::singleton()->getLocalRepo(); + } - protected $initializePathInfo, $mSessionKey, $mVirtualTempPath, - $mFileProps, $mSourceType; - - public static function isValidSessionKey( $key, $sessionData ) { - return !empty( $key ) && - is_array( $sessionData ) && - isset( $sessionData[$key] ) && - isset( $sessionData[$key]['version'] ) && - $sessionData[$key]['version'] == UploadBase::SESSION_VERSION; + if( !$this->stash ) { + $this->stash = new UploadStash( $this->repo ); + } + + return true; + } + + public static function isValidKey( $key ) { + // this is checked in more detail in UploadStash + return preg_match( UploadStash::KEY_FORMAT_REGEX, $key ); } /** @@ -26,45 +39,40 @@ class UploadFromStash extends UploadBase { * @return Boolean */ public static function isValidRequest( $request ) { - $sessionData = $request->getSessionData( UploadBase::SESSION_KEYNAME ); - return self::isValidSessionKey( - $request->getText( 'wpSessionKey' ), - $sessionData - ); + return self::isValidKey( $request->getText( 'wpFileKey' ) || $request->getText( 'wpSessionKey' ) ); } - public function initialize( $name, $sessionKey, $sessionData ) { + public function initialize( $key, $name = 'upload_file' ) { /** * Confirming a temporarily stashed upload. * We don't want path names to be forged, so we keep * them in the session on the server and just give * an opaque key to the user agent. - */ - + */ + $metadata = $this->stash->getMetadata( $key ); $this->initializePathInfo( $name, - $this->getRealPath ( $sessionData['mTempPath'] ), - $sessionData['mFileSize'], + $this->getRealPath ( $metadata['us_path'] ), + $metadata['us_size'], false ); - $this->mSessionKey = $sessionKey; - $this->mVirtualTempPath = $sessionData['mTempPath']; - $this->mFileProps = $sessionData['mFileProps']; - $this->mSourceType = isset( $sessionData['mSourceType'] ) ? - $sessionData['mSourceType'] : null; + $this->mFileKey = $key; + $this->mVirtualTempPath = $metadata['us_path']; + $this->mFileProps = $this->stash->getFileProps( $key ); + $this->mSourceType = $metadata['us_source_type']; } /** * @param $request WebRequest */ public function initializeFromRequest( &$request ) { - $sessionKey = $request->getText( 'wpSessionKey' ); - $sessionData = $request->getSessionData( UploadBase::SESSION_KEYNAME ); + $fileKey = $request->getText( 'wpFileKey' ) || $request->getText( 'wpSessionKey' ); $desiredDestName = $request->getText( 'wpDestFile' ); - if( !$desiredDestName ) - $desiredDestName = $request->getText( 'wpUploadFile' ); - return $this->initialize( $desiredDestName, $sessionKey, $sessionData[$sessionKey] ); + if( !$desiredDestName ) { + $desiredDestName = $request->getText( 'wpUploadFile' ) || $request->getText( 'filename' ); + } + return $this->initialize( $fileKey, $desiredDestName ); } public function getSourceType() { @@ -83,11 +91,18 @@ class UploadFromStash extends UploadBase { /** * There is no need to stash the image twice */ - public function stashSession( $key = null ) { - if ( !empty( $this->mSessionKey ) ) { - return $this->mSessionKey; + public function stashFile( $key = null ) { + if ( !empty( $this->mFileKey ) ) { + return $this->mFileKey; } - return parent::stashSession(); + return parent::stashFileGetKey(); + } + + /** + * Alias for stashFile + */ + public function stashSession( $key = null ) { + return $this->stashFile( $key ); } /** @@ -95,9 +110,7 @@ class UploadFromStash extends UploadBase { * @return success */ public function unsaveUploadedFile() { - $repo = RepoGroup::singleton()->getLocalRepo(); - $success = $repo->freeTemp( $this->mVirtualTempPath ); - return $success; + return $stash->removeFile( $this->mFileKey ); } } \ No newline at end of file diff --git a/includes/upload/UploadStash.php b/includes/upload/UploadStash.php index baa50e6beb..2314a8deca 100644 --- a/includes/upload/UploadStash.php +++ b/includes/upload/UploadStash.php @@ -5,16 +5,23 @@ * - 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 session or temp files haven't been purged. + * - 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 by making the session serve as a URL->file mapping, on the assumption that nobody else can access - * the session, even the uploading user. See SpecialUploadStash, which implements a web interface to some files stored this way. + * 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 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 */ 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*$/'; + + // When a given stashed file can't be loaded, wait for the slaves to catch up. If they're more than MAX_LAG + // behind, throw an exception instead. (at what point is broken better than slow?) + const MAX_LAG = 30; /** * repository that this uses to store temp files @@ -24,92 +31,133 @@ class UploadStash { */ public $repo; - // array of initialized objects obtained from session (lazily initialized upon getFile()) - private $files = array(); - - // TODO: Once UploadBase starts using this, switch to use these constants rather than UploadBase::SESSION* - // const SESSION_VERSION = 2; - // const SESSION_KEYNAME = 'wsUploadData'; + // array of initialized repo objects + protected $files = array(); + + // cache of the file metadata that's stored in the database + protected $fileMetadata = array(); + + // fileprops cache + protected $fileProps = array(); /** - * Represents the session which contains temporarily stored files. + * Represents a temporary filestore, with metadata in the database. * Designed to be compatible with the session stashing code in UploadBase (should replace it eventually) * * @param $repo FileRepo */ public function __construct( $repo ) { - // this might change based on wiki's configuration. $this->repo = $repo; - - if ( ! isset( $_SESSION ) ) { - throw new UploadStashNotAvailableException( 'no session variable' ); - } - - if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME] ) ) { - $_SESSION[UploadBase::SESSION_KEYNAME] = array(); - } - } /** * Get a file and its metadata from the stash. - * May throw exception if session data cannot be parsed due to schema change, or key not found. * - * @param $key Integer: key + * @param $key String: key under which file information is stored * @throws UploadStashFileNotFoundException - * @throws UploadStashBadVersionException + * @throws UploadStashNotLoggedInException + * @throws UploadStashWrongOwnerException + * @throws UploadStashBadPathException * @return UploadStashFile */ public function getFile( $key ) { + global $wgUser; + if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) { throw new UploadStashBadPathException( "key '$key' is not in a proper format" ); } - if ( !isset( $this->files[$key] ) ) { - if ( !isset( $_SESSION[UploadBase::SESSION_KEYNAME][$key] ) ) { - throw new UploadStashFileNotFoundException( "key '$key' not found in stash" ); - } + $userId = $wgUser->getId(); + if( !$userId ) { + throw new UploadStashNotLoggedInException( 'No user is logged in, files must belong to users' ); + } - $data = $_SESSION[UploadBase::SESSION_KEYNAME][$key]; - // guards against PHP class changing while session data doesn't - if ($data['version'] !== UploadBase::SESSION_VERSION ) { - throw new UploadStashBadVersionException( $data['version'] . " does not match current version " . UploadBase::SESSION_VERSION ); + if ( !isset( $this->fileMetadata[$key] ) ) { + // try this first. if it fails to find the row, check for lag, wait, try again. if its still missing, throw an exception. + // this more complex solution keeps things moving for page loads with many requests + // (ie. validating image ownership) when replag is high + if( !$this->fetchFileMetadata($key) ) { + $lag = $dbr->getLag(); + if( $lag > 0 && $lag <= self::MAX_LAG ) { + // if there's not too much replication lag, just wait for the slave to catch up to our last insert. + sleep( ceil( $lag ) ); + } elseif($lag > self::MAX_LAG ) { + // that's a lot of lag to introduce into the middle of the UI. + throw new UploadStashMaxLagExceededException( + 'Couldn\'t load stashed file metadata, and replication lag is above threshold: (MAX_LAG=' . self::MAX_LAG . ')' + ); + } + + // now that the waiting has happened, try again + $this->fetchFileMetadata($key); + } + + if ( !isset( $this->fileMetadata[$key] ) ) { + throw new UploadStashFileNotFoundException( "key '$key' not found in stash" ); } - // separate the stashData into the path, and then the rest of the data - $path = $data['mTempPath']; - unset( $data['mTempPath'] ); + // create $this->files[$key] + $this->initFile( $key ); - $file = new UploadStashFile( $this, $this->repo, $path, $key, $data ); - if ( $file->getSize() === 0 ) { - throw new UploadStashZeroLengthFileException( "File is zero length" ); + // fetch fileprops + $path = $this->fileMetadata[$key]['us_path']; + if ( $this->repo->isVirtualUrl( $path ) ) { + $path = $this->repo->resolveVirtualUrl( $path ); } - $this->files[$key] = $file; - + $this->fileProps[$key] = File::getPropsFromPath( $path ); } + + if( $this->fileMetadata[$key]['us_user'] != $userId ) { + throw new UploadStashWrongOwnerException( "This file ($key) doesn't belong to the current user." ); + } + return $this->files[$key]; } /** - * Stash a file in a temp directory and record that we did this in the session, along with other metadata. - * We store data in a flat key-val namespace because that's how UploadBase did it. This also means we have to - * ensure that the key-val pairs in $data do not overwrite other required fields. + * Getter for file metadata. + * + * @param key String: key under which file information is stored + * @return Array + */ + public function getMetadata ( $key ) { + $this->getFile( $key ); + return $this->fileMetadata[$key]; + } + + /** + * Getter for fileProps + * + * @param key String: 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. * * @param $path String: path to file you want stashed - * @param $data Array: optional, other data you want associated with the file. Do not use 'mTempPath', 'mFileProps', 'mFileSize', or 'version' as keys here - * @param $key String: optional, unique key for this file in this session. Used for directory hashing when storing, otherwise not important + * @param $sourceType String: the type of upload that generated this file (currently, I believe, 'file' or null) + * @param $key String: optional, unique key for this file. Used for directory hashing when storing, otherwise not important * @throws UploadStashBadPathException * @throws UploadStashFileException + * @throws UploadStashNotLoggedInException * @return UploadStashFile: file, or null on failure */ - public function stashFile( $path, $data = array(), $key = null ) { + public function stashFile( $path, $sourceType = null, $key = null ) { + global $wgUser; if ( ! file_exists( $path ) ) { - wfDebug( "UploadStash: tried to stash file at '$path', but it doesn't exist\n" ); + wfDebug( __METHOD__ . " tried to stash file at '$path', but it doesn't exist\n" ); throw new UploadStashBadPathException( "path doesn't exist" ); } $fileProps = File::getPropsFromPath( $path ); + wfDebug( __METHOD__ . " stashing file at '$path'\n" ); + // we will be initializing from some tmpnam files that don't have extensions. // most of MediaWiki assumes all uploaded files have good extensions. So, we fix this. $extension = self::getExtensionForPath( $path ); @@ -127,23 +175,27 @@ class UploadStash { $key = $fileProps['sha1'] . "." . $extension; } + $this->fileProps[$key] = $fileProps; + if ( ! preg_match( self::KEY_FORMAT_REGEX, $key ) ) { throw new UploadStashBadPathException( "key '$key' is not in a proper format" ); } + + wfDebug( __METHOD__ . " key for '$path': $key\n" ); // if not already in a temporary area, put it there - $status = $this->repo->storeTemp( basename( $path ), $path ); + $storeResult = $this->repo->storeTemp( basename( $path ), $path ); - if( ! $status->isOK() ) { + if( ! $storeResult->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 $status and we're throwing it away, but to fix it means + // This is a bit lame, as we may have more info in the $storeResult and we're throwing it away, but to fix it means // redesigning API errors significantly. - // $status->value just contains the virtual URL (if anything) which is probably useless to the caller - $error = $status->getErrorsArray(); + // $storeResult->value just contains the virtual URL (if anything) which is probably useless to the caller + $error = $storeResult->getErrorsArray(); $error = reset( $error ); if ( ! count( $error ) ) { - $error = $status->getWarningsArray(); + $error = $storeResult->getWarningsArray(); $error = reset( $error ); if ( ! count( $error ) ) { $error = array( 'unknown', 'no error recorded' ); @@ -151,52 +203,158 @@ class UploadStash { } throw new UploadStashFileException( "error storing file in '$path': " . implode( '; ', $error ) ); } - $stashPath = $status->value; - - // required info we always store. Must trump any other application info in $data - // 'mTempPath', 'mFileSize', and 'mFileProps' are arbitrary names - // chosen for compatibility with UploadBase's way of doing this. - $requiredData = array( - 'mTempPath' => $stashPath, - 'mFileSize' => $fileProps['size'], - 'mFileProps' => $fileProps, - 'version' => UploadBase::SESSION_VERSION + $stashPath = $storeResult->value; + + // fetch the current user ID + $userId = $wgUser->getId(); + if( !$userId ) { + throw new UploadStashNotLoggedInException( "No user is logged in, files must belong to users" ); + } + + $this->fileMetadata[$key] = array( + 'us_user' => $userId, + 'us_key' => $key, + 'us_orig_path' => $path, + 'us_path' => $stashPath, + 'us_size' => $fileProps['size'], + 'us_sha1' => $fileProps['sha1'], + 'us_mime' => $fileProps['mime'], + 'us_media_type' => $fileProps['media_type'], + 'us_image_width' => $fileProps['width'], + 'us_image_height' => $fileProps['height'], + 'us_image_bits' => $fileProps['bits'], + 'us_source_type' => $sourceType, + 'us_timestamp' => wfTimestamp( TS_MW ) + ); + + // insert the file metadata into the db. + wfDebug( __METHOD__ . " inserting $stashPath under $key\n" ); + $dbw = wfGetDB( DB_MASTER ); + $dbw->insert( + 'uploadstash', + $this->fileMetadata[$key], + __METHOD__ ); - // now, merge required info and extra data into the session. (The extra data changes from application to application. - // UploadWizard wants different things than say FirefoggChunkedUpload.) - wfDebug( __METHOD__ . " storing under $key\n" ); - $_SESSION[UploadBase::SESSION_KEYNAME][$key] = array_merge( $data, $requiredData ); + // 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. + $this->initFile( $key ); + return $this->getFile( $key ); } /** * Remove all files from the stash. * Does not clean up files in the repo, just the record of them. + * + * @throws UploadStashNotLoggedInException * @return boolean: success */ public function clear() { - $_SESSION[UploadBase::SESSION_KEYNAME] = array(); + global $wgUser; + + $userId = $wgUser->getId(); + if( !$userId ) { + throw new UploadStashNotLoggedInException( 'No user is logged in, files must belong to users' ); + } + + wfDebug( __METHOD__ . " clearing all rows for user $userId\n" ); + $dbw = wfGetDB( DB_MASTER ); + $dbw->delete( + 'uploadstash', + array( 'us_user' => $userId ), + __METHOD__ + ); + + # destroy objects. + $this->files = array(); + $this->fileMetadata = array(); + return true; } /** - * Remove a particular file from the stash. - * Does not clean up file in the repo, just the record of it. + * Remove a particular file from the stash. Also removes it from the repo. + * + * @throws UploadStashNotLoggedInException * @return boolean: success */ public function removeFile( $key ) { - unset ( $_SESSION[UploadBase::SESSION_KEYNAME][$key] ); + global $wgUser; + + $userId = $wgUser->getId(); + if( !$userId ) { + throw new UploadStashNotLoggedInException( 'No user is logged in, files must belong to users' ); + } + + wfDebug( __METHOD__ . " clearing row $key for user $userId\n" ); + $dbw = wfGetDB( DB_MASTER ); + + // 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', + array('us_key' => $key), + __METHOD__ + ); + + if( $row->us_user != $userId ) { + throw new UploadStashWrongOwnerException( "Can't delete: the file ($key) doesn't belong to this user." ); + } + + $dbw->delete( + 'uploadstash', + array( 'us_key' => $key, 'us_user' => $userId ), + __METHOD__ + ); + + // 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] ); + unset( $this->fileMetadata[$key] ); + return true; } /** * List all files in the stash. + * + * @throws UploadStashNotLoggedInException + * @return Array */ public function listFiles() { - return array_keys( $_SESSION[UploadBase::SESSION_KEYNAME] ); + global $wgUser; + + $userId = $wgUser->getId(); + if( !$userId ) { + throw new UploadStashNotLoggedInException( 'No user is logged in, files must belong to users' ); + } + + $dbw = wfGetDB( DB_SLAVE ); + $res = $dbr->select( + 'uploadstash', + 'us_key', + array('us_key' => $key), + __METHOD__ + ); + + if( !is_object( $res ) ) { + // nothing there. + return false; + } + + $keys = array(); + while( $row = $dbr->fetchRow( $res ) ) { + array_push( $keys, $row['us_key'] ); + } + + return $keys; } /** @@ -229,12 +387,65 @@ class UploadStash { return File::normalizeExtension( $extension ); } + /** + * Helper function: do the actual database query to fetch file metadata. + * + * @param $key String: key + * @return boolean + */ + protected function fetchFileMetadata( $key ) { + // populate $fileMetadata[$key] + $dbr = wfGetDB( DB_SLAVE ); + $row = $dbr->selectRow( + 'uploadstash', + '*', + array('us_key' => $key), + __METHOD__ + ); + + if( !is_object( $row ) ) { + // key wasn't present in the database. this will happen sometimes. + return false; + } + + $this->fileMetadata[$key] = array( + 'us_user' => $row->us_user, + 'us_key' => $row->us_key, + 'us_orig_path' => $row->us_orig_path, + 'us_path' => $row->us_path, + 'us_size' => $row->us_size, + 'us_sha1' => $row->us_sha1, + 'us_mime' => $row->us_mime, + 'us_media_type' => $row->us_media_type, + 'us_image_width' => $row->us_image_width, + 'us_image_height' => $row->us_image_height, + 'us_image_bits' => $row->us_image_bits, + 'us_source_type' => $row->us_source_type + ); + + return true; + } + + /** + * Helper function: Initialize the UploadStashFile for a given file. + * + * @param $path String: path to file + * @param $key String: key under which to store the object + * @throws UploadStashZeroLengthFileException + * @return bool + */ + protected function initFile( $key ) { + $file = new UploadStashFile( $this->repo, $this->fileMetadata[$key]['us_path'], $key ); + if ( $file->getSize() === 0 ) { + throw new UploadStashZeroLengthFileException( "File is zero length" ); + } + $this->files[$key] = $file; + return true; + } } class UploadStashFile extends UnregisteredLocalFile { - private $sessionStash; - private $sessionKey; - private $sessionData; + private $fileKey; private $urlName; protected $url; @@ -242,18 +453,14 @@ class UploadStashFile extends UnregisteredLocalFile { * 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 $stash UploadStash: useful for obtaining config, stashing transformed files * @param $repo FSRepo: repository where we should find the path * @param $path String: path to file * @param $key String: key to store the path and any stashed data under - * @param $data String: any other data we want stored with this file * @throws UploadStashBadPathException * @throws UploadStashFileNotFoundException */ - public function __construct( $stash, $repo, $path, $key, $data ) { - $this->sessionStash = $stash; - $this->sessionKey = $key; - $this->sessionData = $data; + public function __construct( $repo, $path, $key ) { + $this->fileKey = $key; // resolve mwrepo:// urls if ( $repo->isVirtualUrl( $path ) ) { @@ -333,7 +540,7 @@ 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 session) + * (that's hidden in the db) * * @param $thumbName String: 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 @@ -351,7 +558,7 @@ class UploadStashFile extends UnregisteredLocalFile { */ public function getUrlName() { if ( ! $this->urlName ) { - $this->urlName = $this->sessionKey; + $this->urlName = $this->fileKey; } return $this->urlName; } @@ -380,12 +587,12 @@ class UploadStashFile extends UnregisteredLocalFile { } /** - * Getter for session key (the session-unique id by which this file's location & metadata is stored in the session) + * Getter for file key (the unique id by which this file's location & metadata is stored in the db) * - * @return String: session key + * @return String: file key */ - public function getSessionKey() { - return $this->sessionKey; + public function getFileKey() { + return $this->fileKey; } /** @@ -393,6 +600,11 @@ class UploadStashFile extends UnregisteredLocalFile { * @return Status: success */ public function remove() { + if( !$this->repo->fileExists( $this->path, FileRepo::FILES_ONLY ) ) { + // Maybe the file's already been removed? This could totally happen in UploadBase. + return true; + } + return $this->repo->freeTemp( $this->path ); } @@ -401,7 +613,8 @@ class UploadStashFile extends UnregisteredLocalFile { class UploadStashNotAvailableException extends MWException {}; class UploadStashFileNotFoundException extends MWException {}; class UploadStashBadPathException extends MWException {}; -class UploadStashBadVersionException extends MWException {}; class UploadStashFileException extends MWException {}; class UploadStashZeroLengthFileException extends MWException {}; - +class UploadStashNotLoggedInException extends MWException {}; +class UploadStashWrongOwnerException extends MWException {}; +class UploadStashMaxLagExceededException extends MWException {}; diff --git a/maintenance/archives/patch-uploadstash.sql b/maintenance/archives/patch-uploadstash.sql new file mode 100644 index 0000000000..88a7c7ce54 --- /dev/null +++ b/maintenance/archives/patch-uploadstash.sql @@ -0,0 +1,44 @@ +-- +-- Store information about newly uploaded files before they're +-- moved into the actual filestore +-- +CREATE TABLE /*_*/uploadstash ( + us_id int unsigned NOT NULL PRIMARY KEY auto_increment, + + -- the user who uploaded the file. + us_user int unsigned NOT NULL, + + -- file key. this is how applications actually search for the file. + -- this might go away, or become the primary key. + us_key varchar(255) NOT NULL, + + -- the original path + us_orig_path varchar(255) NOT NULL, + + -- the temporary path at which the file is actually stored + us_path varchar(255) NOT NULL, + + -- which type of upload the file came from (sometimes) + us_source_type varchar(50), + + -- the date/time on which the file was added + us_timestamp varchar(14) not null, + + -- file properties from File::getPropsFromPath. these may prove unnecessary. + -- + us_size int unsigned NOT NULL, + -- this hash comes from File::sha1Base36(), and is 31 characters + us_sha1 varchar(31) NOT NULL, + us_mime varchar(255), + us_media_type varchar(255), + -- image-specific properties + us_image_width smallint unsigned, + us_image_height smallint unsigned, + us_image_bits smallint unsigned + +) /*$wgDBTableOptions*/; + +-- sometimes there's a delete for all of a user's stuff. +CREATE INDEX /*i*/us_user ON /*_*/uploadstash (us_user); +-- pick out files by key, enforce key uniqueness +CREATE UNIQUE INDEX /*i*/us_key ON /*_*/uploadstash (us_key); diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 0880a70205..592bc74343 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -938,6 +938,52 @@ CREATE INDEX /*i*/fa_deleted_timestamp ON /*_*/filearchive (fa_deleted_timestamp CREATE INDEX /*i*/fa_user_timestamp ON /*_*/filearchive (fa_user_text,fa_timestamp); +-- +-- Store information about newly uploaded files before they're +-- moved into the actual filestore +-- +CREATE TABLE /*_*/uploadstash ( + us_id int unsigned NOT NULL PRIMARY KEY auto_increment, + + -- the user who uploaded the file. + us_user int unsigned NOT NULL, + + -- file key. this is how applications actually search for the file. + -- this might go away, or become the primary key. + us_key varchar(255) NOT NULL, + + -- the original path + us_orig_path varchar(255) NOT NULL, + + -- the temporary path at which the file is actually stored + us_path varchar(255) NOT NULL, + + -- which type of upload the file came from (sometimes) + us_source_type varchar(50), + + -- the date/time on which the file was added + us_timestamp varchar(14) not null, + + -- file properties from File::getPropsFromPath. these may prove unnecessary. + -- + us_size int unsigned NOT NULL, + -- this hash comes from File::sha1Base36(), and is 31 characters + us_sha1 varchar(31) NOT NULL, + us_mime varchar(255), + us_media_type varchar(255), + -- image-specific properties + us_image_width smallint unsigned, + us_image_height smallint unsigned, + us_image_bits smallint unsigned + +) /*$wgDBTableOptions*/; + +-- sometimes there's a delete for all of a user's stuff. +CREATE INDEX /*i*/us_user ON /*_*/uploadstash (us_user); +-- pick out files by key, enforce key uniqueness +CREATE UNIQUE INDEX /*i*/us_key ON /*_*/uploadstash (us_key); + + -- -- Primarily a summary table for Special:Recentchanges, -- this table contains some additional info on edits from diff --git a/tests/phpunit/includes/api/ApiUploadTest.php b/tests/phpunit/includes/api/ApiUploadTest.php index e2a31503d7..5c9297845a 100644 --- a/tests/phpunit/includes/api/ApiUploadTest.php +++ b/tests/phpunit/includes/api/ApiUploadTest.php @@ -88,7 +88,7 @@ class ApiUploadTest extends ApiTestCaseUpload { ), $session ); } catch ( UsageException $e ) { $exception = true; - $this->assertEquals( "One of the parameters sessionkey, file, url, statuskey is required", + $this->assertEquals( "One of the parameters filekey, file, url, statuskey is required", $e->getMessage() ); } $this->assertTrue( $exception, "Got exception" ); @@ -398,8 +398,9 @@ class ApiUploadTest extends ApiTestCaseUpload { $this->assertEquals( 'Success', $result['upload']['result'] ); $this->assertEquals( $fileSize, ( int )$result['upload']['imageinfo']['size'] ); $this->assertEquals( $mimeType, $result['upload']['imageinfo']['mime'] ); - $this->assertTrue( isset( $result['upload']['sessionkey'] ) ); - $sessionkey = $result['upload']['sessionkey']; + $this->assertTrue( isset( $result['upload']['filekey'] ) ); + $this->assertEquals( $result['upload']['sessionkey'], $result['upload']['filekey'] ); + $filekey = $result['upload']['filekey']; // it should be visible from Special:UploadStash // XXX ...but how to test this, with a fake WebRequest with the session? @@ -407,12 +408,11 @@ class ApiUploadTest extends ApiTestCaseUpload { // now we should try to release the file from stash $params = array( 'action' => 'upload', - 'sessionkey' => $sessionkey, + 'filekey' => $filekey, 'filename' => $fileName, 'comment' => 'dummy comment', 'text' => "This is the page text for $fileName, altered", ); - $session[ UploadBase::getSessionKeyname() ] = $_SESSION[ UploadBase::getSessionKeyname() ]; $this->clearFakeUploads(); $exception = false; diff --git a/tests/phpunit/includes/upload/UploadStashTest.php b/tests/phpunit/includes/upload/UploadStashTest.php index 3f7863dced..9c39bc6120 100644 --- a/tests/phpunit/includes/upload/UploadStashTest.php +++ b/tests/phpunit/includes/upload/UploadStashTest.php @@ -1,20 +1,38 @@ bug29408File = dirname( __FILE__ ) . '/bug29408'; file_put_contents( $this->bug29408File, "\x00" ); + + self::$users = array( + 'sysop' => new ApiTestUser( + 'Uploadstashtestsysop', + 'Upload Stash Test Sysop', + 'upload_stash_test_sysop@sample.com', + array( 'sysop' ) + ), + 'uploader' => new ApiTestUser( + 'Uploadstashtestuser', + 'Upload Stash Test User', + 'upload_stash_test_user@sample.com', + array() + ) + ); } public function testBug29408() { + global $wgUser; + $wgUser = self::$users['uploader']->user; + $repo = RepoGroup::singleton()->getLocalRepo(); $stash = new UploadStash( $repo ); @@ -23,7 +41,7 @@ class UploadStashTest extends MediaWikiTestCase { // We'll never reach this point if we hit bug 29408 $this->assertTrue( true, 'Unrecognized file without extension' ); - $file->remove(); + $stash->removeFile( $file->getFileKey() ); } public function tearDown() {