[dev] [PATCH] ActiveSync: Fix email sync error on corner case

[Michael J Rubinsky]

Quoting Thomas Jarosch <thomas.jarosch at intra2net.com>:

> Hi Michael,
>
> On Thursday, 12. December 2013 12:18:01 Michael J Rubinsky wrote:
>> > Sounds like a possibility. ActiveSync_State_Base::getChanges() calls
>> > getServerChanges(). If we are not pinging the folder, we call
>> > $folder->updateState() later on.
>> >
>> > When an Horde_Exception is thrown in getServerChanges(),
>> > we just catch it and return an empty changes array.
>> > Still we are executing $folder->updateState().
>> >
>> > What about some kind of race condition: A new message / just deleted
>> > message is not listed in $this->_messages, but part of the $messages
>> > list given to setChanges(). We'll then throw the exception and kill the
>> > flags
>> > later on in updateState().
>>
>> If it's a new message, the UID would be greater than the previous
>> UIDNEXT value, so it would be treated as a new message and added to
>> the $_messages array. Even if it was removed in the small time between
>> we received the message list and called setChanges(), it
>> wouldn't/shouldn't matter. The only way I can see this happening is if
>> the UIDNEXT values was incorrect, or somehow a message with a UID
>> lower than the already existing lowest on the device is provided to
>> setChanges().
>
> ok, makes sense. I've seen you already changed the code a bit
> to be more gentle to errors.
>
> What I wanted to test in a quiet minute (with the old code):
> Throw an exception on purpose in getServerChanges().
>
> Something like this (pseudo code only):
> ---------------------
> clearstatcache();
> if (file_exists("/tmp/throw_exception_once")) {
>    unlink("/tmp/throw_exception_once");
>    throw new Horde_Exception("fake IMAP error");
> }
> ---------------------
>
>
> I want to find out if any thrown exception inside getServerChanges()
> kills the flag array on non-CONDSTORE servers as suspected
> later on in $folder->updateState().

Basically, this issue occurs when updateState() is called without the  
flag array being set. It looks like the only way this can happen (now  
that the previous issue was fixed) is if $folder->setChanges() is not  
called, but $folder->updateState() is. In the case you describe above,  
it would only be a problem if the exception thrown from inside  
Horde_Core_ActiveSync_Driver::getServerChanges() is thrown before  
$this->_imap->getMessageChanges() is called - since that is where  
$folder->setChanges() is called. Also, it looks like we should  
probably catch exceptions from the imap library in  
Horde_ActiveSync_Imap_Adapter::getMessageChanges() to ensure that  
doesn't prevent it from being called.

Either way, it wouldn't hurt to add some logic to  
Horde_ActiveSync_Folder_Imap to ensure that $flags was set before  
mangling $_messages.

$todo++;


-- 
mike

The Horde Project (www.horde.org)
mrubinsk at horde.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5849 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/dev/attachments/20131218/74cf6979/attachment.bin>