From d7e6870b8fcf9aa215702f03c297018684082859 Mon Sep 17 00:00:00 2001 From: Bryan Tong Minh Date: Thu, 29 Jul 2010 13:53:51 +0000 Subject: [PATCH] Made asynchronous upload by URL working, partly. Hid it behind $wgAllowAsyncCopyUploads. If there are no errors then everything works expected; the same if there are unrecoverable errors. User intervention to solve warnings is not yet possible, because $_SESSION is not available in runJobs. This also means that async with leavemessage = false is broken. Other changes: * Moved verifyPermissions check in ApiUpload down pending r70135 implementation in the API. * In User::leaveMessage: append message to end of talk page; add a newline before the heading --- includes/DefaultSettings.php | 5 +++ includes/User.php | 4 +-- includes/api/ApiUpload.php | 56 ++++++++++++++++++++--------- includes/job/UploadFromUrlJob.php | 44 ++++++++++++++++------- includes/upload/UploadFromUrl.php | 60 +++++++++++++++++++++++++++++-- 5 files changed, 136 insertions(+), 33 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 0c5cfc2709..a41b5d210d 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -425,6 +425,11 @@ $wgCacheSharedUploads = true; * The timeout for copy uploads is set by $wgHTTPTimeout. */ $wgAllowCopyUploads = false; +/** + * Allow asynchronous copy uploads. + * This feature is experimental. + */ +$wgAllowAsyncCopyUploads = false; /** * Max size for uploads, in bytes. Applies to all uploads. diff --git a/includes/User.php b/includes/User.php index d7c0bab3d3..ad10fea272 100644 --- a/includes/User.php +++ b/includes/User.php @@ -3767,7 +3767,7 @@ class User { if ( !$template || $template->getNamespace() !== NS_TEMPLATE || !$template->exists() ) { - $text = "== $subject ==\n\n$text\n\n-- $signature"; + $text = "\n== $subject ==\n\n$text\n\n-- $signature"; } else { $text = '{{'. $template->getText() . " | subject=$subject | body=$text | signature=$signature }}"; @@ -3812,7 +3812,7 @@ class User { $flags = $article->checkFlags( $flags ); if ( $flags & EDIT_UPDATE ) { - $text .= $article->getContent(); + $text = $article->getContent() . $text; } $dbw = wfGetDB( DB_MASTER ); diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 4fb182f251..a842311392 100644 --- a/includes/api/ApiUpload.php +++ b/includes/api/ApiUpload.php @@ -59,12 +59,6 @@ class ApiUpload extends ApiBase { // First check permission to upload $this->checkPermissions( $wgUser ); - // Check permission to upload this file - $permErrors = $this->mUpload->verifyPermissions( $wgUser ); - if ( $permErrors !== true ) { - // Todo: more specific error message - $this->dieUsageMsg( array( 'badaccess-groups' ) ); - } // Fetch the file $status = $this->mUpload->fetchFile(); @@ -76,6 +70,13 @@ class ApiUpload extends ApiBase { // Check if the uploaded file is sane $this->verifyUpload(); + + // Check permission to upload this file + $permErrors = $this->mUpload->verifyPermissions( $wgUser ); + if ( $permErrors !== true ) { + // Todo: stash the upload and allow choosing a new name + $this->dieUsageMsg( array( 'badaccess-groups' ) ); + } // Check warnings if necessary $warnings = $this->checkForWarnings(); @@ -198,9 +199,6 @@ class ApiUpload extends ApiBase { $this->dieUsage( 'The filename is not allowed', 'illegal-filename', 0, array( 'filename' => $verification['filtered'] ) ); break; - case UploadBase::OVERWRITE_EXISTING_FILE: - $this->dieUsage( 'Overwriting an existing file is not allowed', 'overwrite' ); - break; case UploadBase::VERIFICATION_ERROR: $this->getResult()->setIndexedTagName( $verification['details'], 'detail' ); $this->dieUsage( 'This file did not pass file verification', 'verification-error', @@ -286,9 +284,19 @@ class ApiUpload extends ApiBase { if ( !$status->isGood() ) { $error = $status->getErrorsArray(); - $this->getResult()->setIndexedTagName( $error, 'error' ); + + if ( count( $error ) == 1 && $error[0][0] == 'async' ) { + // The upload can not be performed right now, because the user + // requested so + return array( + 'result' => 'Queued', + 'sessionkey' => $error[0][1], + ); + } else { + $this->getResult()->setIndexedTagName( $error, 'error' ); - $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error ); + $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error ); + } } $file = $this->mUpload->getLocalFile(); @@ -331,15 +339,22 @@ class ApiUpload extends ApiBase { 'ignorewarnings' => false, 'file' => null, 'url' => null, - 'asyncdownload' => false, - 'leavemessage' => false, + 'sessionkey' => null, ); + + global $wgAllowAsyncCopyUploads; + if ( $wgAllowAsyncCopyUploads ) { + $params += array( + 'asyncdownload' => false, + 'leavemessage' => false, + ); + } return $params; } public function getParamDescription() { - return array( + $params = array( 'filename' => 'Target filename', 'token' => 'Edit token. You can get one of these through prop=info', 'comment' => 'Upload comment. Also used as the initial page text for new files if "text" is not specified', @@ -349,10 +364,19 @@ class ApiUpload extends ApiBase { 'ignorewarnings' => 'Ignore any warnings', 'file' => 'File contents', 'url' => 'Url to fetch the file from', - 'asyncdownload' => 'Make fetching a URL asyncronous', - 'leavemessage' => 'If asyncdownload is used, leave a message on the user talk page if finished', 'sessionkey' => 'Session key returned by a previous upload that failed due to warnings', ); + + global $wgAllowAsyncCopyUploads; + if ( $wgAllowAsyncCopyUploads ) { + $params += array( + 'asyncdownload' => 'Make fetching a URL asynchronous', + 'leavemessage' => 'If asyncdownload is used, leave a message on the user talk page if finished', + ); + } + + return $params; + } public function getDescription() { diff --git a/includes/job/UploadFromUrlJob.php b/includes/job/UploadFromUrlJob.php index 9b297db1d9..d5225b1113 100644 --- a/includes/job/UploadFromUrlJob.php +++ b/includes/job/UploadFromUrlJob.php @@ -10,6 +10,8 @@ * @ingroup JobQueue */ class UploadFromUrlJob extends Job { + public $upload; + protected $user; public function __construct( $title, $params, $id = 0 ) { parent::__construct( 'uploadFromUrl', $title, $params, $id ); @@ -17,8 +19,8 @@ class UploadFromUrlJob extends Job { public function run() { # Initialize this object and the upload object - $upload = new UploadFromUrl(); - $upload->initialize( + $this->upload = new UploadFromUrl(); + $this->upload->initialize( $this->title->getText(), $this->params['url'], false @@ -26,34 +28,47 @@ class UploadFromUrlJob extends Job { $this->user = User::newFromName( $this->params['userName'] ); # Fetch the file - $status = $upload->fetchFile(); + $status = $this->upload->fetchFile(); if ( !$status->isOk() ) { $this->leaveMessage( $status ); return; } + # Verify upload + $result = $this->upload->verifyUpload(); + if ( $result['status'] != UploadBase::OK ) { + $status = $this->upload->convertVerifyErrorToStatus( $result ); + $this->leaveMessage( $status ); + return; + } + # Check warnings if ( !$this->params['ignoreWarnings'] ) { - $warnings = $this->checkWarnings(); + $warnings = $this->upload->checkWarnings(); if ( $warnings ) { if ( $this->params['leaveMessage'] ) { $this->user->leaveUserMessage( wfMsg( 'upload-warning-subj' ), - wfMsg( 'upload-warning-msg', $this->params['sessionKey'] ) + wfMsg( 'upload-warning-msg', + $this->params['sessionKey'], + $this->params['url'] ) ); } else { $this->storeResultInSession( 'Warning', 'warnings', $warnings ); } + + // FIXME: stash in session return; } } # Perform the upload - $status = $upload->performUpload( + $status = $this->upload->performUpload( $this->params['comment'], $this->params['pageText'], - $this->params['watch'] + $this->params['watch'], + $this->user ); $this->leaveMessage( $status ); } @@ -67,13 +82,17 @@ class UploadFromUrlJob extends Job { protected function leaveMessage( $status ) { if ( $this->params['leaveMessage'] ) { if ( $status->isGood() ) { - $file = $this->getLocalFile(); - - $this->user->leaveUserMessage( wfMsg( 'successfulupload' ), - wfMsg( 'upload-success-msg', $file->getDescriptionUrl() ) ); + $this->user->leaveUserMessage( wfMsg( 'upload-success-subj' ), + wfMsg( 'upload-success-msg', + $this->upload->getTitle()->getText(), + $this->params['url'] + ) ); } else { $this->user->leaveUserMessage( wfMsg( 'upload-failure-subj' ), - wfMsg( 'upload-failure-msg', $status->getWikiText() ) ); + wfMsg( 'upload-failure-msg', + $status->getWikiText(), + $this->params['url'] + ) ); } } else { if ( $status->isOk() ) { @@ -89,6 +108,7 @@ class UploadFromUrlJob extends Job { /** * Store a result in the session data + * THIS IS BROKEN. $_SESSION does not exist when using runJobs.php * * @param $result string The result (Success|Warning|Failure) * @param $dataKey string The key of the extra data diff --git a/includes/upload/UploadFromUrl.php b/includes/upload/UploadFromUrl.php index f22461e510..0e4295b27f 100644 --- a/includes/upload/UploadFromUrl.php +++ b/includes/upload/UploadFromUrl.php @@ -10,6 +10,7 @@ */ class UploadFromUrl extends UploadBase { protected $mAsync, $mUrl; + protected $mIgnoreWarnings = true; /** * Checks if the user is allowed to use the upload-by-URL feature. If the @@ -40,12 +41,12 @@ class UploadFromUrl extends UploadBase { * asynchronous download. */ public function initialize( $name, $url, $async = false ) { - global $wgUser; + global $wgAllowAsyncCopyUploads; $this->mUrl = $url; - $this->mAsync = $async; + $this->mAsync = $wgAllowAsyncCopyUploads ? $async : false; - $tempPath = $async ? null : $this->makeTemporaryFile(); + $tempPath = $this->mAsync ? null : $this->makeTemporaryFile(); # File size and removeTempFile will be filled in later $this->initializePathInfo( $name, $tempPath, 0, false ); } @@ -88,9 +89,20 @@ class UploadFromUrl extends UploadBase { } 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' ); } + /** + * Save the result of a HTTP request to the temporary file + * + * @param $req HttpRequest + * @return Status + */ private function saveTempFile( $req ) { if ( $this->mTempPath === false ) { return Status::newFatal( 'tmp-create-error' ); @@ -103,6 +115,10 @@ class UploadFromUrl extends UploadBase { return Status::newGood(); } + /** + * Download the file, save it to the temporary file and update the file + * size and set $mRemoveTempFile to true. + */ protected function reallyFetchFile() { $req = HttpRequest::factory( $this->mUrl ); $status = $req->execute(); @@ -120,6 +136,44 @@ class UploadFromUrl extends UploadBase { return $status; } + /** + * Wrapper around the parent function in order to defer verifying the + * upload until the file really has been fetched. + */ + public function verifyUpload() { + if ( $this->mAsync ) { + return array( 'status' => self::OK ); + } + return parent::verifyUpload(); + } + + /** + * Wrapper around the parent function in order to defer checking warnings + * until the file really has been fetched. + */ + 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 + */ + public function verifyPermissions( $user ) { + if ( $this->mAsync ) { + return true; + } + return parent::verifyPermissions( $user ); + } + + /** + * Wrapper around the parent function in order to defer uploading to the + * job queue for asynchronous uploads + */ public function performUpload( $comment, $pageText, $watch, $user ) { if ( $this->mAsync ) { $sessionKey = $this->insertJob( $comment, $pageText, $watch, $user ); -- 2.20.1