Merge pull request #800 from ArthurHoaro/hotfix/get-bytes-warning

Fix a warning generated in return_bytes function and refactor it

4 files changed, 172 insertions, 31 deletions

diff --git a/application/Utils.php b/application/Utils.php
index d6e06610..3ef2a7e2 100644
--- a/application/Utils.php
+++ b/application/Utils.php
@@ -345,3 +345,92 @@ function format_date($date, $time = true, $intl = true)
 
     return $formatter->format($date);
 }
+
+/**
+ * Check if the input is an integer, no matter its real type.
+ *
+ * PHP is a bit messy regarding this:
+ *   - is_int returns false if the input is a string
+ *   - ctype_digit returns false if the input is an integer or negative
+ *
+ * @param mixed $input value
+ *
+ * @return bool true if the input is an integer, false otherwise
+ */
+function is_integer_mixed($input)
+{
+    if (is_array($input) || is_bool($input) || is_object($input)) {
+        return false;
+    }
+    $input = strval($input);
+    return ctype_digit($input) || (startsWith($input, '-') && ctype_digit(substr($input, 1)));
+}
+
+/**
+ * Convert post_max_size/upload_max_filesize (e.g. '16M') parameters to bytes.
+ *
+ * @param string $val Size expressed in string.
+ *
+ * @return int Size expressed in bytes.
+ */
+function return_bytes($val)
+{
+    if (is_integer_mixed($val) || $val === '0' || empty($val)) {
+        return $val;
+    }
+    $val = trim($val);
+    $last = strtolower($val[strlen($val)-1]);
+    $val = intval(substr($val, 0, -1));
+    switch($last) {
+        case 'g': $val *= 1024;
+        case 'm': $val *= 1024;
+        case 'k': $val *= 1024;
+    }
+    return $val;
+}
+
+/**
+ * Return a human readable size from bytes.
+ *
+ * @param int $bytes value
+ *
+ * @return string Human readable size
+ */
+function human_bytes($bytes)
+{
+    if ($bytes === '') {
+        return t('Setting not set');
+    }
+    if (! is_integer_mixed($bytes)) {
+        return $bytes;
+    }
+    $bytes = intval($bytes);
+    if ($bytes === 0) {
+        return t('Unlimited');
+    }
+
+    $units = [t('B'), t('kiB'), t('MiB'), t('GiB')];
+    for ($i = 0; $i < count($units) && $bytes >= 1024; ++$i) {
+        $bytes /= 1024;
+    }
+
+    return $bytes . $units[$i];
+}
+
+/**
+ * Try to determine max file size for uploads (POST).
+ * Returns an integer (in bytes)
+ *
+ * @param mixed $limitPost   post_max_size PHP setting
+ * @param mixed $limitUpload upload_max_filesize PHP setting
+ *
+ * @return int max upload file size in bytes.
+ */
+function get_max_upload_size($limitPost, $limitUpload)
+{
+    $size1 = return_bytes($limitPost);
+    $size2 = return_bytes($limitUpload);
+    // Return the smaller of two:
+    $maxsize = min($size1, $size2);
+    return human_bytes($maxsize);
+}
diff --git a/index.php b/index.php
index 863d5093..8f26c390 100644
--- a/index.php
+++ b/index.php
@@ -473,34 +473,6 @@ if (isset($_POST['login']))
 }
 
 // ------------------------------------------------------------------------------------------
-// Misc utility functions:
-
-// Convert post_max_size/upload_max_filesize (e.g. '16M') parameters to bytes.
-function return_bytes($val)
-{
-    $val = trim($val); $last=strtolower($val[strlen($val)-1]);
-    switch($last)
-    {
-        case 'g': $val *= 1024;
-        case 'm': $val *= 1024;
-        case 'k': $val *= 1024;
-    }
-    return $val;
-}
-
-// Try to determine max file size for uploads (POST).
-// Returns an integer (in bytes)
-function getMaxFileSize()
-{
-    $size1 = return_bytes(ini_get('post_max_size'));
-    $size2 = return_bytes(ini_get('upload_max_filesize'));
-    // Return the smaller of two:
-    $maxsize = min($size1,$size2);
-    // FIXME: Then convert back to readable notations ? (e.g. 2M instead of 2000000)
-    return $maxsize;
-}
-
-// ------------------------------------------------------------------------------------------
 // Token management for XSRF protection
 // Token should be used in any form which acts on data (create,update,delete,import...).
 if (!isset($_SESSION['tokens'])) $_SESSION['tokens']=array();  // Token are attached to the session.
@@ -1517,7 +1489,7 @@ function renderPage($conf, $pluginManager, $LINKSDB)
 
         if (! isset($_POST['token']) || ! isset($_FILES['filetoupload'])) {
             // Show import dialog
-            $PAGE->assign('maxfilesize', getMaxFileSize());
+            $PAGE->assign('maxfilesize', get_max_upload_size(ini_get('post_max_size'), ini_get('upload_max_filesize')));
             $PAGE->renderPage('import');
             exit;
         }
@@ -1527,7 +1499,7 @@ function renderPage($conf, $pluginManager, $LINKSDB)
             // The file is too big or some form field may be missing.
             echo '<script>alert("The file you are trying to upload is probably'
                 .' bigger than what this webserver can accept ('
