[#11999] Prevent inserting empty share "objects" into $all_shares array
Summary Prevent inserting empty share "objects" into $all_shares array
Queue Horde Framework Packages
Queue Version Git master
Type Bug
State Assigned
Priority 1. Low
Owners Horde Developers
Requester thomas.jarosch@intra2net.com
Created 2013-01-28 (2364 days ago)
Due
Updated 2016-02-02 (1264 days ago)
Assigned 2013-08-27 (2153 days ago)
Resolved
Milestone
Patch Yes

Comments
Thomas Jarosch <thomas.jarosch@intra2net.com> 2013-01-28 10:55:36
Hi,

I just had a crash during login:

[Mon Jan 28 11:14:31 2013] [error] [client 127.0.0.1] PHP Fatal error: 
  Call to a member function getName() on a non-object in 
/usr/share/pear/Horde/Share/Base.php on line 716

Full backtrace:
#0 [internal function]: 
Horde_Share_Base->_sortShares(Object(Horde_Share_Object_Kolab), NULL)
#1 /usr/share/pear/Horde/Share/Base.php(356): uasort(Array, Array)
#2 [internal function]: Horde_Share_Base->listShares('USERNAME', Array)
#3 /usr/share/pear/Horde/Core/Share/Driver.php(61): 
call_user_func_array(Array, Array)
#4 /usr/share/pear/www/horde/turba/lib/Turba.php(623): 
Horde_Core_Share_Driver->__call('listShares', Array)
#5 /usr/share/pear/www/horde/turba/lib/Turba.php(623): 
Horde_Core_Share_Driver->listShares('USERNAME', Array)
#6 /usr/share/pear/www/horde/turba/lib/Turba.php(442): Turba::listShares()
#7 /usr/share/pear/www/horde/turba/lib/Application.php(87): 
Turba::getConfigFromShares(Array)
#8 /usr/share/pear/Horde/Registry/Application.php(105): 
Turba_Application->_init()
#9 [internal function]: Horde_Registry_Application->init()
#10 /usr/share/pear/Horde/Registry.php(1139): 
call_user_func_array(Array, Array)
#11 /usr/share/pear/Horde/Registry.php(1543): 
Horde_Registry->callAppMethod('turba', 'init')
#12 /usr/share/pear/Horde/Registry.php(1071): 
Horde_Registry->pushApp('turba', Array)
#13 /usr/share/pear/Horde/Registry.php(1031): 
Horde_Registry->callByPackage('turba', 'listTimeObjectC...', Array)
#14 /usr/share/pear/www/horde/kronolith/lib/Kronolith.php(912): 
Horde_Registry->call('contacts/listTi...')
#15 /usr/share/pear/www/horde/kronolith/lib/Application.php(75): 
Kronolith::initialize()
#16 /usr/share/pear/Horde/Registry/Application.php(105): 
Kronolith_Application->_init()
#17 [internal function]: Horde_Registry_Application->init()
#18 /usr/share/pear/Horde/Registry.php(1139): 
call_user_func_array(Array, Array)
#19 /usr/share/pear/Horde/Registry.php(1543): 
Horde_Registry->callAppMethod('kronolith', 'init')
#20 /usr/share/pear/Horde/Registry.php(1136): 
Horde_Registry->pushApp('kronolith', Array)
#21 /usr/share/pear/Horde/Core/Factory/Notification.php(63): 
Horde_Registry->callAppMethod('kronolith', 'setupNotificati...', Array)
#22 /usr/share/pear/Horde/Injector/Binder/Factory.php(111): 
Horde_Core_Factory_Notification->create(Object(Horde_Injector))
#23 /usr/share/pear/Horde/Injector.php(213): 
Horde_Injector_Binder_Factory->create(Object(Horde_Injector))
#24 /usr/share/pear/Horde/Injector.php(247): 
Horde_Injector->createInstance('Horde_Notificat...')
#25 /usr/share/pear/Horde/Registry.php(507): 
Horde_Injector->getInstance('Horde_Notificat...')
#26 /usr/share/pear/Horde/Registry.php(238): 
Horde_Registry->__construct(0, Array)
#27 /usr/share/pear/www/horde/index.php(20): 
Horde_Registry::appInit('horde', Array)
#28 {main}


Turned out we added empty "objects" to the $all_shares list.
Attached patch fixes that.

Thomas


Michael Rubinsky <mrubinsk@horde.org> 2013-02-01 20:19:12
I'm not so sure we should be silently ignoring any case where we can't 
load an explicitly requested share.

Do you know *why* at least one of the shares requested in $cids was 
not found by _getShares()?

Thomas Jarosch <thomas.jarosch@intra2net.com> 2013-02-04 08:22:15
> I'm not so sure we should be silently ignoring any case where we 
> can't load an explicitly requested share.

Should we throw an error instead?

> Do you know *why* at least one of the shares requested in $cids was 
> not found by _getShares()?

Unfortunately I couldn't properly trace it as it was a busy productive system
and I didn't want to add too many debug printfs() in there. Every time
I forget an ";", a few users end up with a blank page :o)


Michael Rubinsky <mrubinsk@horde.org> 2013-02-04 14:53:18
>> I'm not so sure we should be silently ignoring any case where we
>> can't load an explicitly requested share.
>
> Should we throw an error instead?

Horde_Exception_NotFound should be thrown from the concrete share 
class' _getShares() method.

Thomas Jarosch <thomas.jarosch@intra2net.com> 2013-02-08 09:34:54
>>> I'm not so sure we should be silently ignoring any case where we
>>> can't load an explicitly requested share.
>>
>> Should we throw an error instead?
>
> Horde_Exception_NotFound should be thrown from the concrete share 
> class' _getShares() method.

I've checked the Kolab Share code and it does thrown an exception if 
one requests a share id that cannot be found.

While debugging this, I remember from my debug output that a rather 
long and a short share id was requested. Wild guess: The short id was 
just the username.

I think this might be a Kolab specific problem, call stack is roughly 
like this:

Base->getShares($ids) -> Kolab->_getShares($ids) -> Kolab->_getShareById($id).

The found share is stored with "$share->getName()" as the key.

Wild guess: For the Kolab case, "$share->getName()" might return 
something different than the request id. For example, you just 
requested the username as id and get an IMAP folder name back.

The Base->getShares() function assumes the returned array of share 
objects strictly uses the id as array key.

@Jan: Is it by design that "Kolab->_getShares($ids)" uses 
"$share->getName()" as key?
The SQL driver seems to work this way.

The "getName()" function in Share/Object/Kolab.php uses getId() as 
fallback if "share_name" is not available. So this some kind of 
fallback automagic...


Michael Rubinsky <mrubinsk@horde.org> 2013-03-19 14:04:33
ping?

Thomas Jarosch <thomas.jarosch@intra2net.com> 2013-12-18 10:49:48
> ping?

ok, I found something out about this:

The Kolab backend crashes a horrible death if you turn the INBOX of a 
user into a groupware folder. This does not make any sense at all, but 
one user managed to set his INBOX to the kolab folder type "notes".

-> We crash badly with this.

I think that was the same issue here. Too bad I don't have the "id" 
anymore that caused it as it's a base64 encoded serialized php array 
-> we could easily reverse it and take a look.
That would also explain the "short" IDs I've been seeing.