From cd1398d4d644cf5e103427f49f24c63d065a27c2 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 26 Feb 2013 15:19:52 -0800
Subject: [PATCH] (bug 44923) Fix API upload with only one chunk

The API supports chunked uploads to upload a file in multiple pieces.
But it doesn't check if the the entire file is uploaded in the first
chunk, so it winds up waiting for a subsequent chunk that can never
come.

It's actually fairly easy to fix, we just need to move the check for
"did we get the entire file?" outside of the "if we're getting a chunk
after the first" check.

Change-Id: I915c1678dfa3107f1739fed8613ab9452139b946
---
 RELEASE-NOTES-1.21         |  2 +
 includes/api/ApiUpload.php | 86 +++++++++++++++++++-------------------
 2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index cbfafa202b..30a73b0e57 100644
--- a/RELEASE-NOTES-1.21
+++ b/RELEASE-NOTES-1.21
@@ -241,6 +241,8 @@ production.
   currently in use on the wiki.
 * (bug 44921) ApiMain::execute() will now return after the CORS check for an
   HTTP OPTIONS request.
+* (bug 44923) action=upload works correctly if the entire file is uploaded in
+  the first chunk.
 
 === API internal changes in 1.21 ===
 * For debugging only, a new global $wgDebugAPI removes many API restrictions when true.
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 2a58119edd..6ec3a71d81 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -196,63 +196,61 @@ class ApiUpload extends ApiBase {
 		$chunkPath = $request->getFileTempname( 'chunk' );
 		$chunkSize = $request->getUpload( 'chunk' )->getSize();
 		if ( $this->mParams['offset'] == 0 ) {
-			$result['filekey'] = $this->performStash();
+			$filekey = $this->performStash();
 		} else {
+			$filekey = $this->mParams['filekey'];
 			$status = $this->mUpload->addChunk(
 				$chunkPath, $chunkSize, $this->mParams['offset'] );
 			if ( !$status->isGood() ) {
 				$this->dieUsage( $status->getWikiText(), 'stashfailed' );
 				return array();
 			}
+		}
 
-			// Check we added the last chunk:
-			if ( $this->mParams['offset'] + $chunkSize == $this->mParams['filesize'] ) {
-				if ( $this->mParams['async'] ) {
-					$progress = UploadBase::getSessionStatus( $this->mParams['filekey'] );
-					if ( $progress && $progress['result'] === 'Poll' ) {
-						$this->dieUsage( "Chunk assembly already in progress.", 'stashfailed' );
-					}
-					UploadBase::setSessionStatus(
-						$this->mParams['filekey'],
-						array( 'result' => 'Poll',
-							'stage' => 'queued', 'status' => Status::newGood() )
-					);
-					$ok = JobQueueGroup::singleton()->push( new AssembleUploadChunksJob(
-						Title::makeTitle( NS_FILE, $this->mParams['filekey'] ),
-						array(
-							'filename'  => $this->mParams['filename'],
-							'filekey'   => $this->mParams['filekey'],
-							'session'   => $this->getRequest()->exportUserSession(),
-							'userid'    => $this->getUser()->getId()
-						)
-					) );
-					if ( $ok ) {
-						$result['result'] = 'Poll';
-					} else {
-						UploadBase::setSessionStatus( $this->mParams['filekey'], false );
-						$this->dieUsage(
-							"Failed to start AssembleUploadChunks.php", 'stashfailed' );
-					}
+		// Check we added the last chunk:
+		if ( $this->mParams['offset'] + $chunkSize == $this->mParams['filesize'] ) {
+			if ( $this->mParams['async'] ) {
+				$progress = UploadBase::getSessionStatus( $this->mParams['filekey'] );
+				if ( $progress && $progress['result'] === 'Poll' ) {
+					$this->dieUsage( "Chunk assembly already in progress.", 'stashfailed' );
+				}
+				UploadBase::setSessionStatus(
+					$this->mParams['filekey'],
+					array( 'result' => 'Poll',
+						'stage' => 'queued', 'status' => Status::newGood() )
+				);
+				$ok = JobQueueGroup::singleton()->push( new AssembleUploadChunksJob(
+					Title::makeTitle( NS_FILE, $this->mParams['filekey'] ),
+					array(
+						'filename'  => $this->mParams['filename'],
+						'filekey'   => $this->mParams['filekey'],
+						'session'   => $this->getRequest()->exportUserSession(),
+						'userid'    => $this->getUser()->getId()
+					)
+				) );
+				if ( $ok ) {
+					$result['result'] = 'Poll';
 				} else {
-					$status = $this->mUpload->concatenateChunks();
-					if ( !$status->isGood() ) {
-						$this->dieUsage( $status->getWikiText(), 'stashfailed' );
-						return array();
-					}
-
-					// We have a new filekey for the fully concatenated file.
-					$result['filekey'] = $this->mUpload->getLocalFile()->getFileKey();
-
-					// Remove chunk from stash. (Checks against user ownership of chunks.)
-					$this->mUpload->stash->removeFile( $this->mParams['filekey'] );
-
-					$result['result'] = 'Success';
+					UploadBase::setSessionStatus( $this->mParams['filekey'], false );
+					$this->dieUsage(
+						"Failed to start AssembleUploadChunks.php", 'stashfailed' );
 				}
 			} else {
-				// Continue passing through the filekey for adding further chunks.
-				$result['filekey'] = $this->mParams['filekey'];
+				$status = $this->mUpload->concatenateChunks();
+				if ( !$status->isGood() ) {
+					$this->dieUsage( $status->getWikiText(), 'stashfailed' );
+					return array();
+				}
+
+				// The fully concatenated file has a new filekey. So remove
+				// the old filekey and fetch the new one.
+				$this->mUpload->stash->removeFile( $filekey );
+				$filekey = $this->mUpload->getLocalFile()->getFileKey();
+
+				$result['result'] = 'Success';
 			}
 		}
+		$result['filekey'] = $filekey;
 		$result['offset'] = $this->mParams['offset'] + $chunkSize;
 		return $result;
 	}
-- 
2.20.1