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

diff --git a/RELEASE-NOTES-1.21 b/RELEASE-NOTES-1.21
index b651f5e..1d56221 100644 (file)
--- 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 4a6dad3..480404c 100644 (file)
--- 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 408805e..1f0a5fa 100644 (file)
--- 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 2d40de8..7053ef3 100644 (file)
--- 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 eabcf9b..05d3b5a 100644 (file)
--- 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,