-                .getMaxFileSize().' bytes).'
+                .get_max_upload_size(ini_get('post_max_size'), ini_get('upload_max_filesize')).').'
                 .' Please upload in smaller chunks.");document.location=\'?do='
                 .Router::$PAGE_IMPORT .'\';</script>';
             exit;
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index e70cc1ae..e5ff01e6 100644
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -4,6 +4,7 @@
  */
 
 require_once 'application/Utils.php';
+require_once 'application/Languages.php';
 require_once 'tests/utils/ReferenceSessionIdHashes.php';
 
 // Initialize reference data before PHPUnit starts a session
@@ -326,4 +327,83 @@ class UtilsTest extends PHPUnit_Framework_TestCase
         $this->assertFalse(format_date([]));
         $this->assertFalse(format_date(null));
     }
+
+    /**
+     * Test is_integer_mixed with valid values
+     */
+    public function testIsIntegerMixedValid()
+    {
+        $this->assertTrue(is_integer_mixed(12));
+        $this->assertTrue(is_integer_mixed('12'));
+        $this->assertTrue(is_integer_mixed(-12));
+        $this->assertTrue(is_integer_mixed('-12'));
+        $this->assertTrue(is_integer_mixed(0));
+        $this->assertTrue(is_integer_mixed('0'));
+        $this->assertTrue(is_integer_mixed(0x0a));
+    }
+
+    /**
+     * Test is_integer_mixed with invalid values
+     */
+    public function testIsIntegerMixedInvalid()
+    {
+        $this->assertFalse(is_integer_mixed(true));
+        $this->assertFalse(is_integer_mixed(false));
+        $this->assertFalse(is_integer_mixed([]));
+        $this->assertFalse(is_integer_mixed(['test']));
+        $this->assertFalse(is_integer_mixed([12]));
+        $this->assertFalse(is_integer_mixed(new DateTime()));
+        $this->assertFalse(is_integer_mixed('0x0a'));
+        $this->assertFalse(is_integer_mixed('12k'));
+        $this->assertFalse(is_integer_mixed('k12'));
+        $this->assertFalse(is_integer_mixed(''));
+    }
+
+    /**
+     * Test return_bytes
+     */
+    public function testReturnBytes()
+    {
+        $this->assertEquals(2 * 1024, return_bytes('2k'));
+        $this->assertEquals(2 * 1024, return_bytes('2K'));
+        $this->assertEquals(2 * (pow(1024, 2)), return_bytes('2m'));
+        $this->assertEquals(2 * (pow(1024, 2)), return_bytes('2M'));
+        $this->assertEquals(2 * (pow(1024, 3)), return_bytes('2g'));
+        $this->assertEquals(2 * (pow(1024, 3)), return_bytes('2G'));
+        $this->assertEquals(374, return_bytes('374'));
+        $this->assertEquals(374, return_bytes(374));
+        $this->assertEquals(0, return_bytes('0'));
+        $this->assertEquals(0, return_bytes(0));
+        $this->assertEquals(-1, return_bytes('-1'));
+        $this->assertEquals(-1, return_bytes(-1));
+        $this->assertEquals('', return_bytes(''));
+    }
+
+    /**
+     * Test human_bytes
+     */
+    public function testHumanBytes()
+    {
+        $this->assertEquals('2kiB', human_bytes(2 * 1024));
+        $this->assertEquals('2kiB', human_bytes(strval(2 * 1024)));
+        $this->assertEquals('2MiB', human_bytes(2 * (pow(1024, 2))));
+        $this->assertEquals('2MiB', human_bytes(strval(2 * (pow(1024, 2)))));
+        $this->assertEquals('2GiB', human_bytes(2 * (pow(1024, 3))));
+        $this->assertEquals('2GiB', human_bytes(strval(2 * (pow(1024, 3)))));
+        $this->assertEquals('374B', human_bytes(374));
+        $this->assertEquals('374B', human_bytes('374'));
+        $this->assertEquals('Unlimited', human_bytes('0'));
+        $this->assertEquals('Unlimited', human_bytes(0));
+        $this->assertEquals('Setting not set', human_bytes(''));
+    }
+
+    /**
+     * Test get_max_upload_size
+     */
+    public function testGetMaxUploadSize()
+    {
+        $this->assertEquals('1MiB', get_max_upload_size(2097152, '1024k'));
+        $this->assertEquals('1MiB', get_max_upload_size('1m', '2m'));
+        $this->assertEquals('100B', get_max_upload_size(100, 100));
+    }
 }
diff --git a/tpl/vintage/import.html b/tpl/vintage/import.html
index 071e1160..bb9e4a56 100644
--- a/tpl/vintage/import.html
+++ b/tpl/vintage/import.html
@@ -5,7 +5,7 @@
 <div id="pageheader">
   {include="page.header"}
   <div id="uploaddiv">
-    Import Netscape HTML bookmarks (as exported from Firefox/Chrome/Opera/Delicious/Diigo...) (Max: {$maxfilesize} bytes).
+    Import Netscape HTML bookmarks (as exported from Firefox/Chrome/Opera/Delicious/Diigo...) (Max: {$maxfilesize}).
     <form method="POST" action="?do=import" enctype="multipart/form-data"
           name="uploadform" id="uploadform">
       <input type="hidden" name="token" value="{$token}">