From 1e155ffc130aa2972c424e69eca33c585edac0cb Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Tue, 4 Dec 2012 13:26:48 +0000 Subject: [PATCH] Fixed stopper issue (found with an audit) due to copies of DBObjectSearch not cloned (or not cloned well) There is still one place where this should be fixed, but it reveals another bug so we've decided to leave it as is for the moment (see comment in DBObjectSearch::AddCondition_PointingTo) SVN:trunk[2497] --- application/displayblock.class.inc.php | 10 ++++----- core/dbobjectsearch.class.php | 29 +++++++++++++++----------- core/dbobjectset.class.php | 4 ++-- core/metamodel.class.php | 2 +- core/sqlquery.class.inc.php | 8 +++++++ pages/audit.php | 4 ++-- 6 files changed, 35 insertions(+), 22 deletions(-) diff --git a/application/displayblock.class.inc.php b/application/displayblock.class.inc.php index 5b430845b..d87a6c69f 100644 --- a/application/displayblock.class.inc.php +++ b/application/displayblock.class.inc.php @@ -52,7 +52,7 @@ class DisplayBlock public function __construct(DBObjectSearch $oFilter, $sStyle = 'list', $bAsynchronous = false, $aParams = array(), $oSet = null) { - $this->m_oFilter = clone $oFilter; + $this->m_oFilter = $oFilter->DeepClone(); $this->m_aConditions = array(); $this->m_sStyle = $sStyle; $this->m_bAsynchronous = $bAsynchronous; @@ -414,7 +414,7 @@ class DisplayBlock foreach($aGroupBy as $iRow => $iCount) { // Build the search for this subset - $oSubsetSearch = clone $this->m_oFilter; + $oSubsetSearch = $this->m_oFilter->DeepClone(); $oCondition = new BinaryExpression($oGroupByExp, '=', new ScalarExpression($aValues[$iRow])); $oSubsetSearch->AddConditionExpression($oCondition); $sFilter = urlencode($oSubsetSearch->serialize()); @@ -492,7 +492,7 @@ class DisplayBlock $sHtml .= "\n"; // Construct a new (parametric) query that will return the content of this block - $oBlockFilter = clone $this->m_oFilter; + $oBlockFilter = $this->m_oFilter->DeepClone(); $aExpressions = array(); $index = 0; foreach($aGroupByFields as $aField) @@ -726,7 +726,7 @@ class DisplayBlock $oAttDef = MetaModel::GetAttributeDef($sClass, $sStateAttrCode); foreach($aStates as $sStateValue) { - $oFilter = clone($this->m_oFilter); + $oFilter = $this->m_oFilter->DeepClone(); $oFilter->AddCondition($sStateAttrCode, $sStateValue, '='); $oSet = new DBObjectSet($oFilter); $aCounts[$sStateValue] = $oSet->Count(); @@ -911,7 +911,7 @@ EOF foreach($aGroupBy as $iRow => $iCount) { // Build the search for this subset - $oSubsetSearch = clone $this->m_oFilter; + $oSubsetSearch = $this->m_oFilter->DeepClone(); $oCondition = new BinaryExpression($oGroupByExp, '=', new ScalarExpression($aValues[$iRow])); $oSubsetSearch->AddConditionExpression($oCondition); $aURLs[$idx] = $oSubsetSearch->serialize(); diff --git a/core/dbobjectsearch.class.php b/core/dbobjectsearch.class.php index 9dd7ee8b0..2e65d725c 100644 --- a/core/dbobjectsearch.class.php +++ b/core/dbobjectsearch.class.php @@ -71,6 +71,14 @@ class DBObjectSearch $this->m_aModifierProperties = array(); } + /** + * Perform a deep clone (as opposed to "clone" which does copy a reference to the underlying objects + **/ + public function DeepClone() + { + return unserialize(serialize($this)); + } + public function AllowAllData() {$this->m_bAllowAllData = true;} public function IsAllDataAllowed() {return $this->m_bAllowAllData;} public function IsDataFiltered() {return $this->m_bDataFiltered; } @@ -658,6 +666,10 @@ class DBObjectSearch public function AddCondition_PointingTo(DBObjectSearch $oFilter, $sExtKeyAttCode, $iOperatorCode = TREE_OPERATOR_EQUALS) { + // Note: though it seems to be a good practice to clone the given source filter + // (as it was done and fixed an issue in MergeWith()) + // this was not implemented here because it was causing a regression (login as admin, select an org, click on any badge) + // buggy: $oFilter = $oFilter->DeepClone(); $aAliasTranslation = array(); $res = $this->AddCondition_PointingTo_InNameSpace($oFilter, $sExtKeyAttCode, $this->m_aClasses, $aAliasTranslation, $iOperatorCode); $this->TransferConditionExpression($oFilter, $aAliasTranslation); @@ -689,6 +701,7 @@ class DBObjectSearch public function AddCondition_ReferencedBy(DBObjectSearch $oFilter, $sForeignExtKeyAttCode) { + $oFilter = $oFilter->DeepClone(); $aAliasTranslation = array(); $res = $this->AddCondition_ReferencedBy_InNameSpace($oFilter, $sForeignExtKeyAttCode, $this->m_aClasses, $aAliasTranslation); $this->TransferConditionExpression($oFilter, $aAliasTranslation); @@ -721,22 +734,13 @@ class DBObjectSearch $oFilter->AddToNamespace($aClassAliases, $aAliasTranslation); // #@# The condition expression found in that filter should not be used - could be another kind of structure like a join spec tree !!!! - //$oNewFilter = clone $oFilter; + //$oNewFilter = $oFilter->DeepClone(); //$oNewFilter->ResetCondition(); $oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode]= $oFilter; } } - public function AddCondition_LinkedTo(DBObjectSearch $oLinkFilter, $sExtKeyAttCodeToMe, $sExtKeyAttCodeTarget, DBObjectSearch $oFilterTarget) - { - $oLinkFilterFinal = clone $oLinkFilter; - // todo : new function prototype - $oLinkFilterFinal->AddCondition_PointingTo($sExtKeyAttCodeToMe); - - $this->AddCondition_ReferencedBy($oLinkFilterFinal, $sExtKeyAttCodeToMe); - } - public function AddCondition_RelatedTo(DBObjectSearch $oFilter, $sRelCode, $iMaxDepth) { MyHelpers::CheckValueInArray('relation code', $sRelCode, MetaModel::EnumRelations()); @@ -745,6 +749,7 @@ class DBObjectSearch public function MergeWith($oFilter) { + $oFilter = $oFilter->DeepClone(); $aAliasTranslation = array(); $res = $this->MergeWith_InNamespace($oFilter, $this->m_aClasses, $aAliasTranslation); $this->TransferConditionExpression($oFilter, $aAliasTranslation); @@ -1166,7 +1171,7 @@ class DBObjectSearch if ($bOQLCacheEnabled && array_key_exists($sQuery, self::$m_aOQLQueries)) { // hit! - $oClone = clone self::$m_aOQLQueries[$sQuery]; + $oClone = self::$m_aOQLQueries[$sQuery]->DeepClone(); if (!is_null($aParams)) { $oClone->m_aParams = $aParams; @@ -1316,7 +1321,7 @@ class DBObjectSearch if ($bOQLCacheEnabled) { - self::$m_aOQLQueries[$sQuery] = clone $oResultFilter; + self::$m_aOQLQueries[$sQuery] = $oResultFilter->DeepClone(); } return $oResultFilter; diff --git a/core/dbobjectset.class.php b/core/dbobjectset.class.php index d6d7a1f68..ee81bd7f1 100644 --- a/core/dbobjectset.class.php +++ b/core/dbobjectset.class.php @@ -42,7 +42,7 @@ class DBObjectSet public function __construct(DBObjectSearch $oFilter, $aOrderBy = array(), $aArgs = array(), $aExtendedDataSpec = null, $iLimitCount = 0, $iLimitStart = 0) { - $this->m_oFilter = clone $oFilter; + $this->m_oFilter = $oFilter->DeepClone(); $this->m_aAddedIds = array(); $this->m_aOrderBy = $aOrderBy; $this->m_aArgs = $aArgs; @@ -271,7 +271,7 @@ class DBObjectSet public function GetFilter() { // Make sure that we carry on the parameters of the set with the filter - $oFilter = clone $this->m_oFilter; + $oFilter = $this->m_oFilter->DeepClone(); $oFilter->SetInternalParams(array_merge($oFilter->GetInternalParams(), $this->m_aArgs)); if (count($this->m_aAddedIds) == 0) diff --git a/core/metamodel.class.php b/core/metamodel.class.php index c1829cd69..d2013ed1a 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -2305,7 +2305,7 @@ abstract class MetaModel $oKPI->ComputeStats('Query APC (store)', $sOqlQuery); } - self::$m_aQueryStructCache[$sOqlId] = clone $oSelect; + self::$m_aQueryStructCache[$sOqlId] = $oSelect->DeepClone(); } } diff --git a/core/sqlquery.class.inc.php b/core/sqlquery.class.inc.php index 58e11406c..ecaa1a3c7 100644 --- a/core/sqlquery.class.inc.php +++ b/core/sqlquery.class.inc.php @@ -72,6 +72,14 @@ class SQLQuery $this->m_oSelectedIdField = $oSelectedIdField; } + /** + * Perform a deep clone (as opposed to "clone" which does copy a reference to the underlying objects + **/ + public function DeepClone() + { + return unserialize(serialize($this)); + } + public function GetTableAlias() { return $this->m_sTableAlias; diff --git a/pages/audit.php b/pages/audit.php index f9eb316c4..852f61564 100644 --- a/pages/audit.php +++ b/pages/audit.php @@ -94,7 +94,7 @@ function GetRuleResultFilter($iRuleId, $oDefinitionFilter, $oAppContext) if ($oRule->Get('valid_flag') == 'false') { // The query returns directly the invalid elements - $oFilter = $oRuleFilter; + $oFilter = $oRuleFilter; $oFilter->MergeWith($oDefinitionFilter); } else @@ -106,7 +106,7 @@ function GetRuleResultFilter($iRuleId, $oDefinitionFilter, $oAppContext) { $aValidIds[] = $aRow['id']; } - $oFilter = clone $oDefinitionFilter; + $oFilter = $oDefinitionFilter->DeepClone(); if (count($aValidIds) > 0) { $aInDefSet = array();