Uploading to a protected title will allow the user to choose a new name instead of...

* Made validateNameAndOverwrite protected and moved it to validateName since overwriting is now checked in verifyPermissions()
* Fixed mime verification in case getTitle was not yet called
* Checking for overwrites no longer uses $wgUser

Other changes:
* convertVerifyErrorToStatus now works
* Allow passing the session key to stashSession in UploadFromStash as well

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 255afa5..460d370 100644 (file)
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -255,6 +255,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
   throw fatal errors
 * (bug 23380) Uploaded files that are larger than allowed by PHP now show a
   useful error message.
+* Uploading to a protected title will allow the user to choose a new name 
+  instead of showing an error page
 
 === API changes in 1.17 ===
 * (bug 22738) Allow filtering by action type on query=logevent.

diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php
index df2585d..d8ad6f2 100644 (file)
--- a/includes/specials/SpecialUpload.php
+++ b/includes/specials/SpecialUpload.php
@@ -193,6 +193,7 @@ class SpecialUpload extends SpecialPage {
                                wfDebug( "Hook 'UploadForm:initial' broke output of the upload form" );
                                return;
                        }
+                       
 
                        $this->showUploadForm( $this->getUploadForm() );
                }
@@ -415,13 +416,6 @@ class SpecialUpload extends SpecialPage {
        protected function processUpload() {
                global $wgUser, $wgOut;
 
-               // Verify permissions
-               $permErrors = $this->mUpload->verifyPermissions( $wgUser );
-               if( $permErrors !== true ) {
-                       $wgOut->showPermissionsErrorPage( $permErrors );
-                       return;
-               }
-
                // Fetch the file if required
                $status = $this->mUpload->fetchFile();
                if( !$status->isOK() ) {
@@ -445,6 +439,15 @@ class SpecialUpload extends SpecialPage {
                        $this->processVerificationError( $details );
                        return;
                }
+               
+               // Verify permissions for this title
+               $permErrors = $this->mUpload->verifyPermissions( $wgUser );
+               if( $permErrors !== true ) {
+                       $code = array_shift( $permErrors[0] );
+                       $this->showRecoverableUploadError( wfMsgExt( $code,
+                                       'parseinline', $permErrors[0] ) );
+                       return;
+               }
 
                $this->mLocalFile = $this->mUpload->getLocalFile();
 
@@ -549,10 +552,6 @@ class SpecialUpload extends SpecialPage {
                                $this->showRecoverableUploadError( wfMsgExt( 'illegalfilename',
                                        'parseinline', $details['filtered'] ) );
                                break;
-                       case UploadBase::OVERWRITE_EXISTING_FILE:
-                               $this->showRecoverableUploadError( wfMsgExt( $details['overwrite'],
-                                       'parseinline' ) );
-                               break;
                        case UploadBase::FILETYPE_MISSING:
                                $this->showRecoverableUploadError( wfMsgExt( 'filetype-missing',
                                        'parseinline' ) );

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 63082a4..1e23fbe 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -25,7 +25,7 @@ abstract class UploadBase {
        const EMPTY_FILE = 3;
        const MIN_LENGTH_PARTNAME = 4;
        const ILLEGAL_FILENAME = 5;
-       const OVERWRITE_EXISTING_FILE = 7;
+       const OVERWRITE_EXISTING_FILE = 7; # Not used anymore; handled by verifyPermissions()
        const FILETYPE_MISSING = 8;
        const FILETYPE_BADTYPE = 9;
        const VERIFICATION_ERROR = 10;
@@ -223,7 +223,7 @@ abstract class UploadBase {
         * Verify whether the upload is sane.
         * @return mixed self::OK or else an array with error information
         */
-       public function verifyUpload( ) {
+       public function verifyUpload() {
                /**
                 * If there was no filename or a zero size given, give up quick.
                 */
@@ -258,7 +258,7 @@ abstract class UploadBase {
                /**
                 * Make sure this file can be created
                 */
-               $result = $this->validateNameAndOverwrite();
+               $result = $this->validateName();
                if( $result !== true ) {
                        return $result;
                }
@@ -279,7 +279,7 @@ abstract class UploadBase {
         * @return mixed true if valid, otherwise and array with 'status'
         * and other keys
         **/
-       public function validateNameAndOverwrite() {
+       protected function validateName() {
                $nt = $this->getTitle();
                if( is_null( $nt ) ) {
                        $result = array( 'status' => $this->mTitleError );
@@ -293,16 +293,6 @@ abstract class UploadBase {
                }
                $this->mDestName = $this->getLocalFile()->getName();
 
-               /**
-                * In some cases we may forbid overwriting of existing files.
-                */
-               $overwrite = $this->checkOverwrite();
-               if( $overwrite !== true ) {
-                       return array(
-                               'status' => self::OVERWRITE_EXISTING_FILE,
-                               'overwrite' => $overwrite
-                       );
-               }
                return true;
        }
 
@@ -347,6 +337,10 @@ abstract class UploadBase {
         * @return mixed true of the file is verified, array otherwise.
         */
        protected function verifyFile() {
+               # get the title, even though we are doing nothing with it, because
+               # we need to populate mFinalExtension 
+               $nt = $this->getTitle();
+               
                $this->mFileProps = File::getPropsFromPath( $this->mTempPath, $this->mFinalExtension );
                $this->checkMacBinary();
 
@@ -382,7 +376,11 @@ abstract class UploadBase {
        }
 
        /**
-        * Check whether the user can edit, upload and create the image.
+        * Check whether the user can edit, upload and create the image. This
+        * checks only against the current title; if it returns errors, it may
+        * very well be that another title will not give errors. Therefore
+        * isAllowed() should be called as well for generic is-user-blocked or
+        * can-user-upload checking.
         *
         * @param $user the User object to verify the permissions against
         * @return mixed An array as returned by getUserPermissionsErrors or true
@@ -409,6 +407,12 @@ abstract class UploadBase {
                        $permErrors = array_merge( $permErrors, wfArrayDiff2( $permErrorsCreate, $permErrors ) );
                        return $permErrors;
                }
+               
+               $overwriteError = $this->checkOverwrite( $user );
+               if ( $overwriteError !== true ) {
+                       return array( array( $overwriteError ) );
+               }
+               
                return true;
        }
 
@@ -1007,12 +1011,11 @@ abstract class UploadBase {
         *
         * @return mixed true on success, error string on failure
         */
-       private function checkOverwrite() {
-               global $wgUser;
+       private function checkOverwrite( $user ) {
                // First check whether the local file can be overwritten
                $file = $this->getLocalFile();
                if( $file->exists() ) {
-                       if( !self::userCanReUpload( $wgUser, $file ) ) {
+                       if( !self::userCanReUpload( $user, $file ) ) {
                                return 'fileexists-forbidden';
                        } else {
                                return true;
@@ -1023,7 +1026,7 @@ abstract class UploadBase {
                 * wfFindFile finds a file, it exists in a shared repository.
                 */
                $file = wfFindFile( $this->getTitle() );
-               if ( $file && !$wgUser->isAllowed( 'reupload-shared' ) ) {
+               if ( $file && !$user->isAllowed( 'reupload-shared' ) ) {
                        return 'fileexists-shared-forbidden';
                }
 
@@ -1187,8 +1190,8 @@ abstract class UploadBase {
        }
 
        public function convertVerifyErrorToStatus( $error ) {
-               $args = func_get_args();
-               array_shift($args);
-               return Status::newFatal( $this->getVerificationErrorCode( $error ), $args );
+               $code = $error['status'];
+               unset( $code['status'] );
+               return Status::newFatal( $this->getVerificationErrorCode( $code ), $error );
        }
 }

diff --git a/includes/upload/UploadFromStash.php b/includes/upload/UploadFromStash.php
index 6347834..d4ddf88 100644 (file)
--- a/includes/upload/UploadFromStash.php
+++ b/includes/upload/UploadFromStash.php
@@ -65,7 +65,7 @@ class UploadFromStash extends UploadBase {
        /**
         * There is no need to stash the image twice
         */
-       public function stashSession() {
+       public function stashSession( $key = null ) {
                if ( !empty( $this->mSessionKey ) )
                        return $this->mSessionKey;
                return parent::stashSession();