From c6fa7b510dc938a14ddd1d2ecf2c98017924b9da Mon Sep 17 00:00:00 2001 From: brettp Date: Mon, 14 Feb 2011 01:24:51 +0000 Subject: Refs #2912. Added checks for constraints in dangerous functions. Unit tests no longer remove all metadata/annotations. git-svn-id: http://code.elgg.org/elgg/trunk@8215 36083f99-b078-4883-b0ff-0f9b5a30f544 --- engine/lib/annotations.php | 4 +-- engine/lib/deprecated-1.8.php | 2 +- engine/lib/elgglib.php | 55 +++++++++++++++++++++++++++++ engine/lib/metadata.php | 6 ++-- engine/tests/api/metastrings.php | 53 +++++++++++++++++++++++++++ mod/pages/views/default/object/page_top.php | 1 + 6 files changed, 116 insertions(+), 5 deletions(-) diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php index 9b3b49626..d4483ab82 100644 --- a/engine/lib/annotations.php +++ b/engine/lib/annotations.php @@ -200,7 +200,7 @@ function elgg_get_annotations(array $options = array()) { * @since 1.8 */ function elgg_delete_annotations(array $options) { - if (!$options || !is_array($options)) { + if (!elgg_is_valid_options_for_batch_operation($options, 'annotations')) { return false; } @@ -218,7 +218,7 @@ function elgg_delete_annotations(array $options) { * @since 1.8 */ function elgg_disable_annotations(array $options) { - if (!$options || !is_array($options)) { + if (!elgg_is_valid_options_for_batch_operation($options, 'annotations')) { return false; } diff --git a/engine/lib/deprecated-1.8.php b/engine/lib/deprecated-1.8.php index f86d94621..431e32a23 100644 --- a/engine/lib/deprecated-1.8.php +++ b/engine/lib/deprecated-1.8.php @@ -3630,7 +3630,7 @@ function clear_metadata_by_owner($owner_guid) { if (!$owner_guid) { return false; } - return elgg_delete_metadata(array('metadata_owner' => $owner_guid, 'limit' => 0)); + return elgg_delete_metadata(array('metadata_owner_guid' => $owner_guid, 'limit' => 0)); } /** diff --git a/engine/lib/elgglib.php b/engine/lib/elgglib.php index a00b21c52..95330844d 100644 --- a/engine/lib/elgglib.php +++ b/engine/lib/elgglib.php @@ -1725,6 +1725,61 @@ function elgg_batch_delete_callback($object) { return $object->delete() ? true : false; } +/** + * Checks if there are some constraints on the options array for + * potentially dangerous operations. + * + * @param array $options Options array + * @param string $type Options type: metadata or annotations + * @return bool + */ +function elgg_is_valid_options_for_batch_operation($options, $type) { + if (!$options || !is_array($options)) { + return false; + } + + // at least one of these is required. + $required = array( + // generic restraints + 'guid', 'guids', 'limit' + ); + + switch ($type) { + case 'metadata': + $metadata_required = array( + 'metadata_owner_guid', 'metadata_owner_guids', + 'metadata_name', 'metadata_names', + 'metadata_value', 'metadata_values' + ); + + $required = array_merge($required, $metadata_required); + break; + + case 'annotations': + case 'annotation': + $annotations_required = array( + 'annotation_owner_guid', 'annotation_owner_guids', + 'annotation_name', 'annotation_names', + 'annotation_value', 'annotation_values' + ); + + $required = array_merge($required, $annotations_required); + break; + + default: + return false; + } + + foreach ($required as $key) { + // check that it exists and is something. + if (isset($options[$key]) && $options[$key]) { + return true; + } + } + + return false; +} + /** * Intercepts the index page when Walled Garden mode is enabled. * diff --git a/engine/lib/metadata.php b/engine/lib/metadata.php index 8a62929d5..c3aebb111 100644 --- a/engine/lib/metadata.php +++ b/engine/lib/metadata.php @@ -281,13 +281,15 @@ function elgg_get_metadata(array $options = array()) { * Deletes metadata based on $options. * * @warning Unlike elgg_get_metadata() this will not accept an empty options array! + * This requires some constraints: metadata_owner_guid(s), + * metadata_name(s), metadata_value(s), or limit must be set. * * @param array $options An options array. {@See elgg_get_metadata()} * @return mixed * @since 1.8 */ function elgg_delete_metadata(array $options) { - if (!$options || !is_array($options)) { + if (!elgg_is_valid_options_for_batch_operation($options, 'metadata')) { return false; } @@ -305,7 +307,7 @@ function elgg_delete_metadata(array $options) { * @since 1.8 */ function elgg_disable_metadata(array $options) { - if (!$options || !is_array($options)) { + if (!elgg_is_valid_options_for_batch_operation($options, 'metadata')) { return false; } diff --git a/engine/tests/api/metastrings.php b/engine/tests/api/metastrings.php index c18e42eb8..9d089f804 100644 --- a/engine/tests/api/metastrings.php +++ b/engine/tests/api/metastrings.php @@ -135,5 +135,58 @@ class ElggCoreMetastringsTest extends ElggCoreUnitTest { } } + public function testKeepMeFromDeletingEverything() { + foreach ($this->metastringTypes as $type) { + $required = array( + 'guid', 'guids', 'limit' + ); + + switch ($type) { + case 'metadata': + $metadata_required = array( + 'metadata_owner_guid', 'metadata_owner_guids', + 'metadata_name', 'metadata_names', + 'metadata_value', 'metadata_values' + ); + + $required = array_merge($required, $metadata_required); + break; + + case 'annotations': + $annotations_required = array( + 'annotation_owner_guid', 'annotation_owner_guids', + 'annotation_name', 'annotation_names', + 'annotation_value', 'annotation_values' + ); + + $required = array_merge($required, $annotations_required); + break; + } + + $options = array(); + $this->assertFalse(elgg_is_valid_options_for_batch_operation($options), $type); + + foreach ($required as $key) { + $options = array(); + $options[$key] = ELGG_ENTITIES_ANY_VALUE; + $this->assertFalse(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = ELGG_ENTITIES_ANY_VALUE"); + + $options[$key] = ELGG_ENTITIES_NO_VALUE; + $this->assertFalse(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = ELGG_ENTITIES_NO_VALUE"); + + $options[$key] = false; + $this->assertFalse(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = bool false"); + + $options[$key] = true; + $this->assertTrue(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = bool true"); + + $options[$key] = 'test'; + $this->assertTrue(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = 'test'"); + + $options[$key] = array('test'); + $this->assertTrue(elgg_is_valid_options_for_batch_operation($options, $type), "Sent $key = array('test')"); + } + } + } } diff --git a/mod/pages/views/default/object/page_top.php b/mod/pages/views/default/object/page_top.php index 89ef25572..f6ee532d3 100644 --- a/mod/pages/views/default/object/page_top.php +++ b/mod/pages/views/default/object/page_top.php @@ -26,6 +26,7 @@ if ($revision) { $annotation = $annotation[0]; } } +$annotation = null; $page_icon = elgg_view('pages/icon', array('annotation' => $annotation, 'size' => 'small')); -- cgit v1.2.3