Merge branch 'support/2.7' into feature/OAuthMail

application/dashlet.class.inc.php

@@ -255,7 +255,7 @@ abstract class Dashlet
 		catch(OqlException $e)
 		{
 			$oPage->add('<div class="dashlet-content">');
-			$oPage->p($e->GetUserFriendlyDescription());
+			$oPage->p(utils::HtmlEntities($e->GetUserFriendlyDescription()));
 			$oPage->add('</div>');
 		}
 		catch(Exception $e)

application/utils.inc.php

@@ -283,6 +283,7 @@ class utils
 	 *
 	 * @since 2.5.2 2.6.0 new 'transaction_id' filter
 	 * @since 2.7.0 new 'element_identifier' filter
+	 * @since 2.7.7, 3.0.2, 3.1.0 new 'url' N°4899
 	 */
 	protected static function Sanitize_Internal($value, $sSanitizationFilter)
 	{
@@ -358,6 +359,11 @@ class utils
 				$retValue = preg_replace('/[^a-zA-Z0-9_]/', '', $value);
 				break;
 
+			// For URL
+			case 'url':
+				$retValue = filter_var($value, FILTER_SANITIZE_URL);
+				break;
+
 			default:
 			case 'raw_data':
 				$retValue = $value;

core/action.class.inc.php

@@ -179,7 +179,7 @@ class ActionEmail extends ActionNotification
 	protected function FindRecipients($sRecipAttCode, $aArgs)
 	{
 		$sOQL = $this->Get($sRecipAttCode);
-		if (strlen($sOQL) == '') return '';
+		if (strlen($sOQL) === 0) return '';
 
 		try
 		{

core/dbobject.class.php

@@ -1977,9 +1977,9 @@ abstract class DBObject implements iDisplay
 	/**
 	 * check attributes together
 	 *
-     * @overwritable-hook You can extend this method in order to provide your own logic.
-     * 
-	 * @return bool 
+	 * @overwritable-hook You can extend this method in order to provide your own logic.
+	 *
+	 * @return true|string true if successful, the error description otherwise
 	 */
 	public function CheckConsistency()
 	{

core/displayablegraph.class.inc.php

@@ -1203,8 +1203,10 @@ class DisplayableGraph extends SimpleGraph
 	 * @param float $xMax Right coordinate of the bounding box to display the graph
 	 * @param float $yMin Top coordinate of the bounding box to display the graph
 	 * @param float $yMax Bottom coordinate of the bounding box to display the graph
+	 *
+	 * @since 2.7.7 3.0.2 3.1.0 N°4985 $sComments param is no longer optional
 	 */
-	function RenderAsPDF(PDFPage $oPage, $sComments = '', $sContextKey, $xMin = -1, $xMax = -1, $yMin = -1, $yMax = -1)
+	function RenderAsPDF(PDFPage $oPage, $sComments, $sContextKey, $xMin = -1, $xMax = -1, $yMin = -1, $yMax = -1)
 	{
 		$aContextDefs = static::GetContextDefinitions($sContextKey, false); // No need to develop the parameters
 		$oPdf = $oPage->get_tcpdf();

datamodels/2.x/itop-portal-base/portal/src/Brick/BrickCollection.php

@@ -162,12 +162,20 @@ class BrickCollection
 		//   - Home
 		$this->aHomeOrdering = $this->aAllowedBricks;
 		usort($this->aHomeOrdering, function (PortalBrick $a, PortalBrick $b) {
-			return $a->GetRankHome() > $b->GetRankHome();
+			if ($a->GetRankHome() === $b->GetRankHome()) {
+				return 0;
+			}
+
+			return $a->GetRankHome() > $b->GetRankHome() ? 1 : -1;
 		});
 		//    - Navigation menu
 		$this->aNavigationMenuOrdering = $this->aAllowedBricks;
 		usort($this->aNavigationMenuOrdering, function (PortalBrick $a, PortalBrick $b) {
-			return $a->GetRankNavigationMenu() > $b->GetRankNavigationMenu();
+			if ($a->GetRankNavigationMenu() === $b->GetRankNavigationMenu()) {
+				return 0;
+			}
+
+			return $a->GetRankNavigationMenu() > $b->GetRankNavigationMenu() ? 1 : -1;
 		});
 	}
 

datamodels/2.x/itop-portal-base/portal/src/Brick/ManageBrick.php

@@ -481,7 +481,11 @@ class ManageBrick extends PortalBrick
 		if (!$this->IsGroupingByDistinctValues($sName))
 		{
 			usort($this->aGrouping[$sName]['groups'], function ($a, $b) {
-				return $a['rank'] > $b['rank'];
+				if ($a['rank'] === $b['rank']) {
+					return 0;
+				}
+
+				return $a['rank'] > $b['rank'] ? 1 : -1;
 			});
 		}
 

datamodels/2.x/itop-portal-base/portal/src/DependencyInjection/SilexCompatBootstrap/PortalXmlConfiguration/Lists.php

@@ -91,7 +91,10 @@ class Lists extends AbstractConfiguration
 				}
 				// - Sorting list items by rank
 				usort($aListItems, function ($a, $b) {
-					return $a['rank'] > $b['rank'];
+					if ($a['rank'] == $b['rank']) {
+						return 0;
+					}
+					return $a['rank'] > $b['rank'] ? 1 : -1;
 				});
 				$aClassLists[$sListId] = $aListItems;
 			}

datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php

@@ -703,7 +703,7 @@ class ObjectFormManager extends FormManager
 
 			/** @var Field $oField */
 			$oField = null;
-			if (is_callable(get_class($oAttDef).'::MakeFormField'))
+			if (is_callable([$oAttDef, 'MakeFormField']))
 			{
 				$oField = $oAttDef->MakeFormField($this->oObject);
 			}

pages/ajax.render.php

@@ -1183,7 +1183,7 @@ try
 
 			$aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data');
 			$sDashboardFile = utils::ReadParam('file', '', false, 'raw_data');
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			$oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sDashboardId);
 			$aResult = array('error' => '');
 			if (!is_null($oDashboard))
@@ -1202,7 +1202,7 @@ try
 			$sDashboardId = utils::ReadParam('dashboard_id', '', false, 'raw_data');
 			$aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data');
 			$sDashboardFile = utils::ReadParam('file', '', false, 'raw_data');
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			$oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sDashboardId);
 			$aResult = array('error' => '');
 			if (!is_null($oDashboard))
