From c7572e614606396617cb1611653f6a759be0a058 Mon Sep 17 00:00:00 2001
From: Paweł Sroka <srokap@gmail.com>
Date: Mon, 27 May 2013 05:17:08 +0200
Subject: Refs #5538 - Added regression test

---
 engine/tests/regression/trac_bugs.php | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

(limited to 'engine')

diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index 7fdd51c27..180fb5112 100644
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -296,4 +296,34 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
 			$this->assertEqual($output, parse_urls($input));
 		}
 	}
+	
+	/**
+	 * Checks if additional select columns does not leak to entity attributes.
+	 * 
+	 * https://github.com/Elgg/Elgg/issues/5538
+	 */
+	public function test_sql_selects_leak_to_attributes() {
+		global $ENTITY_CACHE;
+		//may not have groups in DB - let's create one	
+		$group = new ElggGroup();
+		$group->name = 'test_group';
+		$group->access_id = ACCESS_PUBLIC;
+		$this->assertTrue($group->save() !== false);
+		
+		//entity cache interferes with our test
+		$ENTITY_CACHE = array();
+		
+		foreach (array('site', 'user', 'group', 'object') as $type) {
+			$entities = elgg_get_entities(array(
+				'type' => $type,
+				'selects' => array('42 as added_col'),
+				'limit' => 1,
+			));
+			$entity = array_shift($entities);
+			$this->assertTrue($entity instanceof ElggEntity);
+			$this->assertEqual($entity->added_col, null, "Additional select columns are leaking to attributes for " . get_class($entity));
+		}
+		
+		$group->delete();
+	}
 }
-- 
cgit v1.2.3


From 2e979f75cd625c42ac5251140a3e7a797108f669 Mon Sep 17 00:00:00 2001
From: Steve Clay <steve@mrclay.org>
Date: Sat, 1 Jun 2013 21:51:32 -0400
Subject: Add filtering for site entities, faster filtering

---
 engine/classes/ElggAttributeLoader.php | 30 ++++++++++++++++++++++++++++-
 engine/tests/regression/trac_bugs.php  | 35 ++++++++++++++++++----------------
 2 files changed, 48 insertions(+), 17 deletions(-)

(limited to 'engine')

diff --git a/engine/classes/ElggAttributeLoader.php b/engine/classes/ElggAttributeLoader.php
index b91e4b88a..0b770da75 100644
--- a/engine/classes/ElggAttributeLoader.php
+++ b/engine/classes/ElggAttributeLoader.php
@@ -24,7 +24,7 @@ class ElggAttributeLoader {
 		'time_created',
 		'time_updated',
 		'last_action',
-		'enabled'
+		'enabled',
 	);
 
 	/**
@@ -200,6 +200,8 @@ class ElggAttributeLoader {
 						// saved, these are stored w/ type "site", but with no sites_entity row. These
 						// are probably only created in the unit tests.
 						// @todo Don't save vanilla ElggEntities with type "site"
+
+						$row = $this->filterAddedColumns($row);
 						$row['guid'] = (int) $row['guid'];
 						return $row;
 					}
@@ -209,6 +211,8 @@ class ElggAttributeLoader {
 			}
 		}
 
+		$row = $this->filterAddedColumns($row);
+
 		// Note: If there are still missing attributes, we're running on a 1.7 or earlier schema. We let
 		// this pass so the upgrades can run.
 
@@ -217,4 +221,28 @@ class ElggAttributeLoader {
 
 		return $row;
 	}
+
+	/**
+	 * Filter out keys returned by the query which should not appear in the entity's attributes
+	 *
+	 * @param array $row All columns from the query
+	 * @return array Columns acceptable for the entity's attributes
+	 */
+	protected function filterAddedColumns($row) {
+		// make an array with keys as acceptable attribute names
+		$acceptable_attrs = self::$primary_attr_names;
+		array_splice($acceptable_attrs, count($acceptable_attrs), 0, $this->secondary_attr_names);
+		$acceptable_attrs = array_combine($acceptable_attrs, $acceptable_attrs);
+
+		// @todo remove these when #4584 is in place
+		$acceptable_attrs['tables_split'] = true;
+		$acceptable_attrs['tables_loaded'] = true;
+
+		foreach ($row as $key => $val) {
+			if (!isset($acceptable_attrs[$key])) {
+				unset($row[$key]);
+			}
+		}
+		return $row;
+	}
 }
diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index 180fb5112..d7bb20f3b 100644
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -297,31 +297,34 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
 		}
 	}
 	
-	/**
-	 * Checks if additional select columns does not leak to entity attributes.
-	 * 
-	 * https://github.com/Elgg/Elgg/issues/5538
-	 */
-	public function test_sql_selects_leak_to_attributes() {
+	/**
+	 * Ensure additional select columns do not end up in entity attributes.
+	 * 
+	 * https://github.com/Elgg/Elgg/issues/5538
+	 */
+	public function test_extra_columns_dont_appear_in_attributes() {
 		global $ENTITY_CACHE;
-		//may not have groups in DB - let's create one	
+
+		// may not have groups in DB - let's create one
 		$group = new ElggGroup();
 		$group->name = 'test_group';
-		$group->access_id = ACCESS_PUBLIC;
-		$this->assertTrue($group->save() !== false);
+		$group->access_id = ACCESS_PUBLIC;
+		$this->assertTrue($group->save() !== false);
 		
-		//entity cache interferes with our test
+		// entity cache interferes with our test
 		$ENTITY_CACHE = array();
 		
 		foreach (array('site', 'user', 'group', 'object') as $type) {
 			$entities = elgg_get_entities(array(
-				'type' => $type,
-				'selects' => array('42 as added_col'),
-				'limit' => 1,
+				'type' => $type,
+				'selects' => array('1 as _nonexistent_test_column'),
+				'limit' => 1,
 			));
-			$entity = array_shift($entities);
-			$this->assertTrue($entity instanceof ElggEntity);
-			$this->assertEqual($entity->added_col, null, "Additional select columns are leaking to attributes for " . get_class($entity));
+			if (!$this->assertTrue($entities, "Query for '$type' did not return an entity.")) {
+				continue;
+			}
+			$entity = $entities[0];
+			$this->assertNull($entity->_nonexistent_test_column, "Additional select columns are leaking to attributes for '$type'");
 		}
 		
 		$group->delete();
-- 
cgit v1.2.3