Merge "Introduce new hook UploadVerifyUpload to allow preventing file uploads"

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 4ffff64..d65fb78 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -3306,6 +3306,19 @@ $mime: (string) The uploaded file's MIME type, as detected by MediaWiki.
   representing the problem with the file, where the first element is the message
   key and the remaining elements are used as parameters to the message.
 
+'UploadVerifyUpload': Upload verification, based on both file properties like
+MIME type (same as UploadVerifyFile) and the information entered by the user
+(upload comment, file page contents etc.).
+$upload: (object) An instance of UploadBase, with all info about the upload
+$user: (object) An instance of User, the user uploading this file
+$props: (array) File properties, as returned by FSFile::getPropsFromPath()
+$comment: (string) Upload log comment (also used as edit summary)
+$pageText: (string) File description page text (only used for new uploads)
+&$error: output: If the file upload should be prevented, set this to the reason
+  in the form of array( messagename, param1, param2, ... ) or a MessageSpecifier
+  instance (you might want to use ApiMessage to provide machine-readable details
+  for the API).
+
 'UserIsBot': when determining whether a user is a bot account
 $user: the user
 &$isBot: whether this is user a bot or not (boolean)

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 15c1e39..cb8d938 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -350,9 +350,10 @@ class ApiUpload extends ApiBase {
         * @param array|string|MessageSpecifier $error Error suitable for passing to dieUsageMsg()
         * @param string $parameter Parameter that needs revising
         * @param array $data Optional extra data to pass to the user
+        * @param string $code Error code to use if the error is unknown
         * @throws UsageException
         */
-       private function dieRecoverableError( $error, $parameter, $data = [] ) {
+       private function dieRecoverableError( $error, $parameter, $data = [], $code = 'unknownerror' ) {
                try {
                        $data['filekey'] = $this->performStash();
                        $data['sessionkey'] = $data['filekey'];
@@ -365,6 +366,9 @@ class ApiUpload extends ApiBase {
                if ( isset( $parsed['data'] ) ) {
                        $data = array_merge( $data, $parsed['data'] );
                }
+               if ( $parsed['code'] === 'unknownerror' ) {
+                       $parsed['code'] = $code;
+               }
 
                $this->dieUsage( $parsed['info'], $parsed['code'], 0, $data );
        }
@@ -754,9 +758,14 @@ class ApiUpload extends ApiBase {
                                $this->mParams['text'], $watch, $this->getUser(), $this->mParams['tags'] );
 
                        if ( !$status->isGood() ) {
-                               $error = $status->getErrorsArray();
-                               ApiResult::setIndexedTagName( $error, 'error' );
-                               $this->dieUsage( 'An internal error occurred', 'internal-error', 0, $error );
+                               // Is there really no better way to do this?
+                               $errors = $status->getErrorsByType( 'error' );
+                               $msg = array_merge( [ $errors[0]['message'] ], $errors[0]['params'] );
+                               $data = $status->getErrorsArray();
+                               ApiResult::setIndexedTagName( $data, 'error' );
+                               // For backwards-compatibility, we use the 'internal-error' fallback key and merge $data
+                               // into the root of the response (rather than something sane like [ 'details' => $data ]).
+                               $this->dieRecoverableError( $msg, null, $data, 'internal-error' );
                        }
                        $result['result'] = 'Success';
                }

diff --git a/includes/specials/SpecialUpload.php b/includes/specials/SpecialUpload.php
index 4b731cb..0ef6af1 100644 (file)
--- a/includes/specials/SpecialUpload.php
+++ b/includes/specials/SpecialUpload.php
@@ -535,7 +535,7 @@ class SpecialUpload extends SpecialPage {
                );
 
                if ( !$status->isGood() ) {
-                       $this->showUploadError( $this->getOutput()->parse( $status->getWikiText() ) );
+                       $this->showRecoverableUploadError( $this->getOutput()->parse( $status->getWikiText() ) );
 
                        return;
                }

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 08fbeea..6dcc948 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -717,13 +717,23 @@ abstract class UploadBase {
         */
        public function performUpload( $comment, $pageText, $watch, $user, $tags = [] ) {
                $this->getLocalFile()->load( File::READ_LATEST );
+               $props = $this->mFileProps;
+
+               $error = null;
+               Hooks::run( 'UploadVerifyUpload', [ $this, $user, $props, $comment, $pageText, &$error ] );
+               if ( $error ) {
+                       if ( !is_array( $error ) ) {
+                               $error = [ $error ];
+                       }
+                       return call_user_func_array( 'Status::newFatal', $error );
+               }
 
                $status = $this->getLocalFile()->upload(
                        $this->mTempPath,
                        $comment,
                        $pageText,
                        File::DELETE_SOURCE,
-                       $this->mFileProps,
+                       $props,
                        false,
                        $user,
                        $tags