From b39bbc2c08d270dc76a755b538f50e5a9055bf91 Mon Sep 17 00:00:00 2001 From: Ismayil Khayredinov Date: Fri, 22 Dec 2017 21:05:05 +0100 Subject: [PATCH] fix(search): validate comment ownership in format hook Make sure comment has an owner before trying to render an icon --- engine/tests/classes/Elgg/BaseTestCase.php | 1 + engine/tests/classes/Elgg/HookTesting.php | 20 +++ engine/tests/classes/Elgg/TestableHook.php | 158 ++++++++++++++++++ .../Elgg/Search/FormatComentEntityHook.php | 8 +- mod/search/classes/Elgg/Search/Formatter.php | 70 ++++---- .../Search/FormatCommentEntityHookTest.php | 58 +++++++ 6 files changed, 278 insertions(+), 37 deletions(-) create mode 100644 engine/tests/classes/Elgg/HookTesting.php create mode 100644 engine/tests/classes/Elgg/TestableHook.php create mode 100644 mod/search/tests/phpunit/unit/Elgg/Search/FormatCommentEntityHookTest.php diff --git a/engine/tests/classes/Elgg/BaseTestCase.php b/engine/tests/classes/Elgg/BaseTestCase.php index 3f708d4e228..dc624958daf 100644 --- a/engine/tests/classes/Elgg/BaseTestCase.php +++ b/engine/tests/classes/Elgg/BaseTestCase.php @@ -18,6 +18,7 @@ abstract class BaseTestCase extends TestCase implements Seedable, Testable { use Testing; use PluginTesting; + use HookTesting; static $_instance; static $_settings; diff --git a/engine/tests/classes/Elgg/HookTesting.php b/engine/tests/classes/Elgg/HookTesting.php new file mode 100644 index 00000000000..b033177d000 --- /dev/null +++ b/engine/tests/classes/Elgg/HookTesting.php @@ -0,0 +1,20 @@ +name = $name; + $hook->type = $type; + $hook->handler = $handler; + $hook->priority = $priority; + + return $hook->register($this); + } +} \ No newline at end of file diff --git a/engine/tests/classes/Elgg/TestableHook.php b/engine/tests/classes/Elgg/TestableHook.php new file mode 100644 index 00000000000..a0f413682c4 --- /dev/null +++ b/engine/tests/classes/Elgg/TestableHook.php @@ -0,0 +1,158 @@ +test_case = $test_case; + + elgg_register_plugin_hook_handler($this->name, $this->type, [$this, 'handler'], $this->priority); + + return $this; + } + + /** + * Unregister handler + * @return void + */ + public function unregister() { + elgg_unregister_plugin_hook_handler($this->name, $this->type, [$this, 'handler']); + } + + /** + * Handler hook call + * + * @param Hook $hook Hook + * + * @return mixed + */ + public function handler(\Elgg\Hook $hook) { + $this->calls++; + $this->before_state = $hook; + + list($success, $return, $hook) = _elgg_services()->handlers->call($this->handler, $hook, [ + $this->name, + $this->type, + $hook->getValue(), + $hook->getParams(), + ]); + + $this->after_state = $hook; + + if ($return !== null) { + $this->result = $return; + } else { + $this->result = $hook->getValue(); + } + + return $return; + } + + /** + * @return mixed + */ + public function getResult() { + return $this->result; + } + + /** + * Assert that hook was called expected number of times + * + * @param int $expected Expectation + */ + public function assertNumberOfCalls($expected) { + $this->test_case->assertEquals($expected, $this->calls); + } + + /** + * Assert that before hook handler is called the named parameter had a specific value + * + * @param string $name Name + * @param mixed $value Value + */ + public function assertParamBefore($name, $value) { + $this->test_case->assertEquals($this->before_state->getParam($name), $value); + } + + /** + * Assert that before hook handler is called the named parameter has a specific + * + * @param string $name Name + * @param mixed $value Value + */ + public function assertParamAfter($name, $value) { + $this->test_case->assertEquals($this->after_state->getParam($name), $value); + } + + /** + * Assert hook value before the handler was called + * + * @param mixed $value Value + */ + public function assertValueBefore($value) { + $this->test_case->assertEquals($this->before_state->getValue(), $value); + } + + /** + * Assert hook value after the handler was called + * + * @param mixed $value Value + */ + public function assertValueAfter($value) { + $this->test_case->assertEquals($this->after_state->getValue(), $value); + } + +} \ No newline at end of file diff --git a/mod/search/classes/Elgg/Search/FormatComentEntityHook.php b/mod/search/classes/Elgg/Search/FormatComentEntityHook.php index 878a555aaa9..6994e60cf74 100644 --- a/mod/search/classes/Elgg/Search/FormatComentEntityHook.php +++ b/mod/search/classes/Elgg/Search/FormatComentEntityHook.php @@ -29,10 +29,12 @@ public function __invoke(Hook $hook) { return; } + $icon = ''; $owner = $entity->getOwnerEntity(); - $size = $hook->getParam('size', 'small'); - $icon = elgg_view_entity_icon($owner, $size); - + if ($owner) { + $size = $hook->getParam('size', 'small'); + $icon = elgg_view_entity_icon($owner, $size); + } $container = $entity->getContainerEntity(); if (!$container instanceof ElggEntity) { return; diff --git a/mod/search/classes/Elgg/Search/Formatter.php b/mod/search/classes/Elgg/Search/Formatter.php index 7461d5976c8..4be72e073d6 100644 --- a/mod/search/classes/Elgg/Search/Formatter.php +++ b/mod/search/classes/Elgg/Search/Formatter.php @@ -144,43 +144,45 @@ protected function getPropertyMatches() { } $matches = []; - foreach ($fields as $property_type => $property_type_fields) { - foreach ($property_type_fields as $field) { - if (!empty($exclude[$property_type]) && in_array($field, $exclude[$property_type])) { - continue; - } - - switch ($property_type) { - case 'attributes' : - case 'metadata' : - $property_values = $this->entity->$field; - break; - - case 'annotations' : - $property_values = []; - $annotations = $this->entity->getAnnotations([ - 'annotation_names' => $field, - 'limit' => 0, - ]); - foreach ($annotations as $annotation) { - $property_values[] = $annotation->value; - } - break; + if (!empty($fields)) { + foreach ($fields as $property_type => $property_type_fields) { + foreach ($property_type_fields as $field) { + if (!empty($exclude[$property_type]) && in_array($field, $exclude[$property_type])) { + continue; + } - case 'private_settings' : - $property_values = $this->entity->getPrivateSetting($field); - break; - } + switch ($property_type) { + case 'attributes' : + case 'metadata' : + $property_values = $this->entity->$field; + break; + + case 'annotations' : + $property_values = []; + $annotations = $this->entity->getAnnotations([ + 'annotation_names' => $field, + 'limit' => 0, + ]); + foreach ($annotations as $annotation) { + $property_values[] = $annotation->value; + } + break; + + case 'private_settings' : + $property_values = $this->entity->getPrivateSetting($field); + break; + } - if (is_array($property_values)) { - foreach ($property_values as $text) { - if (stristr($text, $query)) { - $matches[$property_type][$field][] = $this->highlighter->highlight($text, 1, 300); + if (is_array($property_values)) { + foreach ($property_values as $text) { + if (stristr($text, $query)) { + $matches[$property_type][$field][] = $this->highlighter->highlight($text, 1, 300); + } + } + } else { + if (stristr($property_values, $query)) { + $matches[$property_type][$field][] = $this->highlighter->highlight($property_values, 1, 300); } - } - } else { - if (stristr($property_values, $query)) { - $matches[$property_type][$field][] = $this->highlighter->highlight($property_values, 1, 300); } } } diff --git a/mod/search/tests/phpunit/unit/Elgg/Search/FormatCommentEntityHookTest.php b/mod/search/tests/phpunit/unit/Elgg/Search/FormatCommentEntityHookTest.php new file mode 100644 index 00000000000..82553aec644 --- /dev/null +++ b/mod/search/tests/phpunit/unit/Elgg/Search/FormatCommentEntityHookTest.php @@ -0,0 +1,58 @@ +hooks->backup(); + } + + public function down() { + _elgg_services()->hooks->restore(); + } + + public function testCommentVolatileDataIsPopulated() { + $hook = $this->registerTestingHook('search:format', 'entity', FormatComentEntityHook::class); + + $object = $this->createObject([], [ + 'title' => 'Container', + ]); + $comment = $this->createObject([ + 'subtype' => 'comment', + 'owner_guid' => 0, + 'container_guid' => $object->guid, + ], [ + 'title' => 'Comment', + ]); + + $search = new Search([ + 'query' => 'comment', + 'search_type' => 'entities', + 'entity_type' => 'object', + 'entity_subtype' => 'comment', + ]); + + $formatter = new Formatter($comment, $search); + $formatter->format(); + + $hook->assertNumberOfCalls(1); + + $comment = $hook->getResult(); + + $this->assertEmpty($comment->getVolatileData('search_matched_icon')); + + $this->assertEquals( + elgg_echo('search:comment_on', ['Container']), + $comment->getVolatileData('search_matched_title') + ); + + $hook->unregister(); + + } +} \ No newline at end of file