From 546a55a79d87a650abf11c3c3116aaffc56fee5f Mon Sep 17 00:00:00 2001 From: Bryan Tong Minh Date: Tue, 27 Jul 2010 20:38:36 +0000 Subject: [PATCH] (bug 23380) Uploaded files that are larger than allowed by PHP now show a useful error message. Introduced a WebRequestUpload class which is a wrapper around $_FILES and contains all getUpload* and getFile* methods. This has as advantage that the upload can be passed along without $wgRequest. Also because I like objects. --- RELEASE-NOTES | 2 + includes/AutoLoader.php | 1 + includes/WebRequest.php | 133 +++++++++++++++++++++++------ includes/upload/UploadBase.php | 5 +- includes/upload/UploadFromFile.php | 41 +++++++-- 5 files changed, 152 insertions(+), 30 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index cddc3e0404..ddbad62c27 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -253,6 +253,8 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN page. * (bug 24517) LocalFile::newFromKey() and OldLocalFile::newFromKey() no longer throw fatal errors +* (bug 23380) Uploaded files that are larger than allowed by PHP now show a + useful error message. === API changes in 1.17 === * (bug 22738) Allow filtering by action type on query=logevent. diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index f3110b85f9..8043de4721 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -228,6 +228,7 @@ $wgAutoloadLocalClasses = array( 'WatchedItem' => 'includes/WatchedItem.php', 'WatchlistEditor' => 'includes/WatchlistEditor.php', 'WebRequest' => 'includes/WebRequest.php', + 'WebRequestUpload' => 'includes/WebRequest.php', 'WebResponse' => 'includes/WebResponse.php', 'WikiError' => 'includes/WikiError.php', 'WikiErrorMsg' => 'includes/WikiError.php', diff --git a/includes/WebRequest.php b/includes/WebRequest.php index 1bde4aed6f..7ef1060c0e 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -590,23 +590,20 @@ class WebRequest { * @return string or NULL if no such file. */ public function getFileTempname( $key ) { - if( !isset( $_FILES[$key] ) ) { - return null; - } - return $_FILES[$key]['tmp_name']; + $file = new WebRequestUpload( $key ); + return $file->getTempName(); } /** * Return the size of the upload, or 0. * + * @deprecated * @param $key String: * @return integer */ public function getFileSize( $key ) { - if( !isset( $_FILES[$key] ) ) { - return 0; - } - return $_FILES[$key]['size']; + $file = new WebRequestUpload( $key ); + return $file->getSize(); } /** @@ -616,10 +613,8 @@ class WebRequest { * @return integer */ public function getUploadError( $key ) { - if( !isset( $_FILES[$key] ) || !isset( $_FILES[$key]['error'] ) ) { - return 0/*UPLOAD_ERR_OK*/; - } - return $_FILES[$key]['error']; + $file = new WebRequestUpload( $key ); + return $file->getError(); } /** @@ -634,18 +629,18 @@ class WebRequest { * @return string or NULL if no such file. */ public function getFileName( $key ) { - global $wgContLang; - if( !isset( $_FILES[$key] ) ) { - return null; - } - $name = $_FILES[$key]['name']; - - # Safari sends filenames in HTML-encoded Unicode form D... - # Horrid and evil! Let's try to make some kind of sense of it. - $name = Sanitizer::decodeCharReferences( $name ); - $name = $wgContLang->normalize( $name ); - wfDebug( "WebRequest::getFileName() '" . $_FILES[$key]['name'] . "' normalized to '$name'\n" ); - return $name; + $file = new WebRequestUpload( $key ); + return $file->getName(); + } + + /** + * Return a WebRequestUpload object corresponding to the key + * + * @param @key string + * @return WebRequestUpload + */ + public function getUpload( $key ) { + return new WebRequestUpload( $key ); } /** @@ -769,6 +764,96 @@ class WebRequest { } } +/** + * Object to access the $_FILES array + */ +class WebRequestUpload { + protected $doesExist; + protected $fileInfo; + + /** + * Constructor. Should only be called by WebRequest + * + * @param $key string Key in $_FILES array (name of form field) + */ + public function __construct( $key ) { + $this->doesExist = isset( $_FILES[$key] ); + if ( $this->doesExist ) { + $this->fileInfo = $_FILES[$key]; + } + } + + /** + * Return whether a file with this name was uploaded. + * + * @return bool + */ + public function exists() { + return $this->doesExist; + } + + /** + * Return the original filename of the uploaded file + * + * @return mixed Filename or null if non-existent + */ + public function getName() { + if ( !$this->exists() ) { + return null; + } + + global $wgContLang; + $name = $this->fileInfo['name']; + + # Safari sends filenames in HTML-encoded Unicode form D... + # Horrid and evil! Let's try to make some kind of sense of it. + $name = Sanitizer::decodeCharReferences( $name ); + $name = $wgContLang->normalize( $name ); + wfDebug( __METHOD__ . ": {$this->fileInfo['name']} normalized to '$name'\n" ); + return $name; + } + + /** + * Return the file size of the uploaded file + * + * @return int File size or zero if non-existent + */ + public function getSize() { + if ( !$this->exists() ) { + return 0; + } + + return $this->fileInfo['size']; + } + + /** + * Return the path to the temporary file + * + * @return mixed Path or null if non-existent + */ + public function getTempName() { + if ( !$this->exists() ) { + return null; + } + + return $this->fileInfo['tmp_name']; + } + + /** + * Return the upload error. See link for explanation + * http://www.php.net/manual/en/features.file-upload.errors.php + * + * @return int One of the UPLOAD_ constants, 0 if non-existent + */ + public function getError() { + if ( !$this->exists() ) { + return 0; # UPLOAD_ERR_OK + } + + return $this->fileInfo['error']; + } +} + /** * WebRequest clone which takes values from a provided array. * diff --git a/includes/upload/UploadBase.php b/includes/upload/UploadBase.php index 21fc888ecf..2e184501c0 100644 --- a/includes/upload/UploadBase.php +++ b/includes/upload/UploadBase.php @@ -236,7 +236,10 @@ abstract class UploadBase { */ global $wgMaxUploadSize; if( $this->mFileSize > $wgMaxUploadSize ) { - return array( 'status' => self::FILE_TOO_LARGE ); + return array( + 'status' => self::FILE_TOO_LARGE, + 'max' => $wgMaxUploadSize, + ); } /** diff --git a/includes/upload/UploadFromFile.php b/includes/upload/UploadFromFile.php index 73581a61a6..7ab2e6efcc 100644 --- a/includes/upload/UploadFromFile.php +++ b/includes/upload/UploadFromFile.php @@ -8,16 +8,18 @@ * Implements regular file uploads */ class UploadFromFile extends UploadBase { - + protected $mWebUpload = null; function initializeFromRequest( &$request ) { + $this->mWebUpload = $request->getUpload( 'wpUploadFile' ); + $desiredDestName = $request->getText( 'wpDestFile' ); if( !$desiredDestName ) - $desiredDestName = $request->getFileName( 'wpUploadFile' ); + $desiredDestName = $this->mWebUpload->getName(); return $this->initializePathInfo( $desiredDestName, - $request->getFileTempName( 'wpUploadFile' ), - $request->getFileSize( 'wpUploadFile' ) + $this->mWebUpload->getTempName(), + $this->mWebUpload->getSize() ); } /** @@ -27,6 +29,35 @@ class UploadFromFile extends UploadBase { return $this->initializePathInfo( $name, $tempPath, $fileSize ); } static function isValidRequest( $request ) { - return (bool)$request->getFileTempName( 'wpUploadFile' ); + # Allow all requests, even if no file is present, so that an error + # because a post_max_size or upload_max_filesize overflow + return true; + } + + public function verifyUpload() { + # Check for a post_max_size or upload_max_size overflow, so that a + # proper error can be shown to the user + if ( is_null( $this->mTempPath ) || $this->isEmptyFile() ) { + # Using the Content-length header is not an absolutely fail-safe + # method, but the only available option. Otherwise broken uploads + # will be handled by the parent method and return empty-file + $contentLength = intval( $_SERVER['CONTENT_LENGTH'] ); + $maxPostSize = wfShorthandToInteger( ini_get( 'post_max_size' ) ); + if ( $this->mWebUpload->getError() == UPLOAD_ERR_INI_SIZE + || $contentLength > $maxPostSize ) { + + global $wgMaxUploadSize; + return array( + 'status' => self::FILE_TOO_LARGE, + 'max' => min( + $wgMaxUploadSize, + wfShorthandToInteger( ini_get( 'upload_max_filesize' ) ), + $maxPostSize + ), + ); + } + } + + return parent::verifyUpload(); } } -- 2.20.1