Page MenuHomePhabricator
Files F52684

0001-API-Improve-validation-in-chunked-uploading.patch
No One
Actions

Authored By
Anomie
Mar 2 2015, 9:59 PM
Size
10 KB
Referenced Files
None
Subscribers
None

0001-API-Improve-validation-in-chunked-uploading.patch
View Options

From 7e3615413632fbe97bb1ecef974a42ab239fe25e Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Mon, 2 Mar 2015 16:27:17 -0500
Subject: [PATCH] API: Improve validation in chunked uploading
This fixes a few shortcomings in the chunked uploader:
* Raises an error if offset + chunksize > filesize.
* Enforces a minimum chunk size for non-final chunks.
* Refuses additional chunks after seeing a final chunk.
* Status of a chunked upload in progress is now available with
'checkstatus'.
Bug: T91203
Bug: T91205
Change-Id: I36a571864ef4d5481df7e9867bd4750bb24c89eb
---
RELEASE-NOTES-1.25 | 6 +++
includes/DefaultSettings.php | 8 ++++
includes/GlobalFunctions.php | 5 ++-
includes/Setup.php | 9 +++++
includes/api/ApiQuerySiteinfo.php | 1 +
includes/api/ApiUpload.php | 83 ++++++++++++++++++++++++++++++++-------
6 files changed, 96 insertions(+), 16 deletions(-)
diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 45e669a..901708f 100644
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -45,6 +45,8 @@ production.
* The 'daemonized' flag must be set to true in $wgJobTypeConf for any redis
job queues. This means that mediawiki/services/jobrunner service has to
be installed and running for any such queues to work.
+* Added $wgMinUploadChunkSize to restrict the size of chunks in chunked file
+ uploading.
=== New features in 1.25 ===
* (T64861) Updated plural rules to CLDR 26. Includes incompatible changes
@@ -228,6 +230,10 @@ production.
Title::userCan() via the API.
* Default type param for query list=watchlist and list=recentchanges has
been changed from all types (e.g. including 'external') to 'edit|new|log'.
+* action=upload will enforce a minimum chunk size (see $wgMinUploadChunkSize).
+* action=upload will refuse chunks that would make the file larger than the
+ stated filesize, or any additional chunks after the stated size is reached.
+* action=upload&checkstatus will now return status for chunked uploads in progress.
=== Action API internal changes in 1.25 ===
* ApiHelp has been rewritten to support i18n and paginated HTML output.
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 6786f81..71b47d1 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -674,6 +674,14 @@ $wgCopyUploadAsyncTimeout = false;
$wgMaxUploadSize = 1024 * 1024 * 100; # 100MB
/**
+ * Minimum upload chunk size, in bytes. When using chunked upload, non-final
+ * chunks smaller than this will be rejected. May be reduced based on the
+ * 'upload_max_filesize' or 'post_max_size' PHP settings.
+ * @since 1.25
+ */
+$wgMinUploadChunkSize = 1024 * 1024; # 1MB
+
+/**
* Point the upload navigation link to an external URL
* Useful if you want to use a shared repository by default
* without disabling local uploads (use $wgEnableUploads = false for that).
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 5232413..eeb28d6 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -3913,12 +3913,13 @@ function wfMemoryLimit() {
* Converts shorthand byte notation to integer form
*
* @param string $string
+ * @param int $default Returned if $string is empty
* @return int
*/
-function wfShorthandToInteger( $string = '' ) {
+function wfShorthandToInteger( $string = '', $default = -1 ) {
$string = trim( $string );
if ( $string === '' ) {
- return -1;
+ return $default;
}
$last = $string[strlen( $string ) - 1];
$val = intval( $string );
diff --git a/includes/Setup.php b/includes/Setup.php
index 6939f95..b661224 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -368,6 +368,15 @@ if ( $wgResourceLoaderMaxQueryLength === false ) {
$wgResourceLoaderMaxQueryLength = $maxValueLength > 0 ? $maxValueLength : -1;
}
+// Ensure the minimum chunk size is less than PHP upload limits or the maximum
+// upload size.
+$wgMinUploadChunkSize = min(
+ $wgMinUploadChunkSize,
+ $wgMaxUploadSize,
+ wfShorthandToInteger( ini_get( 'upload_max_filesize' ), 1e100 ),
+ wfShorthandToInteger( ini_get( 'post_max_size' ), 1e100) - 1024 # Leave room for other parameters
+);
+
/**
* Definitions of the NS_ constants are in Defines.php
* @private
diff --git a/includes/api/ApiQuerySiteinfo.php b/includes/api/ApiQuerySiteinfo.php
index 5ac1036..e83d7b3 100644
--- a/includes/api/ApiQuerySiteinfo.php
+++ b/includes/api/ApiQuerySiteinfo.php
@@ -261,6 +261,7 @@ class ApiQuerySiteinfo extends ApiQueryBase {
}
$data['maxuploadsize'] = UploadBase::getMaxUploadSize();
+ $data['minuploadchunksize'] = (int)$this->getConfig()->get( 'MinUploadChunkSize' );
$data['thumblimits'] = $config->get( 'ThumbLimits' );
$this->getResult()->setIndexedTagName( $data['thumblimits'], 'limit' );
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 78a4971..6f9355a 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -81,7 +81,7 @@ class ApiUpload extends ApiBase {
// Check if the uploaded file is sane
if ( $this->mParams['chunk'] ) {
- $maxSize = $this->mUpload->getMaxUploadSize();
+ $maxSize = UploadBase::getMaxUploadSize();
if ( $this->mParams['filesize'] > $maxSize ) {
$this->dieUsage( 'The file you submitted was too large', 'file-too-large' );
}
@@ -197,13 +197,30 @@ class ApiUpload extends ApiBase {
private function getChunkResult( $warnings ) {
$result = array();
- $result['result'] = 'Continue';
if ( $warnings && count( $warnings ) > 0 ) {
$result['warnings'] = $warnings;
}
+
$request = $this->getMain()->getRequest();
$chunkPath = $request->getFileTempname( 'chunk' );
$chunkSize = $request->getUpload( 'chunk' )->getSize();
+ $totalSoFar = $this->mParams['offset'] + $chunkSize;
+ $minChunkSize = $this->getConfig()->get( 'MinUploadChunkSize' );
+
+ // Sanity check sizing
+ if ( $totalSoFar > $this->mParams['filesize'] ) {
+ $this->dieUsage(
+ 'Offset plus current chunk is greater than claimed file size', 'invalid-chunk'
+ );
+ }
+
+ // Enforce minimum chunk size
+ if ( $totalSoFar != $this->mParams['filesize'] && $chunkSize < $minChunkSize ) {
+ $this->dieUsage(
+ "Minimum chunk size is $minChunkSize bytes for non-final chunks", 'chunk-too-small'
+ );
+ }
+
if ( $this->mParams['offset'] == 0 ) {
try {
$filekey = $this->performStash();
@@ -215,6 +232,18 @@ class ApiUpload extends ApiBase {
}
} else {
$filekey = $this->mParams['filekey'];
+
+ // Don't allow further uploads to an already-completed session
+ $progress = UploadBase::getSessionStatus( $this->getUser(), $filekey );
+ if ( !$progress ) {
+ // Probably can't get here, but check anyway just in case
+ $this->dieUsage( 'No chunked upload session with this key', 'stashfailed' );
+ } elseif ( $progress['result'] !== 'Continue' || $progress['stage'] !== 'uploading' ) {
+ $this->dieUsage(
+ 'Chunked upload is already completed, check status for details', 'stashfailed'
+ );
+ }
+
$status = $this->mUpload->addChunk(
$chunkPath, $chunkSize, $this->mParams['offset'] );
if ( !$status->isGood() ) {
@@ -223,18 +252,12 @@ class ApiUpload extends ApiBase {
);
$this->dieUsage( $status->getWikiText(), 'stashfailed', 0, $extradata );
-
- return array();
}
}
// Check we added the last chunk:
- if ( $this->mParams['offset'] + $chunkSize == $this->mParams['filesize'] ) {
+ if ( $totalSoFar == $this->mParams['filesize'] ) {
if ( $this->mParams['async'] ) {
- $progress = UploadBase::getSessionStatus( $this->getUser(), $filekey );
- if ( $progress && $progress['result'] === 'Poll' ) {
- $this->dieUsage( "Chunk assembly already in progress.", 'stashfailed' );
- }
UploadBase::setSessionStatus(
$this->getUser(),
$filekey,
@@ -254,21 +277,38 @@ class ApiUpload extends ApiBase {
} else {
$status = $this->mUpload->concatenateChunks();
if ( !$status->isGood() ) {
+ UploadBase::setSessionStatus(
+ $this->getUser(),
+ $filekey,
+ array( 'result' => 'Failure', 'stage' => 'assembling', 'status' => $status )
+ );
$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.
+ UploadBase::setSessionStatus( $this->getUser(), $filekey, false );
$this->mUpload->stash->removeFile( $filekey );
$filekey = $this->mUpload->getLocalFile()->getFileKey();
$result['result'] = 'Success';
}
+ } else {
+ UploadBase::setSessionStatus(
+ $this->getUser(),
+ $filekey,
+ array(
+ 'result' => 'Continue',
+ 'stage' => 'uploading',
+ 'offset' => $totalSoFar,
+ 'status' => Status::newGood(),
+ )
+ );
+ $result['result'] = 'Continue';
+ $result['offset'] = $totalSoFar;
}
+
$result['filekey'] = $filekey;
- $result['offset'] = $this->mParams['offset'] + $chunkSize;
return $result;
}
@@ -378,6 +418,10 @@ class ApiUpload extends ApiBase {
// Chunk upload
$this->mUpload = new UploadFromChunks();
if ( isset( $this->mParams['filekey'] ) ) {
+ if ( $this->mParams['offset'] === 0 ) {
+ $this->dieUsage( 'Cannot supply a filekey when offset is 0', 'badparams' );
+ }
+
// handle new chunk
$this->mUpload->continueChunks(
$this->mParams['filename'],
@@ -385,6 +429,10 @@ class ApiUpload extends ApiBase {
$request->getUpload( 'chunk' )
);
} else {
+ if ( $this->mParams['offset'] !== 0 ) {
+ $this->dieUsage( 'Must supply a filekey when offset is non-zero', 'badparams' );
+ }
+
// handle first chunk
$this->mUpload->initialize(
$this->mParams['filename'],
@@ -760,8 +808,15 @@ class ApiUpload extends ApiBase {
),
'stash' => false,
- 'filesize' => null,
- 'offset' => null,
+ 'filesize' => array(
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_MIN => 0,
+ ApiBase::PARAM_MAX => UploadBase::getMaxUploadSize(),
+ ),
+ 'offset' => array(
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_MIN => 0,
+ ),
'chunk' => array(
ApiBase::PARAM_TYPE => 'upload',
),
--
2.1.4

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
50470
Default Alt Text
0001-API-Improve-validation-in-chunked-uploading.patch (10 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits