Fixed bug that caused memory_limit=-1 to lead to 'not enough memory' … (#1)

* Fixed bug that caused memory_limit=-1 to lead to 'not enough memory' error
* Added Unit Test to Memory Limit Check

application/utils.inc.php

@@ -559,6 +559,18 @@ class utils
         return $iReturn;
     }
   
+  /**
+   * Checks if the memory limit is at least what is required
+   *
+   * @param int $memoryLimit set limit in bytes
+   * @param int $requiredLimit required limit in bytes
+   * @return bool
+   */
+  public static function IsMemoryLimitOk($memoryLimit, $requiredLimit)
+  {
+      return ($memoryLimit >= $requiredLimit) || ($memoryLimit == -1);
+  }
+
 	/**
 	 * Format a value into a more friendly format (KB, MB, GB, TB) instead a juste a Bytes amount.
 	 *

setup/ajax.dataloader.php

@@ -62,7 +62,7 @@ else
 	// Check that the limit will allow us to load the data
 	//
 	$iMemoryLimit = utils::ConvertToBytes($sMemoryLimit);
-	if ($iMemoryLimit < SAFE_MINIMUM_MEMORY)
+	if (!utils::IsMemoryLimitOk($iMemoryLimit, SAFE_MINIMUM_MEMORY))
 	{
 		if (ini_set('memory_limit', SAFE_MINIMUM_MEMORY) === FALSE)
 		{

setup/setuputils.class.inc.php

@@ -274,7 +274,7 @@ class SetupUtils
 			// Check that the limit will allow us to load the data
 			//
 			$iMemoryLimit = utils::ConvertToBytes($sMemoryLimit);
-			if ($iMemoryLimit < self::MIN_MEMORY_LIMIT)
+			if (!utils::IsMemoryLimitOk($iMemoryLimit, self::MIN_MEMORY_LIMIT))
 			{
 				$aResult[] = new CheckResult(CheckResult::ERROR, "memory_limit ($iMemoryLimit) is too small, the minimum value to run the application is ".self::MIN_MEMORY_LIMIT.".");
 			}

test/application/UtilsTest.php

@@ -0,0 +1,53 @@
+<?php
+/**
+ * Copyright (C) 2018 Dennis Lassiter
+ *
+ * This file is part of iTop.
+ *
+ *  iTop is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * iTop 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 iTop. If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase
+{
+    public function setUp()
+    {
+        parent::setUp();
+        require_once(APPROOT . 'application/utils.inc.php');
+    }
+
+    /**
+     * @dataProvider memoryLimitDataProvider
+     */
+    public function testIsMemoryLimit($expected, $memoryLimit, $requiredMemory)
+    {
+        $this->assertSame($expected, utils::IsMemoryLimitOk($memoryLimit, $requiredMemory));
+    }
+
+    /**
+     * DataProvider for testIsMemoryLimitOk
+     *
+     * @return array
+     */
+    public function memoryLimitDataProvider()
+    {
+        return [
+            [true, '-1', 1024],
+            [true, -1, 1024],
+            [true, 1024, 1024],
+            [true, 2048, 1024],
+            [false, 1024, 2048],
+        ];
+    }
+}

webservices/backoffice.dataloader.php

@@ -59,7 +59,7 @@ function SetMemoryLimit($oP)
 		// Check that the limit will allow us to load the data
 		//
 		$iMemoryLimit = utils::ConvertToBytes($sMemoryLimit);
-		if ($iMemoryLimit < SAFE_MINIMUM_MEMORY)
+		if (!utils::IsMemoryLimitOk($iMemoryLimit, SAFE_MINIMUM_MEMORY))
 		{
 			if (ini_set('memory_limit', SAFE_MINIMUM_MEMORY) === FALSE)
 			{