Remove validation bypass
authoreileen <emcnaughton@wikimedia.org>
Mon, 25 May 2020 00:15:55 +0000 (12:15 +1200)
committereileen <emcnaughton@wikimedia.org>
Mon, 25 May 2020 00:20:35 +0000 (12:20 +1200)
I was trying to understand why executeQuery allows validation to be bypassed. That seems
intuitively like something that should be handled by the calling function. This is one of the few
places that calls it - but I can't see a reason for it to by pass validation....

My best guess is that a parameter could legitimately be NULL - and the group properly is the
only one that seems to be a candidate

I also note the case for passing i18nRewrite seems dubious as the table has no localisation

CRM/Utils/Cache/SqlGroup.php

index 3f499aa93022ff201cc8c61be5d797772f240c9d..8d0205a8e666d42d601c1440a307c301bc1b91ce 100644 (file)
@@ -99,7 +99,12 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
    * @param string $key
    * @param mixed $value
    * @param null|int|\DateInterval $ttl
+   *
    * @return bool
+   *
+   * @throws \CRM_Core_Exception
+   * @throws \CRM_Utils_Cache_CacheException
+   * @throws \CRM_Utils_Cache_InvalidArgumentException
    */
   public function set($key, $value, $ttl = NULL) {
     CRM_Utils_Cache::assertValidKey($key);
@@ -127,18 +132,18 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
         2 => [time(), 'Positive'],
         3 => [$expires, 'Positive'],
       ];
-      $dao = CRM_Core_DAO::executeQuery($sql, $args, FALSE, NULL, FALSE, FALSE);
+      CRM_Core_DAO::executeQuery($sql, $args, TRUE, NULL, FALSE, FALSE);
     }
     else {
       $sql = "INSERT INTO {$this->table} (group_name,path,data,created_date,expired_date) VALUES (%1,%2,%3,FROM_UNIXTIME(%4),FROM_UNIXTIME(%5))";
       $args = [
-        1 => [$this->group, 'String'],
+        1 => [(string) $this->group, 'String'],
         2 => [$key, 'String'],
         3 => [$dataSerialized, 'String'],
         4 => [time(), 'Positive'],
         5 => [$expires, 'Positive'],
       ];
-      $dao = CRM_Core_DAO::executeQuery($sql, $args, FALSE, NULL, FALSE, FALSE);
+      CRM_Core_DAO::executeQuery($sql, $args, TRUE, NULL, FALSE, FALSE);
     }
 
     $lock->release();
@@ -153,6 +158,8 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
    * @param mixed $default
    *
    * @return mixed
+   *
+   * @throws \CRM_Utils_Cache_InvalidArgumentException
    */
   public function get($key, $default = NULL) {
     CRM_Utils_Cache::assertValidKey($key);
@@ -167,12 +174,17 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
     return (isset($this->expiresCache[$key]) && time() < $this->expiresCache[$key]) ? $this->reobjectify($this->valueCache[$key]) : $default;
   }
 
+  /**
+   * @param mixed $value
+   *
+   * @return object
+   */
   private function reobjectify($value) {
     return is_object($value) ? unserialize(serialize($value)) : $value;
   }
 
   /**
-   * @param $key
+   * @param string $key
    * @param null $default
    *
    * @return mixed
@@ -193,7 +205,9 @@ class CRM_Utils_Cache_SqlGroup implements CRM_Utils_Cache_Interface {
 
   /**
    * @param string $key
+   *
    * @return bool
+   * @throws \CRM_Utils_Cache_InvalidArgumentException
    */
   public function delete($key) {
     CRM_Utils_Cache::assertValidKey($key);