6.0.0-git
2018-12-15

[#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 (at)
Requester thomas.jarosch (at) intra2net (dot) com
Created 2013-01-28 (2147 days ago)
Due
Updated 2016-02-02 (1047 days ago)
Assigned 2013-08-27 (1936 days ago)
Resolved
Milestone
Patch Yes

History
2016-02-02 09:28:11 Jan Schneider Assigned to Horde DevelopersHorde Developers
Taken from Jan Schneider
 
2013-12-18 10:49:48 Thomas Jarosch Comment #7 Reply to this comment
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.

2013-08-27 10:41:44 Jan Schneider Assigned to Jan Schneider
State ⇒ Assigned
 
2013-03-19 14:04:33 Michael Rubinsky Comment #6 Reply to this comment
ping?
2013-02-08 09:34:54 Thomas Jarosch Comment #5 Reply to this comment
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...

2013-02-04 14:53:18 Michael Rubinsky Comment #4 Reply to this comment
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.
2013-02-04 08:22:15 Thomas Jarosch Comment #3 Reply to this comment
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)

2013-02-01 20:19:12 Michael Rubinsky Comment #2
State ⇒ Feedback
Reply to this comment
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()?
2013-01-28 10:57:11 Thomas Jarosch New Attachment: 0001-Fix-11999-Prevent-inserting-empty-share-objects-into.patch Download
 
2013-01-28 10:55:36 Thomas Jarosch Comment #1
Type ⇒ Bug
State ⇒ Unconfirmed
Priority ⇒ 1. Low
Summary ⇒ Prevent inserting empty share "objects" into $all_shares array
Queue ⇒ Horde Framework Packages
Milestone ⇒
Patch ⇒ Yes
Reply to this comment
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

Saved Queries