From 85e4c16f39a8b00b229644bcd175663541dfd51a Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 4 Feb 2013 20:37:25 -0500
Subject: Doc fixes and inline type hints for variables (big static analysis
 cleanup)

---
 engine/classes/ElggAutoP.php | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

(limited to 'engine/classes/ElggAutoP.php')

diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index 89d77e583..40600aa13 100644
--- a/engine/classes/ElggAutoP.php
+++ b/engine/classes/ElggAutoP.php
@@ -117,6 +117,8 @@ class ElggAutoP {
 		// serialize back to HTML
 		$html = $this->_doc->saveHTML();
 
+		// Note: we create <autop> elements, which will later be converted to paragraphs
+
 		// split AUTOPs into multiples at /\n\n+/
 		$html = preg_replace('/(' . $this->_unique . 'NL){2,}/', '</autop><autop>', $html);
 		$html = str_replace(array($this->_unique . 'BR', $this->_unique . 'NL', '<br>'), 
@@ -134,6 +136,7 @@ class ElggAutoP {
 
 		// strip AUTOPs that only have comments/whitespace
 		foreach ($this->_xpath->query('//autop') as $autop) {
+			/* @var DOMElement $autop */
 			$hasContent = false;
 			if (trim($autop->textContent) !== '') {
 				$hasContent = true;
@@ -146,13 +149,14 @@ class ElggAutoP {
 				}
 			}
 			if (!$hasContent) {
-				// strip w/ preg_replace later (faster than moving nodes out)
+				// mark to be later replaced w/ preg_replace (faster than moving nodes out)
 				$autop->setAttribute("r", "1");
 			}
 		}
 
-		// remove a single AUTOP inside certain elements
+		// If a DIV contains a single AUTOP, remove it
 		foreach ($this->_xpath->query('//div') as $el) {
+			/* @var DOMElement $el */
 			$autops = $this->_xpath->query('./autop', $el);
 			if ($autops->length === 1) {
 				// strip w/ preg_replace later (faster than moving nodes out)
@@ -185,7 +189,7 @@ class ElggAutoP {
 	 * @param DOMElement $el
 	 */
 	protected function _addParagraphs(DOMElement $el) {
-		// no need to recurse, just queue up
+		// no need to call recursively, just queue up
 		$elsToProcess = array($el);
 		$inlinesToProcess = array();
 		while ($el = array_shift($elsToProcess)) {
-- 
cgit v1.2.3


From 707df9db27cf2154910978427b9844448bcf931a Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Mon, 4 Feb 2013 20:51:07 -0500
Subject: Small changes to remove static analysis noise and simplify flow

---
 engine/classes/ElggAutoP.php           | 11 ++++++-----
 engine/classes/ElggData.php            |  1 +
 engine/classes/ElggDiskFilestore.php   |  2 --
 engine/classes/ElggFile.php            |  4 +++-
 engine/classes/ElggGroup.php           |  2 +-
 engine/classes/ElggMenuBuilder.php     |  1 +
 engine/classes/ElggRelationship.php    |  2 ++
 engine/lib/admin.php                   |  2 +-
 engine/lib/annotations.php             |  1 +
 engine/lib/cache.php                   |  6 +++---
 engine/lib/configuration.php           |  2 +-
 engine/lib/cron.php                    |  1 -
 engine/lib/elgglib.php                 |  1 +
 engine/lib/export.php                  | 17 +++++++++--------
 engine/lib/location.php                |  2 +-
 engine/lib/plugins.php                 | 16 +++++++++-------
 engine/lib/relationships.php           |  3 +--
 engine/lib/river.php                   |  1 +
 engine/lib/sessions.php                |  2 +-
 engine/lib/sites.php                   |  2 +-
 engine/lib/statistics.php              |  3 ++-
 engine/lib/system_log.php              |  6 +++---
 engine/lib/tags.php                    |  1 +
 engine/lib/upgrades/create_upgrade.php |  3 ++-
 engine/lib/user_settings.php           |  1 +
 engine/lib/users.php                   | 16 ++++++++--------
 engine/lib/views.php                   |  6 +-----
 27 files changed, 62 insertions(+), 53 deletions(-)

(limited to 'engine/classes/ElggAutoP.php')

diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index 40600aa13..f3c7cc972 100644
--- a/engine/classes/ElggAutoP.php
+++ b/engine/classes/ElggAutoP.php
@@ -159,8 +159,9 @@ class ElggAutoP {
 			/* @var DOMElement $el */
 			$autops = $this->_xpath->query('./autop', $el);
 			if ($autops->length === 1) {
-				// strip w/ preg_replace later (faster than moving nodes out)
-				$autops->item(0)->setAttribute("r", "1");
+				$firstAutop = $autops->item(0);
+				/* @var DOMElement $firstAutop */
+				$firstAutop->setAttribute("r", "1");
 			}
 		}
 
@@ -220,12 +221,12 @@ class ElggAutoP {
 
 				$isElement = ($node->nodeType === XML_ELEMENT_NODE);
 				if ($isElement) {
-					$elName = $node->nodeName;
+					$isBlock = in_array($node->nodeName, $this->_blocks);
+				} else {
+					$isBlock = false;
 				}
-				$isBlock = ($isElement && in_array($elName, $this->_blocks));
 
 				if ($alterInline) {
-					$isInline = $isElement && ! $isBlock;
 					$isText = ($node->nodeType === XML_TEXT_NODE);
 					$isLastInline = (! $node->nextSibling
 								   || ($node->nextSibling->nodeType === XML_ELEMENT_NODE
diff --git a/engine/classes/ElggData.php b/engine/classes/ElggData.php
index a0df3c924..426248ca3 100644
--- a/engine/classes/ElggData.php
+++ b/engine/classes/ElggData.php
@@ -273,6 +273,7 @@ abstract class ElggData implements
 		if (array_key_exists($key, $this->attributes)) {
 			return $this->attributes[$key];
 		}
+		return null;
 	}
 
 	/**
diff --git a/engine/classes/ElggDiskFilestore.php b/engine/classes/ElggDiskFilestore.php
index 5483eed81..2d86e7c2d 100644
--- a/engine/classes/ElggDiskFilestore.php
+++ b/engine/classes/ElggDiskFilestore.php
@@ -316,8 +316,6 @@ class ElggDiskFilestore extends ElggFilestore {
 		} else {
 			return str_split($string);
 		}
-
-		return false;
 	}
 
 	/**
diff --git a/engine/classes/ElggFile.php b/engine/classes/ElggFile.php
index 86406d108..3f652ff19 100644
--- a/engine/classes/ElggFile.php
+++ b/engine/classes/ElggFile.php
@@ -127,9 +127,11 @@ class ElggFile extends ElggObject {
 	 * @param mixed $default A default. Useful to pass what the browser thinks it is.
 	 * @since 1.7.12
 	 *
+	 * @note If $file is provided, this may be called statically
+	 *
 	 * @return mixed Detected type on success, false on failure.
 	 */
-	static function detectMimeType($file = null, $default = null) {
+	public function detectMimeType($file = null, $default = null) {
 		if (!$file) {
 			if (isset($this) && $this->filename) {
 				$file = $this->filename;
diff --git a/engine/classes/ElggGroup.php b/engine/classes/ElggGroup.php
index d17fa4f1a..24bdee79e 100644
--- a/engine/classes/ElggGroup.php
+++ b/engine/classes/ElggGroup.php
@@ -284,7 +284,7 @@ class ElggGroup extends ElggEntity
 	 *
 	 * @return bool
 	 */
-	public function isMember($user = 0) {
+	public function isMember($user = null) {
 		if (!($user instanceof ElggUser)) {
 			$user = elgg_get_logged_in_user_entity();
 		}
diff --git a/engine/classes/ElggMenuBuilder.php b/engine/classes/ElggMenuBuilder.php
index 2fd5ad9c4..028eef15f 100644
--- a/engine/classes/ElggMenuBuilder.php
+++ b/engine/classes/ElggMenuBuilder.php
@@ -122,6 +122,7 @@ class ElggMenuBuilder {
 			// attach children to parents
 			$iteration = 0;
 			$current_gen = $parents;
+			$next_gen = null;
 			while (count($children) && $iteration < 5) {
 				foreach ($children as $index => $menu_item) {
 					$parent_name = $menu_item->getParentName();
diff --git a/engine/classes/ElggRelationship.php b/engine/classes/ElggRelationship.php
index 377a41093..d2e88882a 100644
--- a/engine/classes/ElggRelationship.php
+++ b/engine/classes/ElggRelationship.php
@@ -180,6 +180,8 @@ class ElggRelationship extends ElggData implements
 				return true;
 			}
 		}
+
+		return false;
 	}
 
 	// SYSTEM LOG INTERFACE ////////////////////////////////////////////////////////////
diff --git a/engine/lib/admin.php b/engine/lib/admin.php
index 3677a3b69..ec19a5476 100644
--- a/engine/lib/admin.php
+++ b/engine/lib/admin.php
@@ -346,7 +346,7 @@ function elgg_admin_add_plugin_settings_menu() {
 	$active_plugins = elgg_get_plugins('active');
 	if (!$active_plugins) {
 		// nothing added because no items
-		return FALSE;
+		return;
 	}
 
 	foreach ($active_plugins as $plugin) {
diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index 9ba0491b2..3d94a1d55 100644
--- a/engine/lib/annotations.php
+++ b/engine/lib/annotations.php
@@ -542,6 +542,7 @@ function elgg_comment_url_handler(ElggAnnotation $comment) {
 	if ($entity) {
 		return $entity->getURL() . '#item-annotation-' . $comment->id;
 	}
+	return "";
 }
 
 /**
diff --git a/engine/lib/cache.php b/engine/lib/cache.php
index 5c917bb18..74644019c 100644
--- a/engine/lib/cache.php
+++ b/engine/lib/cache.php
@@ -125,7 +125,7 @@ function elgg_get_filepath_cache() {
  * @access private
  */
 function elgg_filepath_cache_reset() {
-	return elgg_reset_system_cache();
+	elgg_reset_system_cache();
 }
 /**
  * @access private
@@ -143,13 +143,13 @@ function elgg_filepath_cache_load($type) {
  * @access private
  */
 function elgg_enable_filepath_cache() {
-	return elgg_enable_system_cache();
+	elgg_enable_system_cache();
 }
 /**
  * @access private
  */
 function elgg_disable_filepath_cache() {
-	return elgg_disable_system_cache();
+	elgg_disable_system_cache();
 }
 
 /* Simplecache */
diff --git a/engine/lib/configuration.php b/engine/lib/configuration.php
index 851430127..a0f297f0c 100644
--- a/engine/lib/configuration.php
+++ b/engine/lib/configuration.php
@@ -182,7 +182,7 @@ function verify_installation() {
 	global $CONFIG;
 
 	if (isset($CONFIG->installed)) {
-		return $CONFIG->installed;
+		return;
 	}
 
 	try {
diff --git a/engine/lib/cron.php b/engine/lib/cron.php
index f2939bdd6..dd83a9849 100644
--- a/engine/lib/cron.php
+++ b/engine/lib/cron.php
@@ -52,7 +52,6 @@ function cron_page_handler($page) {
 	$params['time'] = time();
 
 	// Data to return to
-	$std_out = "";
 	$old_stdout = "";
 	ob_start();
 
diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php
index 746fd8aa9..6a2ebf97b 100644
--- a/engine/lib/elgglib.php
+++ b/engine/lib/elgglib.php
@@ -1903,6 +1903,7 @@ function elgg_cacheable_view_page_handler($page, $type) {
 		echo $return;
 		return true;
 	}
+	return false;
 }
 
 /**
diff --git a/engine/lib/export.php b/engine/lib/export.php
index 31afc1b74..1996bc885 100644
--- a/engine/lib/export.php
+++ b/engine/lib/export.php
@@ -117,18 +117,19 @@ function _process_element(ODD $odd) {
 	global $IMPORTED_DATA, $IMPORTED_OBJECT_COUNTER;
 
 	// See if anyone handles this element, return true if it is.
+	$to_be_serialised = null;
 	if ($odd) {
 		$handled = elgg_trigger_plugin_hook("import", "all", array("element" => $odd), $to_be_serialised);
-	}
 
-	// If not, then see if any of its sub elements are handled
-	if ($handled) {
-		// Increment validation counter
-		$IMPORTED_OBJECT_COUNTER ++;
-		// Return the constructed object
-		$IMPORTED_DATA[] = $handled;
+		// If not, then see if any of its sub elements are handled
+		if ($handled) {
+			// Increment validation counter
+			$IMPORTED_OBJECT_COUNTER ++;
+			// Return the constructed object
+			$IMPORTED_DATA[] = $handled;
 
-		return true;
+			return true;
+		}
 	}
 
 	return false;
diff --git a/engine/lib/location.php b/engine/lib/location.php
index 5b889509b..b319bb3bb 100644
--- a/engine/lib/location.php
+++ b/engine/lib/location.php
@@ -101,7 +101,7 @@ function elgg_get_entities_from_location(array $options = array()) {
 	$long_min = $long - $long_distance;
 	$long_max = $long + $long_distance;
 
-	$where = array();
+	$wheres = array();
 	$wheres[] = "lat_name.string='geo:lat'";
 	$wheres[] = "lat_value.string >= $lat_min";
 	$wheres[] = "lat_value.string <= $lat_max";
diff --git a/engine/lib/plugins.php b/engine/lib/plugins.php
index 3a42cb9b8..973ca4026 100644
--- a/engine/lib/plugins.php
+++ b/engine/lib/plugins.php
@@ -261,6 +261,8 @@ function elgg_get_max_plugin_priority() {
 	$data = get_data($q);
 	if ($data) {
 		$max = $data[0]->max;
+	} else {
+		$max = 1;
 	}
 
 	// can't have a priority of 0.
@@ -442,6 +444,7 @@ function elgg_set_plugin_priorities(array $order) {
 	// though we do start with 1
 	$order = array_values($order);
 
+	$missing_plugins = array();
 	foreach ($plugins as $plugin) {
 		$plugin_id = $plugin->getID();
 
@@ -640,19 +643,18 @@ function elgg_get_plugins_provides($type = null, $name = null) {
  * @access private
  */
 function elgg_check_plugins_provides($type, $name, $version = null, $comparison = 'ge') {
-	if (!$provided = elgg_get_plugins_provides($type, $name)) {
+	$provided = elgg_get_plugins_provides($type, $name);
+	if (!$provided) {
 		return array(
 			'status' => false,
 			'version' => ''
 		);
 	}
 
-	if ($provided) {
-		if ($version) {
-			$status = version_compare($provided['version'], $version, $comparison);
-		} else {
-			$status = true;
-		}
+	if ($version) {
+		$status = version_compare($provided['version'], $version, $comparison);
+	} else {
+		$status = true;
 	}
 
 	return array(
diff --git a/engine/lib/relationships.php b/engine/lib/relationships.php
index 74954b4ff..6f18bda93 100644
--- a/engine/lib/relationships.php
+++ b/engine/lib/relationships.php
@@ -571,9 +571,8 @@ function import_relationship_plugin_hook($hook, $entity_type, $returnvalue, $par
 	if ($element instanceof ODDRelationship) {
 		$tmp = new ElggRelationship();
 		$tmp->import($element);
-
-		return $tmp;
 	}
+	return $tmp;
 }
 
 /**
diff --git a/engine/lib/river.php b/engine/lib/river.php
index 148fee051..6274887b5 100644
--- a/engine/lib/river.php
+++ b/engine/lib/river.php
@@ -500,6 +500,7 @@ function elgg_get_river_type_subtype_where_sql($table, $types, $subtypes, $pairs
 		return '';
 	}
 
+	$wheres = array();
 	$types_wheres = array();
 	$subtypes_wheres = array();
 
diff --git a/engine/lib/sessions.php b/engine/lib/sessions.php
index 72ca0a1c2..eb451f05a 100644
--- a/engine/lib/sessions.php
+++ b/engine/lib/sessions.php
@@ -616,7 +616,7 @@ function _elgg_session_destroy($id) {
 		global $sess_save_path;
 
 		$sess_file = "$sess_save_path/sess_$id";
-		return(@unlink($sess_file));
+		return (@unlink($sess_file));
 	}
 
 	return false;
diff --git a/engine/lib/sites.php b/engine/lib/sites.php
index fe9af8c7a..8ee3213d3 100644
--- a/engine/lib/sites.php
+++ b/engine/lib/sites.php
@@ -26,7 +26,7 @@ function elgg_get_site_entity($site_guid = 0) {
 		$site = get_entity($site_guid);
 	}
 	
-	if($site instanceof ElggSite){
+	if ($site instanceof ElggSite) {
 		$result = $site;
 	}
 
diff --git a/engine/lib/statistics.php b/engine/lib/statistics.php
index 5ee640549..0c9a3c945 100644
--- a/engine/lib/statistics.php
+++ b/engine/lib/statistics.php
@@ -101,9 +101,10 @@ function get_online_users() {
 	if ($objects) {
 		return elgg_view_entity_list($objects, array(
 			'count' => $count,
-			'limit' => 10
+			'limit' => 10,
 		));
 	}
+	return '';
 }
 
 /**
diff --git a/engine/lib/system_log.php b/engine/lib/system_log.php
index 8149d3fac..38bc4ba96 100644
--- a/engine/lib/system_log.php
+++ b/engine/lib/system_log.php
@@ -25,9 +25,9 @@
  * @param string    $ip_address The IP address.
  * @return mixed
  */
-function get_system_log($by_user = "", $event = "", $class = "", $type = "", $subtype = "",
-$limit = 10, $offset = 0, $count = false, $timebefore = 0, $timeafter = 0, $object_id = 0,
-$ip_address = false) {
+function get_system_log($by_user = "", $event = "", $class = "", $type = "", $subtype = "", $limit = 10,
+						$offset = 0, $count = false, $timebefore = 0, $timeafter = 0, $object_id = 0,
+						$ip_address = "") {
 
 	global $CONFIG;
 
diff --git a/engine/lib/tags.php b/engine/lib/tags.php
index a25bd7d33..586a9b9e4 100644
--- a/engine/lib/tags.php
+++ b/engine/lib/tags.php
@@ -172,6 +172,7 @@ function elgg_get_tags(array $options = array()) {
 	// catch for tags that were spaces
 	$wheres[] = "msv.string != ''";
 
+	$sanitised_tags = array();
 	foreach ($options['tag_names'] as $tag) {
 		$sanitised_tags[] = '"' . sanitise_string($tag) . '"';
 	}
diff --git a/engine/lib/upgrades/create_upgrade.php b/engine/lib/upgrades/create_upgrade.php
index 3652e18a2..f0fa9b594 100644
--- a/engine/lib/upgrades/create_upgrade.php
+++ b/engine/lib/upgrades/create_upgrade.php
@@ -93,7 +93,7 @@ if (!$h) {
 	die("Could not open file $upgrade_file");
 }
 
-if (!fputs($h, $upgrade_code)) {
+if (!fwrite($h, $upgrade_code)) {
 	die("Could not write to $upgrade_file");
 } else {
 	elgg_set_version_dot_php_version($upgrade_version);
@@ -130,6 +130,7 @@ function elgg_set_version_dot_php_version($version) {
 
 	fputs($h, $out);
 	fclose($h);
+	return true;
 }
 
 /**
diff --git a/engine/lib/user_settings.php b/engine/lib/user_settings.php
index e4069fb53..cca5359a4 100644
--- a/engine/lib/user_settings.php
+++ b/engine/lib/user_settings.php
@@ -332,6 +332,7 @@ function usersettings_page_handler($page) {
 		require $path;
 		return true;
 	}
+	return false;
 }
 
 /**
diff --git a/engine/lib/users.php b/engine/lib/users.php
index 22ce08e10..7a6245261 100644
--- a/engine/lib/users.php
+++ b/engine/lib/users.php
@@ -560,7 +560,7 @@ function get_user_by_username($username) {
 
 	// Caching
 	if ((isset($USERNAME_TO_GUID_MAP_CACHE[$username]))
-	&& (retrieve_cached_entity($USERNAME_TO_GUID_MAP_CACHE[$username]))) {
+			&& (retrieve_cached_entity($USERNAME_TO_GUID_MAP_CACHE[$username]))) {
 		return retrieve_cached_entity($USERNAME_TO_GUID_MAP_CACHE[$username]);
 	}
 
@@ -687,15 +687,14 @@ function send_new_password_request($user_guid) {
 		$code = generate_random_cleartext_password();
 		$user->setPrivateSetting('passwd_conf_code', $code);
 
-
 		// generate link
-		$link = $CONFIG->site->url . "resetpassword?u=$user_guid&c=$code";
+		$link = elgg_get_site_url() . "resetpassword?u=$user_guid&c=$code";
 
 		// generate email
 		$email = elgg_echo('email:resetreq:body', array($user->name, $_SERVER['REMOTE_ADDR'], $link));
 
-		return notify_user($user->guid, $CONFIG->site->guid,
-			elgg_echo('email:resetreq:subject'), $email, NULL, 'email');
+		return notify_user($user->guid, elgg_get_site_entity()->guid,
+			elgg_echo('email:resetreq:subject'), $email, array(), 'email');
 	}
 
 	return false;
@@ -760,7 +759,7 @@ function execute_new_password_request($user_guid, $conf_code) {
 				$email = elgg_echo('email:resetpassword:body', array($user->name, $password));
 
 				return notify_user($user->guid, $CONFIG->site->guid,
-					elgg_echo('email:resetpassword:subject'), $email, NULL, 'email');
+					elgg_echo('email:resetpassword:subject'), $email, array(), 'email');
 			}
 		}
 	}
@@ -1036,7 +1035,7 @@ function elgg_get_user_validation_status($user_guid) {
 		'metadata_name' => 'validated'
 	));
 	if ($md == false) {
-		return;
+		return null;
 	}
 
 	if ($md[0]->value) {
@@ -1208,7 +1207,8 @@ function set_last_login($user_guid) {
 function user_create_hook_add_site_relationship($event, $object_type, $object) {
 	global $CONFIG;
 
-	add_entity_relationship($object->getGUID(), 'member_of_site', $CONFIG->site->getGUID());
+	add_entity_relationship($object->getGUID(), 'member_of_site', elgg_get_site_entity()->guid);
+	return true;
 }
 
 /**
diff --git a/engine/lib/views.php b/engine/lib/views.php
index 9ee7ae00c..7d8347863 100644
--- a/engine/lib/views.php
+++ b/engine/lib/views.php
@@ -1451,17 +1451,13 @@ function elgg_get_views($dir, $base) {
  */
 function elgg_view_tree($view_root, $viewtype = "") {
 	global $CONFIG;
-	static $treecache;
+	static $treecache = array();
 
 	// Get viewtype
 	if (!$viewtype) {
 		$viewtype = elgg_get_viewtype();
 	}
 
-	// Has the treecache been initialised?
-	if (!isset($treecache)) {
-		$treecache = array();
-	}
 	// A little light internal caching
 	if (!empty($treecache[$view_root])) {
 		return $treecache[$view_root];
-- 
cgit v1.2.3


From 00819122111a081c17f1ae4c53974b0deb50757c Mon Sep 17 00:00:00 2001
From: cash <cash.costello@gmail.com>
Date: Sat, 16 Mar 2013 12:41:16 -0400
Subject: more coding standard fixes

---
 engine/classes/ElggAttributeLoader.php       | 30 ++++++++-
 engine/classes/ElggAutoP.php                 | 24 +++++---
 engine/classes/ElggDiskFilestore.php         |  9 +++
 engine/classes/ElggVolatileMetadataCache.php | 92 +++++++++++++++-------------
 engine/classes/ElggXMLElement.php            |  4 ++
 engine/lib/opendd.php                        |  4 ++
 6 files changed, 110 insertions(+), 53 deletions(-)

(limited to 'engine/classes/ElggAutoP.php')

diff --git a/engine/classes/ElggAttributeLoader.php b/engine/classes/ElggAttributeLoader.php
index 2d1c1abde..d1e15008e 100644
--- a/engine/classes/ElggAttributeLoader.php
+++ b/engine/classes/ElggAttributeLoader.php
@@ -4,6 +4,9 @@
  * Loads ElggEntity attributes from DB or validates those passed in via constructor
  *
  * @access private
+ * 
+ * @package    Elgg.Core
+ * @subpackage DataModel
  */
 class ElggAttributeLoader {
 
@@ -65,9 +68,11 @@ class ElggAttributeLoader {
 	public $full_loader = '';
 
 	/**
-	 * @param string $class class of object being loaded
-	 * @param string $required_type entity type this is being used to populate
-	 * @param array $initialized_attrs attributes after initializeAttributes() has been run
+	 * Constructor
+	 * 
+	 * @param string $class             class of object being loaded
+	 * @param string $required_type     entity type this is being used to populate
+	 * @param array  $initialized_attrs attributes after initializeAttributes() has been run
 	 * @throws InvalidArgumentException
 	 */
 	public function __construct($class, $required_type, array $initialized_attrs) {
@@ -87,14 +92,33 @@ class ElggAttributeLoader {
 		$this->secondary_attr_names = array_diff($all_attr_names, self::$primary_attr_names);
 	}
 
+	/**
+	 * Get primary attributes missing that are missing
+	 * 
+	 * @param stdClass $row Database row
+	 * @return array
+	 */
 	protected function isMissingPrimaries($row) {
 		return array_diff(self::$primary_attr_names, array_keys($row)) !== array();
 	}
 
+	/**
+	 * Get secondary attributes that are missing
+	 * 
+	 * @param stdClass $row Database row
+	 * @return array
+	 */
 	protected function isMissingSecondaries($row) {
 		return array_diff($this->secondary_attr_names, array_keys($row)) !== array();
 	}
 
+	/**
+	 * Check that the type is correct
+	 * 
+	 * @param stdClass $row Database row
+	 * @return void
+	 * @throws InvalidClassException
+	 */
 	protected function checkType($row) {
 		if ($row['type'] !== $this->required_type) {
 			$msg = elgg_echo('InvalidClassException:NotValidElggStar', array($row['guid'], $this->class));
diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index f3c7cc972..71536c433 100644
--- a/engine/classes/ElggAutoP.php
+++ b/engine/classes/ElggAutoP.php
@@ -7,6 +7,9 @@
  *
  * In DIV elements, Ps are only added when there would be at
  * least two of them.
+ * 
+ * @package    Elgg.Core
+ * @subpackage Output
  */
 class ElggAutoP {
 
@@ -51,8 +54,12 @@ class ElggAutoP {
 	protected $_alterList = 'article aside blockquote body details div footer header
 		section';
 
+	/** @var string */
 	protected $_unique = '';
 
+	/**
+	 * Constructor
+	 */
 	public function __construct() {
 		$this->_blocks = preg_split('@\\s+@', $this->_blocks);
 		$this->_descendList = preg_split('@\\s+@', $this->_descendList);
@@ -98,7 +105,7 @@ class ElggAutoP {
 		$html = str_replace('&', $this->_unique . 'AMP', $html);
 
 		$this->_doc = new DOMDocument();
-	   
+
 		// parse to DOM, suppressing loadHTML warnings
 		// http://www.php.net/manual/en/domdocument.loadhtml.php#95463
 		libxml_use_internal_errors(true);
@@ -112,7 +119,7 @@ class ElggAutoP {
 		$this->_xpath = new DOMXPath($this->_doc);
 		// start processing recursively at the BODY element
 		$nodeList = $this->_xpath->query('//body[1]');
-		$this->_addParagraphs($nodeList->item(0));
+		$this->addParagraphs($nodeList->item(0));
 
 		// serialize back to HTML
 		$html = $this->_doc->saveHTML();
@@ -187,15 +194,16 @@ class ElggAutoP {
 	/**
 	 * Add P and BR elements as necessary
 	 *
-	 * @param DOMElement $el
+	 * @param DOMElement $el DOM element
+	 * @return void
 	 */
-	protected function _addParagraphs(DOMElement $el) {
+	protected function addParagraphs(DOMElement $el) {
 		// no need to call recursively, just queue up
 		$elsToProcess = array($el);
 		$inlinesToProcess = array();
 		while ($el = array_shift($elsToProcess)) {
 			// if true, we can alter all child nodes, if not, we'll just call
-			// _addParagraphs on each element in the descendInto list
+			// addParagraphs on each element in the descendInto list
 			$alterInline = in_array($el->nodeName, $this->_alterList);
 
 			// inside affected elements, we want to trim leading whitespace from
@@ -229,8 +237,8 @@ class ElggAutoP {
 				if ($alterInline) {
 					$isText = ($node->nodeType === XML_TEXT_NODE);
 					$isLastInline = (! $node->nextSibling
-								   || ($node->nextSibling->nodeType === XML_ELEMENT_NODE
-									   && in_array($node->nextSibling->nodeName, $this->_blocks)));
+							|| ($node->nextSibling->nodeType === XML_ELEMENT_NODE
+								&& in_array($node->nextSibling->nodeName, $this->_blocks)));
 					if ($isElement) {
 						$isFollowingBr = ($node->nodeName === 'br');
 					}
@@ -263,7 +271,7 @@ class ElggAutoP {
 					if ($isBlock) {
 						if (in_array($node->nodeName, $this->_descendList)) {
 							$elsToProcess[] = $node;
-							//$this->_addParagraphs($node);
+							//$this->addParagraphs($node);
 						}
 					}
 					$openP = true;
diff --git a/engine/classes/ElggDiskFilestore.php b/engine/classes/ElggDiskFilestore.php
index 7374aad35..29547d83b 100644
--- a/engine/classes/ElggDiskFilestore.php
+++ b/engine/classes/ElggDiskFilestore.php
@@ -254,6 +254,7 @@ class ElggDiskFilestore extends ElggFilestore {
 		}
 	}
 
+	// @codingStandardsIgnoreStart
 	/**
 	 * Create a directory $dirroot
 	 *
@@ -268,6 +269,7 @@ class ElggDiskFilestore extends ElggFilestore {
 
 		return $this->makeDirectoryRoot($dirroot);
 	}
+	// @codingStandardsIgnoreEnd
 
 	/**
 	 * Create a directory $dirroot
@@ -287,6 +289,7 @@ class ElggDiskFilestore extends ElggFilestore {
 		return true;
 	}
 
+	// @codingStandardsIgnoreStart
 	/**
 	 * Multibyte string tokeniser.
 	 *
@@ -318,7 +321,9 @@ class ElggDiskFilestore extends ElggFilestore {
 			return str_split($string);
 		}
 	}
+	// @codingStandardsIgnoreEnd
 
+	// @codingStandardsIgnoreStart
 	/**
 	 * Construct a file path matrix for an entity.
 	 *
@@ -332,6 +337,7 @@ class ElggDiskFilestore extends ElggFilestore {
 
 		return $this->makefileMatrix($identifier);
 	}
+	// @codingStandardsIgnoreEnd
 
 	/**
 	 * Construct a file path matrix for an entity.
@@ -351,7 +357,9 @@ class ElggDiskFilestore extends ElggFilestore {
 
 		return "$time_created/$entity->guid/";
 	}
+	// @codingStandardsIgnoreEnd
 
+	// @codingStandardsIgnoreStart
 	/**
 	 * Construct a filename matrix.
 	 *
@@ -370,6 +378,7 @@ class ElggDiskFilestore extends ElggFilestore {
 
 		return $this->makeFileMatrix($guid);
 	}
+	// @codingStandardsIgnoreEnd
 
 	/**
 	 * Returns a list of attributes to save to the database when saving
diff --git a/engine/classes/ElggVolatileMetadataCache.php b/engine/classes/ElggVolatileMetadataCache.php
index 8a33c198d..4acda7cee 100644
--- a/engine/classes/ElggVolatileMetadataCache.php
+++ b/engine/classes/ElggVolatileMetadataCache.php
@@ -33,9 +33,11 @@ class ElggVolatileMetadataCache {
 	protected $ignoreAccess = null;
 
 	/**
-	 * @param int $entity_guid
-	 *
-	 * @param array $values
+	 * Cache metadata for an entity
+	 * 
+	 * @param int   $entity_guid The GUID of the entity
+	 * @param array $values      The metadata values to cache
+	 * @return void
 	 */
 	public function saveAll($entity_guid, array $values) {
 		if (!$this->getIgnoreAccess()) {
@@ -45,8 +47,9 @@ class ElggVolatileMetadataCache {
 	}
 
 	/**
-	 * @param int $entity_guid
-	 *
+	 * Get the metadata for an entity
+	 * 
+	 * @param int $entity_guid The GUID of the entity
 	 * @return array
 	 */
 	public function loadAll($entity_guid) {
@@ -61,15 +64,17 @@ class ElggVolatileMetadataCache {
 	 * Declare that there may be fetch-able metadata names in storage that this
 	 * cache doesn't know about
 	 *
-	 * @param int $entity_guid
+	 * @param int $entity_guid The GUID of the entity
+	 * @return void
 	 */
 	public function markOutOfSync($entity_guid) {
 		unset($this->isSynchronized[$entity_guid]);
 	}
 
 	/**
-	 * @param $entity_guid
-	 *
+	 * Have all the metadata for this entity been cached?
+	 * 
+	 * @param int $entity_guid The GUID of the entity
 	 * @return bool
 	 */
 	public function isSynchronized($entity_guid) {
@@ -77,13 +82,15 @@ class ElggVolatileMetadataCache {
 	}
 
 	/**
-	 * @param int $entity_guid
-	 *
-	 * @param string $name
-	 *
-	 * @param array|int|string|null $value  null means it is known that there is no
-	 *                                      fetch-able metadata under this name
-	 * @param bool $allow_multiple
+	 * Cache a piece of metadata
+	 * 
+	 * @param int                   $entity_guid    The GUID of the entity
+	 * @param string                $name           The metadata name
+	 * @param array|int|string|null $value          The metadata value. null means it is 
+	 *                                              known that there is no fetch-able 
+	 *                                              metadata under this name
+	 * @param bool                  $allow_multiple Can the metadata be an array
+	 * @return void
 	 */
 	public function save($entity_guid, $name, $value, $allow_multiple = false) {
 		if ($this->getIgnoreAccess()) {
@@ -115,10 +122,8 @@ class ElggVolatileMetadataCache {
 	 * function's return value should be trusted (otherwise a null return value
 	 * is ambiguous).
 	 *
-	 * @param int $entity_guid
-	 *
-	 * @param string $name
-	 *
+	 * @param int    $entity_guid The GUID of the entity
+	 * @param string $name        The metadata name
 	 * @return array|string|int|null null = value does not exist
 	 */
 	public function load($entity_guid, $name) {
@@ -133,9 +138,9 @@ class ElggVolatileMetadataCache {
 	 * Forget about this metadata entry. We don't want to try to guess what the
 	 * next fetch from storage will return
 	 *
-	 * @param int $entity_guid
-	 *
-	 * @param string $name
+	 * @param int    $entity_guid The GUID of the entity
+	 * @param string $name        The metadata name
+	 * @return void
 	 */
 	public function markUnknown($entity_guid, $name) {
 		unset($this->values[$entity_guid][$name]);
@@ -145,10 +150,8 @@ class ElggVolatileMetadataCache {
 	/**
 	 * If true, load() will return an accurate value for this name
 	 *
-	 * @param int $entity_guid
-	 *
-	 * @param string $name
-	 *
+	 * @param int    $entity_guid The GUID of the entity
+	 * @param string $name        The metadata name
 	 * @return bool
 	 */
 	public function isKnown($entity_guid, $name) {
@@ -163,10 +166,8 @@ class ElggVolatileMetadataCache {
 	/**
 	 * Declare that metadata under this name is known to be not fetch-able from storage
 	 *
-	 * @param int $entity_guid
-	 *
-	 * @param string $name
-	 *
+	 * @param int    $entity_guid The GUID of the entity
+	 * @param string $name        The metadata name
 	 * @return array
 	 */
 	public function markEmpty($entity_guid, $name) {
@@ -176,7 +177,8 @@ class ElggVolatileMetadataCache {
 	/**
 	 * Forget about all metadata for an entity
 	 *
-	 * @param int $entity_guid
+	 * @param int $entity_guid The GUID of the entity
+	 * @return void
 	 */
 	public function clear($entity_guid) {
 		$this->values[$entity_guid] = array();
@@ -185,6 +187,8 @@ class ElggVolatileMetadataCache {
 
 	/**
 	 * Clear entire cache and mark all entities as out of sync
+	 * 
+	 * @return void
 	 */
 	public function flush() {
 		$this->values = array();
@@ -197,7 +201,8 @@ class ElggVolatileMetadataCache {
 	 *
 	 * This setting makes this component a little more loosely-coupled.
 	 *
-	 * @param bool $ignore
+	 * @param bool $ignore Whether to ignore access or not
+	 * @return void
 	 */
 	public function setIgnoreAccess($ignore) {
 		$this->ignoreAccess = (bool) $ignore;
@@ -205,12 +210,16 @@ class ElggVolatileMetadataCache {
 
 	/**
 	 * Tell the cache to call elgg_get_ignore_access() to determing access status.
+	 * 
+	 * @return void
 	 */
 	public function unsetIgnoreAccess() {
 		$this->ignoreAccess = null;
 	}
 
 	/**
+	 * Get the ignore access value
+	 * 
 	 * @return bool
 	 */
 	protected function getIgnoreAccess() {
@@ -225,12 +234,10 @@ class ElggVolatileMetadataCache {
 	 * Invalidate based on options passed to the global *_metadata functions
 	 *
 	 * @param string $action  Action performed on metadata. "delete", "disable", or "enable"
-	 *
-	 * @param array $options  Options passed to elgg_(delete|disable|enable)_metadata
-	 *
-	 *   "guid" if given, invalidation will be limited to this entity
-	 *
-	 *   "metadata_name" if given, invalidation will be limited to metadata with this name
+	 * @param array  $options Options passed to elgg_(delete|disable|enable)_metadata
+	 *                         "guid" if given, invalidation will be limited to this entity
+	 *                         "metadata_name" if given, invalidation will be limited to metadata with this name
+	 * @return void
 	 */
 	public function invalidateByOptions($action, array $options) {
 		// remove as little as possible, optimizing for common cases
@@ -254,7 +261,10 @@ class ElggVolatileMetadataCache {
 	}
 
 	/**
-	 * @param int|array $guids
+	 * Populate the cache from a set of entities
+	 * 
+	 * @param int|array $guids Array of or single GUIDs
+	 * @return void
 	 */
 	public function populateFromEntities($guids) {
 		if (empty($guids)) {
@@ -318,9 +328,7 @@ class ElggVolatileMetadataCache {
 	 * cache if RAM usage becomes an issue.
 	 *
 	 * @param array $guids GUIDs of entities to examine
-	 *
-	 * @param int $limit Limit in characters of all metadata (with ints casted to strings)
-	 *
+	 * @param int   $limit Limit in characters of all metadata (with ints casted to strings)
 	 * @return array
 	 */
 	public function filterMetadataHeavyEntities(array $guids, $limit = 1024000) {
diff --git a/engine/classes/ElggXMLElement.php b/engine/classes/ElggXMLElement.php
index d7e912035..6f2633e25 100644
--- a/engine/classes/ElggXMLElement.php
+++ b/engine/classes/ElggXMLElement.php
@@ -77,6 +77,8 @@ class ElggXMLElement {
 	}
 
 	/**
+	 * Override ->
+	 * 
 	 * @param string $name Property name
 	 * @return mixed
 	 */
@@ -99,6 +101,8 @@ class ElggXMLElement {
 	}
 
 	/**
+	 * Override isset
+	 * 
 	 * @param string $name Property name
 	 * @return boolean
 	 */
diff --git a/engine/lib/opendd.php b/engine/lib/opendd.php
index f00ea6aab..7d635a295 100644
--- a/engine/lib/opendd.php
+++ b/engine/lib/opendd.php
@@ -7,6 +7,8 @@
  * @version 0.4
  */
 
+// @codingStandardsIgnoreStart
+
 /**
  * Attempt to construct an ODD object out of a XmlElement or sub-elements.
  *
@@ -103,3 +105,5 @@ function ODD_Import($xml) {
 function ODD_Export(ODDDocument $document) {
 	return "$document";
 }
+
+// @codingStandardsIgnoreEnd
-- 
cgit v1.2.3


From d53447f7e6b3277f3249d9a70e56ec01a90c3a60 Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Thu, 11 Jul 2013 13:24:01 -0400
Subject: Disable loading external entities during XML parsing

---
 engine/classes/ElggAutoP.php                    | 14 ++++++++++++++
 engine/classes/ElggXMLElement.php               |  8 ++++++--
 engine/tests/regression/trac_bugs.php           | 10 ++++++++++
 engine/tests/test_files/xxe/external_entity.txt |  1 +
 engine/tests/test_files/xxe/request.xml         |  8 ++++++++
 5 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 engine/tests/test_files/xxe/external_entity.txt
 create mode 100644 engine/tests/test_files/xxe/request.xml

(limited to 'engine/classes/ElggAutoP.php')

diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index 71536c433..05842d1b2 100644
--- a/engine/classes/ElggAutoP.php
+++ b/engine/classes/ElggAutoP.php
@@ -110,12 +110,19 @@ class ElggAutoP {
 		// http://www.php.net/manual/en/domdocument.loadhtml.php#95463
 		libxml_use_internal_errors(true);
 
+		// Do not load entities. May be unnecessary, better safe than sorry
+		$disable_load_entities = libxml_disable_entity_loader(true);
+
 		if (!$this->_doc->loadHTML("<html><meta http-equiv='content-type' " 
 				. "content='text/html; charset={$this->encoding}'><body>{$html}</body>"
 				. "</html>")) {
+
+			libxml_disable_entity_loader($disable_load_entities);
 			return false;
 		}
 
+		libxml_disable_entity_loader($disable_load_entities);
+
 		$this->_xpath = new DOMXPath($this->_doc);
 		// start processing recursively at the BODY element
 		$nodeList = $this->_xpath->query('//body[1]');
@@ -135,9 +142,16 @@ class ElggAutoP {
 
 		// re-parse so we can handle new AUTOP elements
 
+		// Do not load entities. May be unnecessary, better safe than sorry
+		$disable_load_entities = libxml_disable_entity_loader(true);
+
 		if (!$this->_doc->loadHTML($html)) {
+			libxml_disable_entity_loader($disable_load_entities);
 			return false;
 		}
+
+		libxml_disable_entity_loader($disable_load_entities);
+
 		// must re-create XPath object after DOM load
 		$this->_xpath = new DOMXPath($this->_doc);
 
diff --git a/engine/classes/ElggXMLElement.php b/engine/classes/ElggXMLElement.php
index 6f2633e25..cbd3fc5ce 100644
--- a/engine/classes/ElggXMLElement.php
+++ b/engine/classes/ElggXMLElement.php
@@ -20,7 +20,12 @@ class ElggXMLElement {
 		if ($xml instanceof SimpleXMLElement) {
 			$this->_element = $xml;
 		} else {
+			// do not load entities
+			$disable_load_entities = libxml_disable_entity_loader(true);
+
 			$this->_element = new SimpleXMLElement($xml);
+
+			libxml_disable_entity_loader($disable_load_entities);
 		}
 	}
 
@@ -123,5 +128,4 @@ class ElggXMLElement {
 		}
 		return false;
 	}
-
-}
\ No newline at end of file
+}
diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index ef1348cf6..e6773c8af 100644
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -373,4 +373,14 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
 		//delete group and annotations
 		$group->delete();
 	}
+
+	public function test_ElggXMLElement_does_not_load_external_entities() {
+		$payload = file_get_contents(dirname(dirname(__FILE__)) . '/test_files/xxe/request.xml');
+		$payload = sprintf($payload, 'file://' . realpath(dirname(dirname(__FILE__)) . '/test_files/xxe/external_entity.txt'));
+
+		$el = new ElggXMLElement($payload);
+		$chidren = $el->getChildren();
+		$content = $chidren[0]->getContent();
+		$this->assertNoPattern('/secret/', $content);
+	}
 }
diff --git a/engine/tests/test_files/xxe/external_entity.txt b/engine/tests/test_files/xxe/external_entity.txt
new file mode 100644
index 000000000..536aca34d
--- /dev/null
+++ b/engine/tests/test_files/xxe/external_entity.txt
@@ -0,0 +1 @@
+secret
\ No newline at end of file
diff --git a/engine/tests/test_files/xxe/request.xml b/engine/tests/test_files/xxe/request.xml
new file mode 100644
index 000000000..4390f9db2
--- /dev/null
+++ b/engine/tests/test_files/xxe/request.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<!DOCTYPE foo [
+<!ELEMENT methodName ANY >
+<!ENTITY xxe SYSTEM "%s" >
+]>
+<methodCall>
+    <methodName>test&xxe;test</methodName>
+</methodCall>
-- 
cgit v1.2.3