From 90cb6aaa50e6f9e2efe588230225791c7815fee2 Mon Sep 17 00:00:00 2001
From: Matthias Mullie <git@mullie.eu>
Date: Thu, 15 Dec 2016 14:34:13 +0100
Subject: [PATCH] Do not lose message parameters in
 UploadFromChunks::verifyChunk()

This change is similar to If9ce05045ada1e3f55e031639e4c4ebc2a216de8

Having verifyChunk inside doStashFile was annoying. We'd have to
catch the exception in UploadBase::tryStashFile in order to convert
it to a proper Status object instead of the generic one that is
currently built there.
I felt like UploadBase::tryStashFile shouldn't have to be aware of
this exception, so I moved that catch into a new
UploadFromChunks::tryStashFile.
It makes no sense to perform that check twice when running
tryStashFile, so I got rid of it in doStashFile. But that also
meant we had to add it to a few other (now deprecated) places calling
doStashFile... But they should be cleaned up at some point anyway.

This will make sure we get error output like this:
"code":"filetype-bad-ie-mime",
"key":"filetype-bad-ie-mime",
"params":["text/html"]

instead of:
"code":"stashfailed",
"key":"Cannot upload this file because Internet Explorer would detect it as \"text/html\", which is a disallowed and potentially dangerous file type.",
"params":[]

Bug: T32095
Change-Id: I2fa767656cb3a5b366210042b8b504dc10ddaf68
---
 includes/upload/UploadFromChunks.php | 47 +++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/includes/upload/UploadFromChunks.php b/includes/upload/UploadFromChunks.php
index 449fc05ec4..3dabc0d4b6 100644
--- a/includes/upload/UploadFromChunks.php
+++ b/includes/upload/UploadFromChunks.php
@@ -63,6 +63,52 @@ class UploadFromChunks extends UploadFromFile {
 		}
 	}
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function tryStashFile( User $user, $isPartial = false ) {
+		try {
+			$this->verifyChunk();
+		} catch ( UploadChunkVerificationException $e ) {
+			return Status::newFatal( $e->msg );
+		}
+
+		return parent::tryStashFile( $user, $isPartial );
+	}
+
+	/**
+	 * {@inheritdoc}
+	 * @throws UploadChunkVerificationException
+	 * @deprecated since 1.28 Use tryStashFile() instead
+	 */
+	public function stashFile( User $user = null ) {
+		wfDeprecated( __METHOD__, '1.28' );
+		$this->verifyChunk();
+		return parent::stashFile( $user );
+	}
+
+	/**
+	 * {@inheritdoc}
+	 * @throws UploadChunkVerificationException
+	 * @deprecated since 1.28
+	 */
+	public function stashFileGetKey() {
+		wfDeprecated( __METHOD__, '1.28' );
+		$this->verifyChunk();
+		return parent::stashFileGetKey();
+	}
+
+	/**
+	 * {@inheritdoc}
+	 * @throws UploadChunkVerificationException
+	 * @deprecated since 1.28
+	 */
+	public function stashSession() {
+		wfDeprecated( __METHOD__, '1.28' );
+		$this->verifyChunk();
+		return parent::stashSession();
+	}
+
 	/**
 	 * Calls the parent doStashFile and updates the uploadsession table to handle "chunks"
 	 *
@@ -74,7 +120,6 @@ class UploadFromChunks extends UploadFromFile {
 		$this->mChunkIndex = 0;
 		$this->mOffset = 0;
 
-		$this->verifyChunk();
 		// Create a local stash target
 		$this->mStashFile = parent::doStashFile( $user );
 		// Update the initial file offset (based on file size)
-- 
2.20.1