(bug 44923) Fix API upload with only one chunk
authorBrad Jorsch <bjorsch@wikimedia.org>
Tue, 26 Feb 2013 23:19:52 +0000 (15:19 -0800)
committerBrad Jorsch <bjorsch@wikimedia.org>
Wed, 27 Feb 2013 01:36:42 +0000 (17:36 -0800)
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
includes/api/ApiUpload.php

index cbfafa2..30a73b0 100644 (file)
@@ -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.
index 2a58119..6ec3a71 100644 (file)
@@ -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;
        }