From c7572e614606396617cb1611653f6a759be0a058 Mon Sep 17 00:00:00 2001 From: Paweł Sroka 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 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