From aac84f732dd192a5f13fe4b31ad0a384d6aa890b Mon Sep 17 00:00:00 2001 From: Robin McCorkell Date: Mon, 20 Jul 2015 15:03:09 +0100 Subject: [PATCH] Unit test OC_Files::setUploadLimit() There was also a bug with checking the upper limit on the passed upload size. PHP does funny things with integer vs float comparisons, so our check didn't work. Now the check is much simpler, and ensures the value is sane. --- lib/private/files.php | 25 +++--- tests/data/setUploadLimit/htaccess | 60 +++++++++++++ tests/data/setUploadLimit/user.ini | 7 ++ tests/lib/files.php | 135 +++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 14 deletions(-) create mode 100644 tests/data/setUploadLimit/htaccess create mode 100644 tests/data/setUploadLimit/user.ini create mode 100644 tests/lib/files.php diff --git a/lib/private/files.php b/lib/private/files.php index b9fd109331..6268bf8a12 100644 --- a/lib/private/files.php +++ b/lib/private/files.php @@ -269,37 +269,34 @@ class OC_Files { * set the maximum upload size limit for apache hosts using .htaccess * * @param int $size file size in bytes + * @param array $files override '.htaccess' and '.user.ini' locations * @return bool false on failure, size on success */ - static function setUploadLimit($size) { + public static function setUploadLimit($size, $files = []) { //don't allow user to break his config - if ($size > PHP_INT_MAX) { - //max size is always 1 byte lower than computerFileSize returns - if ($size > PHP_INT_MAX + 1) - return false; - $size -= 1; - } + $size = intval($size); if ($size < self::UPLOAD_MIN_LIMIT_BYTES) { return false; } $size = OC_Helper::phpFileSize($size); - //don't allow user to break his config -- broken or malicious size input - if (intval($size) === 0) { - return false; - } - $phpValueKeys = array( 'upload_max_filesize', 'post_max_size' ); + // default locations if not overridden by $files + $files = array_merge([ + '.htaccess' => OC::$SERVERROOT . '/.htaccess', + '.user.ini' => OC::$SERVERROOT . '/.user.ini' + ], $files); + $updateFiles = [ - OC::$SERVERROOT . '/.htaccess' => [ + $files['.htaccess'] => [ 'pattern' => '/php_value %1$s (\S)*/', 'setting' => 'php_value %1$s %2$s' ], - OC::$SERVERROOT . '/.user.ini' => [ + $files['.user.ini'] => [ 'pattern' => '/%1$s=(\S)*/', 'setting' => '%1$s=%2$s' ] diff --git a/tests/data/setUploadLimit/htaccess b/tests/data/setUploadLimit/htaccess new file mode 100644 index 0000000000..65957a2983 --- /dev/null +++ b/tests/data/setUploadLimit/htaccess @@ -0,0 +1,60 @@ +# Version: 8.2.0 + + + + SetEnvIfNoCase ^Authorization$ "(.+)" XAUTHORIZATION=$1 + RequestHeader set XAuthorization %{XAUTHORIZATION}e env=XAUTHORIZATION + + + + + # Add security and privacy related headers + Header set X-Content-Type-Options "nosniff" + Header set X-XSS-Protection "1; mode=block" + Header set X-Robots-Tag "none" + Header set X-Frame-Options "SAMEORIGIN" + SetEnv modHeadersAvailable true + + + # Add cache control for CSS and JS files + + Header set Cache-Control "max-age=7200, public" + + + +php_value upload_max_filesize 513M +php_value post_max_size 513M +php_value memory_limit 512M +php_value mbstring.func_overload 0 +php_value always_populate_raw_post_data -1 +php_value default_charset 'UTF-8' +php_value output_buffering off + + SetEnv htaccessWorking true + + + +RewriteEngine on +RewriteRule .* - [env=HTTP_AUTHORIZATION:%{HTTP:Authorization}] +RewriteRule ^\.well-known/host-meta /public.php?service=host-meta [QSA,L] +RewriteRule ^\.well-known/host-meta\.json /public.php?service=host-meta-json [QSA,L] +RewriteRule ^\.well-known/carddav /remote.php/carddav/ [R=301,L] +RewriteRule ^\.well-known/caldav /remote.php/caldav/ [R=301,L] +RewriteRule ^apps/calendar/caldav\.php remote.php/caldav/ [QSA,L] +RewriteRule ^apps/contacts/carddav\.php remote.php/carddav/ [QSA,L] +RewriteRule ^remote/(.*) remote.php [QSA,L] +RewriteRule ^(build|tests|config|lib|3rdparty|templates)/.* - [R=404,L] +RewriteRule ^(\.|autotest|occ|issue|indie|db_|console).* - [R=404,L] + + +AddType image/svg+xml svg svgz +AddEncoding gzip svgz + + +DirectoryIndex index.php index.html + +AddDefaultCharset utf-8 +Options -Indexes + + ModPagespeed Off + diff --git a/tests/data/setUploadLimit/user.ini b/tests/data/setUploadLimit/user.ini new file mode 100644 index 0000000000..c5996e8d47 --- /dev/null +++ b/tests/data/setUploadLimit/user.ini @@ -0,0 +1,7 @@ +upload_max_filesize=513M +post_max_size=513M +memory_limit=512M +mbstring.func_overload=0 +always_populate_raw_post_data=-1 +default_charset='UTF-8' +output_buffering=off diff --git a/tests/lib/files.php b/tests/lib/files.php new file mode 100644 index 0000000000..6808b3e9f6 --- /dev/null +++ b/tests/lib/files.php @@ -0,0 +1,135 @@ + + * + * @copyright Copyright (c) 2015, ownCloud, Inc. + * @license AGPL-3.0 + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU AFFERO GENERAL PUBLIC LICENSE for more details. + * + * You should have received a copy of the GNU Affero General Public + * License along with this library. If not, see . + * + */ + +namespace Test; + +class Files extends \Test\TestCase { + + const UPLOAD_LIMIT_DEFAULT_STR = '513M'; + const UPLOAD_LIMIT_SETTING_STR = '2M'; + const UPLOAD_LIMIT_SETTING_BYTES = 2097152; + + /** @var array $tmpDirs */ + private $tmpDirs = []; + + /** + * @return array + */ + private function getUploadLimitTestFiles() { + $dir = \OC::$server->getTempManager()->getTemporaryFolder(); + $this->tmpDirs[] = $dir; + $result = [ + '.htaccess' => $dir . '/htaccess', + '.user.ini' => $dir . '/user.ini' + ]; + copy(\OC::$SERVERROOT . '/tests/data/setUploadLimit/htaccess', $result['.htaccess']); + copy(\OC::$SERVERROOT . '/tests/data/setUploadLimit/user.ini', $result['.user.ini']); + return $result; + } + + protected function tearDown() { + foreach ($this->tmpDirs as $dir) { + \OC_Helper::rmdirr($dir); + } + parent::tearDown(); + } + + public function testSetUploadLimitSizeSanity() { + $this->assertFalse(\OC_Files::setUploadLimit(PHP_INT_MAX + 10)); + $this->assertFalse(\OC_Files::setUploadLimit(\OC_Files::UPLOAD_MIN_LIMIT_BYTES - 10)); + $this->assertFalse(\OC_Files::setUploadLimit('foobar')); + } + + public function setUploadLimitWriteProvider() { + return [ + [ + // both files writable + true, true, + self::UPLOAD_LIMIT_SETTING_BYTES, self::UPLOAD_LIMIT_SETTING_BYTES, + self::UPLOAD_LIMIT_SETTING_STR, self::UPLOAD_LIMIT_SETTING_STR + ], + [ + // neither file writable + false, false, + self::UPLOAD_LIMIT_SETTING_BYTES, false, + self::UPLOAD_LIMIT_DEFAULT_STR, self::UPLOAD_LIMIT_DEFAULT_STR + ], + [ + // only .htaccess writable + true, false, + self::UPLOAD_LIMIT_SETTING_BYTES, false, + self::UPLOAD_LIMIT_SETTING_STR, self::UPLOAD_LIMIT_DEFAULT_STR + ], + [ + // only .user.ini writable + false, true, + self::UPLOAD_LIMIT_SETTING_BYTES, false, + self::UPLOAD_LIMIT_DEFAULT_STR, self::UPLOAD_LIMIT_SETTING_STR + ], + [ + // test rounding of values + true, true, + self::UPLOAD_LIMIT_SETTING_BYTES + 20, self::UPLOAD_LIMIT_SETTING_BYTES, + self::UPLOAD_LIMIT_SETTING_STR, self::UPLOAD_LIMIT_SETTING_STR + ] + ]; + } + + /** + * @dataProvider setUploadLimitWriteProvider + */ + public function testSetUploadLimitWrite( + $htaccessWritable, $userIniWritable, + $setSize, $expectedSize, + $htaccessStr, $userIniStr + ) { + $files = $this->getUploadLimitTestFiles(); + chmod($files['.htaccess'], ($htaccessWritable ? 0644 : 0444)); + chmod($files['.user.ini'], ($userIniWritable ? 0644 : 0444)); + + $htaccessSize = filesize($files['.htaccess']); + $userIniSize = filesize($files['.user.ini']); + $htaccessSizeMod = 2*(strlen($htaccessStr) - strlen(self::UPLOAD_LIMIT_DEFAULT_STR)); + $userIniSizeMod = 2*(strlen($userIniStr) - strlen(self::UPLOAD_LIMIT_DEFAULT_STR)); + + $this->assertEquals($expectedSize, \OC_Files::setUploadLimit($setSize, $files)); + + // check file contents + $htaccess = file_get_contents($files['.htaccess']); + $this->assertEquals(1, + preg_match('/php_value upload_max_filesize '.$htaccessStr.'/', $htaccess) + ); + $this->assertEquals(1, + preg_match('/php_value post_max_size '.$htaccessStr.'/', $htaccess) + ); + $this->assertEquals($htaccessSize + $htaccessSizeMod, filesize($files['.htaccess'])); + + $userIni = file_get_contents($files['.user.ini']); + $this->assertEquals(1, + preg_match('/upload_max_filesize='.$userIniStr.'/', $userIni) + ); + $this->assertEquals(1, + preg_match('/post_max_size='.$userIniStr.'/', $userIni) + ); + $this->assertEquals($userIniSize + $userIniSizeMod, filesize($files['.user.ini'])); + } +}