diff --git a/core/coreexception.class.inc.php b/core/coreexception.class.inc.php index cfc741012..3365f8575 100644 --- a/core/coreexception.class.inc.php +++ b/core/coreexception.class.inc.php @@ -188,7 +188,7 @@ class CoreOqlException extends CoreException } -class CoreOqlMultipleResultsFoundException extends CoreOqlException +class CoreOqlMultipleResultsForbiddenException extends CoreOqlException { } diff --git a/core/dbsearch.class.php b/core/dbsearch.class.php index 3e16ae7ae..c129497ba 100644 --- a/core/dbsearch.class.php +++ b/core/dbsearch.class.php @@ -1077,16 +1077,16 @@ abstract class DBSearch * @param array $aSearchParams * * @return null|\DBObject query result - * @throws \CoreOqlMultipleResultsFoundException if multiple results found and parameter enforce the check + * @throws \CoreOqlMultipleResultsForbiddenException if multiple results found and parameter enforce the check * @throws \CoreException * @throws \CoreUnexpectedValue * @throws \MySQLException */ public function GetFirstResult($bMustHaveOneResultMax = true, $aOrderBy = array(), $aSearchParams = array()) { - $oSet = new DBObjectSet($this, array(), $aSearchParams); + $oSet = new DBObjectSet($this, array(), $aSearchParams, null, 2); $oFirstResult = $oSet->Fetch(); - if ($oFirstResult === null) + if ($oFirstResult === null) // useless but here for readability ;) { return null; } @@ -1094,9 +1094,10 @@ abstract class DBSearch if ($bMustHaveOneResultMax) { $oSecondResult = $oSet->Fetch(); - if ($oSecondResult != null) + if ($oSecondResult !== null) { - throw new CoreOqlMultipleResultsFoundException('TODO'); + throw new CoreOqlMultipleResultsForbiddenException( + 'Search returned multiple results, this is forbidden. Query was: '.$this->ToOQL()); } } diff --git a/datamodels/2.x/itop-portal-base/portal/src/Helper/ContextManipulatorHelper.php b/datamodels/2.x/itop-portal-base/portal/src/Helper/ContextManipulatorHelper.php index 904ca491d..31716ab00 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Helper/ContextManipulatorHelper.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Helper/ContextManipulatorHelper.php @@ -22,19 +22,22 @@ namespace Combodo\iTop\Portal\Helper; -use ModuleDesign; +use BinaryExpression; use Combodo\iTop\Portal\Brick\BrickCollection; -use Exception; -use DOMNodeList; -use DOMFormatException; -use Symfony\Component\Routing\RouterInterface; -use UserRights; +use CoreOqlMultipleResultsForbiddenException; +use CoreUnexpectedValue; use DBObject; use DBSearch; -use DBObjectSet; -use BinaryExpression; +use DOMFormatException; +use DOMNodeList; +use Exception; use FieldExpression; +use IssueLog; +use ModuleDesign; use ScalarExpression; +use Symfony\Component\Routing\RouterInterface; +use TrueExpression; +use UserRights; /** * Class ContextManipulatorHelper @@ -342,6 +345,15 @@ class ContextManipulatorHelper } } + $oSearchRootCondition = $oSearch->GetCriteria(); + if (($oSearchRootCondition === null) || ($oSearchRootCondition instanceof TrueExpression)) + { + // N°2555 : disallow searches without any condition + $sErrMsg = "Portal query was stopped: action_rule '$sId' searches for '$sSearchClass' without any condition is forbidden"; + IssueLog::Error($sErrMsg); + throw new CoreUnexpectedValue($sErrMsg); + } + // Checking for silos $oScopeSearch = $this->oScopeValidator->GetScopeFilterForProfiles(UserRights::ListProfiles(), $sSearchClass, UR_ACTION_READ); @@ -351,27 +363,34 @@ class ContextManipulatorHelper } // Retrieving source object(s) and applying rules - $oSet = new DBObjectSet($oSearch, array(), $aSearchParams); - while ($oSourceObject = $oSet->Fetch()) + try { - // Changing behaviour to remove usage of ObjectCopier as its now being integrated in the core - // Old code : iTopObjectCopier::PrepareObject($aRule, $oObject, $oSourceObject); - // Presets - if (isset($aRule['preset']) && !empty($aRule['preset'])) - { - $oObject->ExecActions($aRule['preset'], array('source' => $oSourceObject)); - } - // Retrofits - if (isset($aRule['retrofit']) && !empty($aRule['retrofit'])) - { - $oSourceObject->ExecActions($aRule['retrofit'], array('source' => $oObject)); - } + $oSourceObject = $oSearch->GetFirstResult(true, array(), $aSearchParams); + } + catch (CoreOqlMultipleResultsForbiddenException $e) + { + // N°2555 : disallow searches returning more than 1 result + $sErrMsg = "Portal query was stopped: action_rule '$sId' gives more than 1 '$sSearchClass' result"; + IssueLog::Error($sErrMsg); + throw new CoreUnexpectedValue($sErrMsg); + } + if ($oSourceObject === null) + { + continue; + } + // Presets + if (isset($aRule['preset']) && !empty($aRule['preset'])) + { + $oObject->ExecActions($aRule['preset'], array('source' => $oSourceObject)); + } + // Retrofits + if (isset($aRule['retrofit']) && !empty($aRule['retrofit'])) + { + $oSourceObject->ExecActions($aRule['retrofit'], array('source' => $oObject)); } } else { - // Changing behaviour to remove usage of ObjectCopier as its now being integrated in the core - // Old code : iTopObjectCopier::PrepareObject($aRule, $oObject, $oObject); // Presets if (isset($aRule['preset']) && !empty($aRule['preset'])) { diff --git a/test/core/DBSearchTest.php b/test/core/DBSearchTest.php index 8c7ddb228..29f2c181d 100644 --- a/test/core/DBSearchTest.php +++ b/test/core/DBSearchTest.php @@ -29,7 +29,7 @@ namespace Combodo\iTop\Test\UnitTest\Core; use CMDBSource; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; -use CoreOqlMultipleResultsFoundException; +use CoreOqlMultipleResultsForbiddenException; use DBSearch; use Exception; use Expression; @@ -525,7 +525,7 @@ class DBSearchTest extends ItopDataTestCase * * @param string $sOql query to test * @param bool $bMustHaveOneResultMax arg passed to the tested function - * @param int $iReturn 0 if should return null, 1 if should return object, -1 if should throw + * @param int $sReturn * * @throws \CoreException * @throws \CoreUnexpectedValue @@ -534,7 +534,7 @@ class DBSearchTest extends ItopDataTestCase * * @covers DBSearch::GetFirstResult() */ - public function testGetFirstResult($sOql, $bMustHaveOneResultMax, $iReturn) + public function testGetFirstResult($sOql, $bMustHaveOneResultMax, $sReturn) { $oSearch = DBSearch::FromOQL($sOql); @@ -543,21 +543,21 @@ class DBSearchTest extends ItopDataTestCase { $oFirstResult = $oSearch->GetFirstResult($bMustHaveOneResultMax); } - catch (CoreOqlMultipleResultsFoundException $e) + catch (CoreOqlMultipleResultsForbiddenException $e) { $oFirstResult = null; $bHasThrownException = true; } - switch ($iReturn) + switch ($sReturn) { - case -1: + case 'exception': self::assertEquals(true, $bHasThrownException, 'Exception raised'); break; - case 0: + case 'null': self::assertNull($oFirstResult, 'Null returned'); break; - case 1: + case 'object': self::assertInternalType('object', $oFirstResult, 'Object returned'); break; } @@ -569,22 +569,27 @@ class DBSearchTest extends ItopDataTestCase 'One result' => array( 'SELECT Person WHERE id = 1', false, - 1, + 'object', ), 'Multiple results, no exception' => array( 'SELECT Person', false, - 1, + 'object', ), 'Multiple results, with exception' => array( 'SELECT Person', true, - -1, + 'exception', + ), + 'Multiple results with "WHERE 1", with exception' => array( + 'SELECT Person WHERE 1', + true, + 'exception', ), 'No result' => array( 'SELECT Person WHERE id = -1', true, - 0, + 'null', ), ); }