Introduce new hook UploadVerifyUpload to allow preventing file uploads

The new hook runs at the beginning of UploadBase::performUpload().
Unlike the existing UploadVerifyFile hook, the new one provides the
information about the file description page being created, and the
user who is performing the upload. It also does not run for uploads
to stash.

The action=upload API and Special:Upload have been changed to treat
errors from UploadBase::performUpload() as recoverable, which means
that they will attempt to stash the file (previously they would require
the user to upload it again). This is mostly intended for errors from
the new hook, but I think it makes sense for the existing ones, which
are mostly transient filesystem erorrs.

Bug: T89302
Change-Id: Ie68801b307de8456e1753ba54a29c34c8063bc36

diff --git a/docs/hooks.txt b/docs/hooks.txt
index 1d11893..0e7c1c0 100644 (file)
--- a/docs/hooks.txt
+++ b/docs/hooks.txt
@@ -3285,6 +3285,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 0a79aa4..3af0dff 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -350,9 +350,10 @@ class ApiUpload extends ApiBase {
         * @param array $error Error array 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 ba5171f..5d0bc13 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