Do not call the 'UploadStashFile' hook for partially uploaded files

Consumers of this hook don't expect partial files, and even if they did,
they have no way to tell if they've been given a partial file.

Also document a pre-existing restriction that verifyUpload() must be
called first (for non-partial files). ApiUpload previously called this
function for partial files too, causing T143161.

Follow-up to 2ee27d9a4de163dc3a2b752823a04f5338005627.

Bug: T143161
Change-Id: I107cb957e046545ea72834d72233cc1ed2867e42

diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 89d9af8..6a70e5a 100644 (file)
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -315,8 +315,9 @@ class ApiUpload extends ApiBase {
         * @return string|null File key
         */
        private function performStash( $failureMode, &$data = null ) {
+               $isPartial = (bool)$this->mParams['chunk'];
                try {
-                       $status = $this->mUpload->tryStashFile( $this->getUser() );
+                       $status = $this->mUpload->tryStashFile( $this->getUser(), $isPartial );
 
                        if ( $status->isGood() && !$status->getValue() ) {
                                // Not actually a 'good' status...

diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php
index 9f534d2..1ec205c 100644 (file)
--- a/includes/upload/UploadBase.php
+++ b/includes/upload/UploadBase.php
@@ -924,23 +924,27 @@ abstract class UploadBase {
        }
 
        /**
-        * Like stashFile(), but respects extensions' wishes to prevent the stashing.
+        * Like stashFile(), but respects extensions' wishes to prevent the stashing. verifyUpload() must
+        * be called before calling this method (unless $isPartial is true).
         *
         * Upload stash exceptions are also caught and converted to an error status.
         *
         * @since 1.28
         * @param User $user
+        * @param bool $isPartial Pass `true` if this is a part of a chunked upload (not a complete file).
         * @return Status If successful, value is an UploadStashFile instance
         */
-       public function tryStashFile( User $user ) {
-               $props = $this->mFileProps;
-               $error = null;
-               Hooks::run( 'UploadStashFile', [ $this, $user, $props, &$error ] );
-               if ( $error ) {
-                       if ( !is_array( $error ) ) {
-                               $error = [ $error ];
+       public function tryStashFile( User $user, $isPartial = false ) {
+               if ( !$isPartial ) {
+                       $props = $this->mFileProps;
+                       $error = null;
+                       Hooks::run( 'UploadStashFile', [ $this, $user, $props, &$error ] );
+                       if ( $error ) {
+                               if ( !is_array( $error ) ) {
+                                       $error = [ $error ];
+                               }
+                               return call_user_func_array( 'Status::newFatal', $error );
                        }
-                       return call_user_func_array( 'Status::newFatal', $error );
                }
                try {
                        $file = $this->doStashFile( $user );