Refs #2912. Added checks for constraints in dangerous functions. Unit tests no longer...

git-svn-id: http://code.elgg.org/elgg/trunk@8215 36083f99-b078-4883-b0ff-0f9b5a30f544

diff --git a/engine/lib/annotations.php b/engine/lib/annotations.php
index 9b3b4962674c15cc1214c8a2d3ff00995ed69e87..d4483ab8262c94bd78a9380079f0fc0fe1dc996f 100644 (file)
--- 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 f86d946212538bdf32b4bccfd1def25f8f460721..431e32a239c052d2b942075f84fb297499d1ba17 100644 (file)
--- 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 a00b21c5289490a063c0c89657ded2cf799ac210..95330844d159850511c7d8a7b2e38a5138c14b05 100644 (file)
--- 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 8a62929d5856985af4800f7681322694cbd17e4d..c3aebb1110b59b6afaac73fd1dfe7144d9a3c077 100644 (file)
--- 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 c18e42eb8bc0cc101f920b85d46d18ab41ef11ba..9d089f804707ea22a326b49612cd98b7c1e5ec3d 100644 (file)
--- 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 89ef255729a6c31a161602bdd36059c24ba44483..f6ee532d30c68c4ebddada646310ac4de26fdbf9 100644 (file)
--- 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'));