From f8d1bf2ad124bf804d87f300a39f43a7450b4d23 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Tue, 26 Feb 2013 13:45:37 -0800
Subject: [PATCH] Add "upload" type to API

If a file upload is not formatted correctly for PHP to recognize it as a
file upload rather than a regular field, the API will wind up trying to
load the file contents as a text field. Since these file contents are
often a large binary file, this will tend to run out of memory trying to
apply Unicode normalization.

To prevent this and to allow for a helpful error message, mark
parameters that are supposed to be file uploads.

Bug: 44909
Change-Id: Ia4586953e2ad2d72d08852689e060e39e7920d50
---
 RELEASE-NOTES-1.21         |  4 ++++
 includes/api/ApiBase.php   | 28 ++++++++++++++++++++++++++++
 includes/api/ApiImport.php |  4 +++-
 includes/api/ApiMain.php   | 12 ++++++++++++
 includes/api/ApiUpload.php |  8 ++++++--
 5 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index b651f5eea4..1d5622107b 100644
--- a/RELEASE-NOTES-1.21
+++ b/RELEASE-NOTES-1.21
@@ -244,6 +244,10 @@ production.
 * (bug 44923) action=upload works correctly if the entire file is uploaded in
   the first chunk.
 * Added 'continue=' parameter to streamline client iteration over complex query results
+* (bug 44909) API parameters may now be marked as type "upload", which is now
+  used for action=upload's 'file' and 'chunk' parameters. This type will raise
+  an error during parameter validation if the parameter is given but not
+  recognized as an uploaded file.
 
 === API internal changes in 1.21 ===
 * For debugging only, a new global $wgDebugAPI removes many API restrictions when true.
diff --git a/includes/api/ApiBase.php b/includes/api/ApiBase.php
index 4a6dad3532..480404ccca 100644
--- a/includes/api/ApiBase.php
+++ b/includes/api/ApiBase.php
@@ -468,6 +468,9 @@ abstract class ApiBase extends ContextSource {
 									$desc .= $paramPrefix . $intRangeStr;
 								}
 								break;
+							case 'upload':
+								$desc .= $paramPrefix . "Must be posted as a file upload using multipart/form-data";
+								break;
 						}
 					}
 
@@ -917,6 +920,29 @@ abstract class ApiBase extends ContextSource {
 			}
 
 			$value = $this->getMain()->getCheck( $encParamName );
+		} elseif ( $type == 'upload' ) {
+			if ( isset( $default ) ) {
+				// Having a default value is not allowed
+				ApiBase::dieDebug( __METHOD__, "File upload param $encParamName's default is set to '$default'. File upload parameters may not have a default." );
+			}
+			if ( $multi ) {
+				ApiBase::dieDebug( __METHOD__, "Multi-values not supported for $encParamName" );
+			}
+			$value = $this->getMain()->getUpload( $encParamName );
+			if ( !$value->exists() ) {
+				// This will get the value without trying to normalize it
+				// (because trying to normalize a large binary file
+				// accidentally uploaded as a field fails spectacularly)
+				$value = $this->getMain()->getRequest()->unsetVal( $encParamName );
+				if ( $value !== null ) {
+					$this->dieUsage(
+						"File upload param $encParamName is not a file upload; " .
+						"be sure to use multipart/form-data for your POST and include " .
+						"a filename in the Content-Disposition header.",
+						"badupload_{$encParamName}"
+					);
+				}
+			}
 		} else {
 			$value = $this->getMain()->getVal( $encParamName, $default );
 
@@ -1013,6 +1039,8 @@ abstract class ApiBase extends ContextSource {
 							$value = $value[0];
 						}
 						break;
+					case 'upload': // nothing to do
+						break;
 					default:
 						ApiBase::dieDebug( __METHOD__, "Param $encParamName's type is unknown - $type" );
 				}
diff --git a/includes/api/ApiImport.php b/includes/api/ApiImport.php
index 408805ee63..1f0a5fab41 100644
--- a/includes/api/ApiImport.php
+++ b/includes/api/ApiImport.php
@@ -105,7 +105,9 @@ class ApiImport extends ApiBase {
 				ApiBase::PARAM_REQUIRED => true
 			),
 			'summary' => null,
-			'xml' => null,
+			'xml' => array(
+				ApiBase::PARAM_TYPE => 'upload',
+			),
 			'interwikisource' => array(
 				ApiBase::PARAM_TYPE => $wgImportSources
 			),
diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php
index 2d40de8894..7053ef37f0 100644
--- a/includes/api/ApiMain.php
+++ b/includes/api/ApiMain.php
@@ -918,6 +918,18 @@ class ApiMain extends ApiBase {
 		return $this->getRequest()->getCheck( $name );
 	}
 
+	/**
+	 * Get a request upload, and register the fact that it was used, for logging.
+	 *
+	 * @since 1.21
+	 * @param $name string Parameter name
+	 * @return WebRequestUpload
+	 */
+	public function getUpload( $name ) {
+		$this->mParamsUsed[$name] = true;
+		return $this->getRequest()->getUpload( $name );
+	}
+
 	/**
 	 * Report unused parameters, so the client gets a hint in case it gave us parameters we don't know,
 	 * for example in case of spelling mistakes or a missing 'g' prefix for generators.
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index eabcf9b246..05d3b5afa5 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -679,7 +679,9 @@ class ApiUpload extends ApiBase {
 				),
 			),
 			'ignorewarnings' => false,
-			'file' => null,
+			'file' => array(
+				ApiBase::PARAM_TYPE => 'upload',
+			),
 			'url' => null,
 			'filekey' => null,
 			'sessionkey' => array(
@@ -690,7 +692,9 @@ class ApiUpload extends ApiBase {
 
 			'filesize' => null,
 			'offset' => null,
-			'chunk' => null,
+			'chunk' => array(
+				ApiBase::PARAM_TYPE => 'upload',
+			),
 
 			'async' => false,
 			'asyncdownload' => false,
-- 
2.20.1