@@ -1219,7 +1219,7 @@ try
 		case 'save_dashboard':
 			$sDashboardId = utils::ReadParam('dashboard_id', '', false, 'context_param');
 			$aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data');
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			$sJSExtraParams = json_encode($aExtraParams);
 			$aParams = array();
 			$aParams['layout_class'] = utils::ReadParam('layout_class', '');
@@ -1252,7 +1252,7 @@ JS
 
 		case 'revert_dashboard':
 			$sDashboardId = utils::ReadParam('dashboard_id', '', false, 'raw_data');
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			appUserPreferences::UnsetPref('display_original_dashboard_'.$sDashboardId);
 			$oDashboard = new RuntimeDashboard($sDashboardId);
 			$oDashboard->Revert();
@@ -1282,7 +1282,7 @@ EOF
 			$aParams['cells'] = utils::ReadParam('cells', array(), false, 'raw_data');
 			$aParams['auto_reload'] = utils::ReadParam('auto_reload', false);
 			$aParams['auto_reload_sec'] = utils::ReadParam('auto_reload_sec', 300);
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			$oKPI = new ExecutionKPI();
 			$oDashboard = new RuntimeDashboard($sDashboardId);
 			$oDashboard->FromParams($aParams);
@@ -1296,7 +1296,7 @@ EOF
 			$aExtraParams = utils::ReadParam('extra_params', array(), false, 'raw_data');
 			$aExtraParams['dashboard_div_id'] = utils::Sanitize($sId, '', 'element_identifier');
 			$sDashboardFile = utils::ReadParam('file', '', false, 'string');
-			$sReloadURL = utils::ReadParam('reload_url', '', false, 'raw_data');
+			$sReloadURL = utils::ReadParam('reload_url', '', false, 'url');
 			$oKPI = new ExecutionKPI();
 			$oDashboard = RuntimeDashboard::GetDashboard($sDashboardFile, $sId);
 			if (!is_null($oDashboard))

pages/csvimport.php

@@ -197,6 +197,11 @@ try {
 			throw new CoreException(Dict::S('UI:ActionNotAllowed'));
 		}
 
+		// CSRF transaction id verification
+		if(!$bSimulate && !utils::IsTransactionValid(utils::ReadPostedParam('transaction_id', '', 'raw_data'))){
+			throw new CoreException(Dict::S('UI:Error:InvalidToken'));
+		}
+
 		$aResult = array();
 		$sCSVData = utils::ReadParam('csvdata', '', false, 'raw_data');
 		$sCSVDataTruncated = utils::ReadParam('csvdata_truncated', '', false, 'raw_data');
@@ -487,11 +492,12 @@ try {
 			$sHtml .= "<td class=\"$sCSSMessageClass\" style=\"background-color:#f1f1f1;\">$sMessage</td>";
 			$sHtml .= '</tr>';
 		}
-		
+
 		$iUnchanged = count($aRes) - $iErrors - $iModified - $iCreated;
 		$sHtml .= '</table>';
 		$oPage->add('<div class="wizContainer" style="width:auto;display:inline-block;">');
 		$oPage->add('<form enctype="multipart/form-data" id="wizForm" method="post">');
+		$oPage->add('<input type="hidden" name="transaction_id" value="' . utils::GetNewTransactionId() . '">');
 		$oPage->add('<input type="hidden" name="step" value="'.($iCurrentStep+1).'"/>');
 		$oPage->add('<input type="hidden" name="separator" value="'.htmlentities($sSeparator, ENT_QUOTES, 'UTF-8').'"/>');
 		$oPage->add('<input type="hidden" name="text_qualifier" value="'.htmlentities($sTextQualifier, ENT_QUOTES, 'UTF-8').'"/>');

