Browse Source

FIX protection against infinite loop on hierarchy

Laurent Destailleur 8 years ago
parent
commit
32678015c9
3 changed files with 35 additions and 12 deletions
  1. 6 3
      htdocs/core/lib/treeview.lib.php
  2. 20 8
      htdocs/user/class/user.class.php
  3. 9 1
      htdocs/user/hierarchy.php

+ 6 - 3
htdocs/core/lib/treeview.lib.php

@@ -97,7 +97,7 @@ function tree_showpad(&$fulltree,$key,$silent=0)
 // ------------------------------- Used by menu editor, category view, ... -----------------
 
 /**
- *  Recursive function to output menu tree. <ul id="iddivjstree"><li>...</li></ul>
+ *  Recursive function to output a tree. <ul id="iddivjstree"><li>...</li></ul>
  *  It is also used for the tree of categories.
  *  Note: To have this function working, check you have loaded the js and css for treeview.
  *  $arrayofjs=array('/includes/jquery/plugins/jquerytreeview/jquery.treeview.js',
@@ -106,7 +106,7 @@ function tree_showpad(&$fulltree,$key,$silent=0)
  *  TODO Replace with jstree plugin instead of treeview plugin.
  *
  *  @param	array	$tab    		Array of all elements
- *  @param  int	    $pere   		Array with parent ids ('rowid'=>,'mainmenu'=>,'leftmenu'=>,'fk_mainmenu=>,'fk_leftmenu=>)
+ *  @param  array   $pere   		Array with parent ids ('rowid'=>,'mainmenu'=>,'leftmenu'=>,'fk_mainmenu=>,'fk_leftmenu=>)
  *  @param  int	    $rang   		Level of element
  *  @param	string	$iddivjstree	Id to use for parent ul element
  *  @param  int     $donoresetalreadyloaded     Do not reset global array $donoresetalreadyloaded used to avoid to go down on an aleady processed record
@@ -139,7 +139,10 @@ function tree_recur($tab, $pere, $rang, $iddivjstree='iddivjstree', $donoresetal
 		print '<ul id="'.$iddivjstree.'">';
 	}
 	
-	if ($rang > 50)	return;	// Protect against infinite loop. Max 50 depth
+	if ($rang > 50)	
+	{
+	    return;	// Protect against infinite loop. Max 50 depth
+	}
 
 	//ballayage du tableau
 	$sizeoftab=count($tab);

+ 20 - 8
htdocs/user/class/user.class.php

@@ -2544,7 +2544,12 @@ class User extends CommonObject
 		dol_syslog(get_class($this)."::get_full_tree call to build_path_from_id_user", LOG_DEBUG);
 		foreach($this->users as $key => $val)
 		{
-			$this->build_path_from_id_user($key,0);	// Process a branch from the root user key (this user has no parent)
+			$result = $this->build_path_from_id_user($key,0);	// Process a branch from the root user key (this user has no parent)
+			if ($result < 0) 
+			{
+			    $this->error='ErrorLoopInHierarchy';
+			    return -1;
+			}
 		}
 
 		// Exclude leaf including $deleteafterid from tree
@@ -2601,10 +2606,10 @@ class User extends CommonObject
 	 *  Function called by get_full_tree().
 	 *
 	 * 	@param		int		$id_user		id_user entry to update
-	 * 	@param		int		$protection		Deep counter to avoid infinite loop
-	 *	@return		void
+	 * 	@param		int		$protection		Deep counter to avoid infinite loop (no more required, a protection is added with array useridfound)
+	 *	@return		int                     < 0 if KO (infinit loop), >= 0 if OK
 	 */
-	function build_path_from_id_user($id_user,$protection=1000)
+	function build_path_from_id_user($id_user,$protection=0)
 	{
 		dol_syslog(get_class($this)."::build_path_from_id_user id_user=".$id_user." protection=".$protection, LOG_DEBUG);
 
@@ -2612,7 +2617,7 @@ class User extends CommonObject
 		{
 			// Already defined
 			dol_syslog(get_class($this)."::build_path_from_id_user fullpath and fullname already defined", LOG_WARNING);
-			return;
+			return 0;
 		}
 
 		// Define fullpath and fullname
@@ -2620,9 +2625,16 @@ class User extends CommonObject
 		$this->users[$id_user]['fullname'] = $this->users[$id_user]['lastname'];
 		$i=0; $cursor_user=$id_user;
 
-		while ((empty($protection) || $i < $protection) && ! empty($this->parentof[$cursor_user]))
+		$useridfound=array($id_user);
+		while (! empty($this->parentof[$cursor_user]))
 		{
-			$this->users[$id_user]['fullpath'] = '_'.$this->parentof[$cursor_user].$this->users[$id_user]['fullpath'];
+			if (in_array($this->parentof[$cursor_user], $useridfound)) 
+			{
+				dol_syslog("The hierarchy of user has a recursive loop", LOG_WARNING);
+				return -1;     // Should not happen. Protection against looping hierarchy
+			}
+			$useridfound[]=$this->parentof[$cursor_user];
+		    $this->users[$id_user]['fullpath'] = '_'.$this->parentof[$cursor_user].$this->users[$id_user]['fullpath'];
 			$this->users[$id_user]['fullname'] = $this->users[$this->parentof[$cursor_user]]['lastname'].' >> '.$this->users[$id_user]['fullname'];
 			$i++; $cursor_user=$this->parentof[$cursor_user];
 		}
@@ -2630,7 +2642,7 @@ class User extends CommonObject
 		// We count number of _ to have level
 		$this->users[$id_user]['level']=dol_strlen(preg_replace('/[^_]/i','',$this->users[$id_user]['fullpath']));
 
-		return;
+		return 1;
 	}
 
 	/**

+ 9 - 1
htdocs/user/hierarchy.php

@@ -71,9 +71,15 @@ print load_fiche_titre($langs->trans("ListOfUsers"). ' ('.$langs->trans("Hierarc
 // Load hierarchy of users
 $user_arbo = $userstatic->get_full_tree(0, ($search_statut != '' && $search_statut >= 0) ? "statut = ".$search_statut : '');
 
+if (! is_array($user_arbo) && $user_arbo < 0)
+{
+    setEventMessages($userstatic->error, $userstatic->errors, 'warnings');    
+}
+else
+{
 // Define fulltree array
 $fulltree=$user_arbo;
-
+//var_dump($fulltree);
 // Define data (format for treeview)
 $data=array();
 $data[] = array('rowid'=>0,'fk_menu'=>-1,'title'=>"racine",'mainmenu'=>'','leftmenu'=>'','fk_mainmenu'=>'','fk_leftmenu'=>'');
@@ -131,6 +137,7 @@ foreach($fulltree as $key => $val)
 	);
 }
 
+//var_dump($data);
 
 print '<form method="POST" id="searchFormList" action="'.$_SERVER["PHP_SELF"].'">'."\n";
 
@@ -185,6 +192,7 @@ else
 
 print "</table>";
 print "</form>\n";
+}
 
 //
 /*print '<script type="text/javascript" language="javascript">