Tickets :: Comment on [#6060] Incorrect return type from cache miss

* Your Email Address
* Spam protection	Enter the letters below: .___..__ .___.. ..__. \| [__) _/ \ /\| \| \| \| \./__. \/ \|__\|
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.
Attachment
Watch this ticket

Saved Queries