test/application/UtilsTest.php

@@ -22,7 +22,7 @@
 /**
  * @covers utils
  */
-class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase
+class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopDataTestCase
 {
 	public function testEndsWith()
 	{
@@ -441,4 +441,50 @@ class UtilsTest extends \Combodo\iTop\Test\UnitTest\ItopTestCase
 			'2G'            => ['2G', 2 * 1024 * 1024 * 1024],
 		];
 	}
+
+	/**
+	 * Test sanitizer.
+	 *
+	 * @param $type string type of sanitizer
+	 * @param $valueToSanitize ? value to sanitize
+	 * @param $expectedResult ? expected result
+	 *
+	 * @return void
+	 *
+	 * @dataProvider sanitizerDataProvider
+	 */
+	public function testSanitizer($type, $valueToSanitize, $expectedResult)
+	{
+		$this->assertEquals($expectedResult, utils::Sanitize($valueToSanitize, null, $type), 'url sanitize failed');
+	}
+
+	/**
+	 * DataProvider for testSanitizer
+	 *
+	 * @return array
+	 */
+	public function sanitizerDataProvider()
+	{
+		return [
+			'good integer' => ['integer', '2565', '2565'],
+			'bad integer' => ['integer', 'a2656', '2656'],
+			'good class' => ['class', 'UserRequest', 'UserRequest'],
+			'bad class' => ['class', 'MyUserRequest',null],
+			'good string' => ['string', 'Is Peter smart and funny?', 'Is Peter smart and funny?'],
+			'bad string' => ['string', 'Is Peter <smart> & funny?', 'Is Peter &#60;smart&#62; &#38; funny?'],
+			'good transaction_id' => ['transaction_id', '8965.-dd', '8965.-dd'],
+			'bad transaction_id' => ['transaction_id', '8965.-dd+', null],
+			'good parameter' => ['parameter', 'JU8965-dd=_', 'JU8965-dd=_'],
+			'bad parameter' => ['parameter', '8965.-dd+', null],
+			'good field_name' => ['field_name', 'Name->bUzz38', 'Name->bUzz38'],
+			'bad field_name' => ['field_name', 'name-buzz', null],
+			'good context_param' => ['context_param', '%dssD25_=%:+-', '%dssD25_=%:+-'],
+			'bad context_param' => ['context_param', '%dssD,25_=%:+-', null],
+			'good element_identifier' => ['element_identifier', 'AD05nb', 'AD05nb'],
+			'bad element_identifier' => ['element_identifier', 'AD05nb+', 'AD05nb'],
+			'good url' => ['url', 'https://www.w3schools.com', 'https://www.w3schools.com'],
+			'bad url' => ['url', 'https://www.w3schoo<6F><6F>ls.co<63>m', 'https://www.w3schools.com'],
+			'raw_data' => ['raw_data', '<Test>\s😃😃😃', '<Test>\s😃😃😃'],
+		];
+	}
 }

test/phpunit.xml.dist

@@ -44,10 +44,6 @@
     <testsuite name="Itop-Tickets">
       <directory>itop-tickets</directory>
     </testsuite>
-    <!-- OQL : taking too long (20min+)... we should move this to nightlies ! See N°4655 -->
-    <!-- testsuite name="OQL">
-        <directory>OQL</directory>
-    </testsuite -->
     <testsuite name="Setup">
       <directory>setup</directory>
     </testsuite>

test/phpunit_nightly.xml.dist

@@ -0,0 +1,42 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/8.5/phpunit.xsd"
+        bootstrap="unittestautoload.php"
+        backupGlobals="true"
+        colors="true"
+        convertErrorsToExceptions="true"
+        convertNoticesToExceptions="true"
+        convertWarningsToExceptions="true"
+        processIsolation="false"
+        stopOnError="false"
+        stopOnFailure="false"
+        stopOnIncomplete="false"
+        stopOnRisky="false"
+        stopOnSkipped="false"
+        verbose="true"
+>
+
+    <php>
+        <ini name="error_reporting" value="E_ALL"/>
+        <ini name="display_errors" value="On"/>
+        <ini name="log_errors" value="On"/>
+        <ini name="html_errors" value="On"/>
+    </php>
+
+    <testsuites>
+        <testsuite name="OQL">
+            <directory>OQL</directory>
+        </testsuite>
+    </testsuites>
+
+    <!-- Code coverage white list -->
+    <filter>
+        <whitelist>
+            <file>../core/apc-emulation.php</file>
+            <file>../core/ormlinkset.class.inc.php</file>
+            <file>../datamodels/2.x/itop-tickets/main.itop-tickets.php</file>
+        </whitelist>
+    </filter>
+
+</phpunit>