From 09f2cba0068378854a69320f10e6d71c4546f516 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Wed, 17 Aug 2016 01:34:52 +0200 Subject: [PATCH] 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 --- includes/api/ApiUpload.php | 3 ++- includes/upload/UploadBase.php | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php index 89d9af8ee8..6a70e5a010 100644 --- 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 9f534d2eb6..1ec205cc2a 100644 --- 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 ); -- 2.20.1