6.0.0-git
2021-01-18

[#6060] Incorrect return type from cache miss
Summary Incorrect return type from cache miss
Queue Horde Framework Packages
Queue Version FRAMEWORK_3
Type Bug
State Resolved
Priority 1. Low
Owners chuck (at) horde (dot) org
Requester bklang (at) horde (dot) org
Created 2008-01-02 (4765 days ago)
Due
Updated 2008-01-02 (4765 days ago)
Assigned
Resolved 2008-01-02 (4765 days ago)
Milestone
Patch No

History
2008-01-02 19:38:32 Chuck Hagenbuch Comment #2
Assigned to Chuck Hagenbuch
State ⇒ Resolved
Reply to this comment
Yup, needed an is_null check. Thanks.
2008-01-02 03:43:56 Ben Klang Comment #1
Type ⇒ Bug
State ⇒ Unconfirmed
Priority ⇒ 1. Low
Summary ⇒ Incorrect return type from cache miss
Queue ⇒ Horde Framework Packages
Reply to this comment
I have Horde FW_3 from CVS as of about 2 hours ago.  My installation 
is configured to use the MySQL cache backend.  While calling 
$perms->exists() I see that a cache miss results in an erroneous 
$perms result.  Fortunately it is a "fail-closed" so no extra 
permissions are granted, but the unsuspecting user will not see the 
application to which he has been granted.



From framework/Perms/Perms/datatree.php line 139:

         $perm = $this->_cache->get('perm_' . $name, 
$GLOBALS['conf']['cache']['default_lifetime']);

         if ($perm === false) {

$this->_cache->set('perm_' . $name, serialize($perm), 
$GLOBALS['conf']['cache']['default_lifetime']);

             $permsCache[$name] = $perm;

         }



This code expects a cache miss to return a false.  Inspection of the 
code seems to take some care to ensure this happens (for instance on a 
DB error).  However the return type for DB::common::getOne() is NULL 
if the row does not exist.  This causes entries which have not yet 
been cached to return a type of NULL.  From line 150 of 
framework/Cache/Cache/sql.php:



         $result = $this->_db->getOne($query, $values);

         if (is_a($result, 'PEAR_Error')) {

             Horde::logMessage($result, __FILE__, __LINE__, PEAR_LOG_ERR);

             return false;

         } else {

             if ($this->_mc) {

                 $this->_mc->set($key, $result);

             }

             return $result;

         }



I *think* the correct action here would be to check for a return type 
of NULL from getOne() and then return false, indicating a cache miss.   
However, I'm not very coherent on the bigger picture for the Cache 
system yet, so I'll leave it for discussion here.  Did I miss something?



Practical example:  In my case I have a module called "site" which has 
granted "show/read" perms to Guests.  When a guest goes into that app 
they are prompted with a login screen.  Inserting debug statements 
into the code shows that while I have indeed defined a "site" 
permission, it is never checked directly because the Perms/datatree 
code receives a NULL from the Cache driver.

Saved Queries