From abe71f46c3b2e657db25ac16c43a4c76b2212a9f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dan=20Vr=C3=A1til?= <dvratil@redhat.com>
Date: Wed, 17 Jun 2015 13:04:13 +0200
Subject: [PATCH 32/34] Don't throw exception when MOVE handler finds no items
 to move

Instead return "OK MOVE complete" right away. The reason for this is that
when client tries to move an Item from a folder into the same folder (it's
possible in KMail, also mailfilter agent might trigger this situation) the
subsequent command gets eaten by ImapStreamParser and the client's Job gets
stuck waiting for response forever. According to Laurent this could also fix
the Mail Filter Agent getting stuck occasionally.

The problem is in ImapStreamParser::atCommandEnd() method, which is called
by the Move handler at some point. atCommandEnd() checks whether we reached
command end in the stream by looking if the next characters in the stream
are "\r\n" and if so it will consume the command end ("\r\n"), effectively
moving the streaming position BEYOND the command. In case of MOVE the
command has already been completely parsed so we are actually at the end of
the command and so ImapStreamParser will consume the "\r\n" and position the
stream beyond the command end.

After that the Move handler tries to get the items from DB and throws the
exception (the second part of the condition in the SQL query causes that
the query yields no results in this situation)  which gets us back to
Connection where we then call ImapStreamParser::skipCommand(). At this point
however there are no more data in the stream (because atCommandEnd() moved
us beyond the end of the MOVE command) and so ImapStreamParser will block
and wait for more data (with 30 seconds timeout). If client sends another
command within this time the ImapStreamParser will think that this is the
command to be skipped and will consume it. This means that the command never
really reaches the Connection as it's consumed as soon as it's captured by
ImapStreamParser. And because Akonadi never receives the command it cannot
send a response and thus the Job in client will wait forever and ever...

Proper fix would be to make ImapStreamParser::atCommandEnd() to only peek
instead of actually altering the position in the stream however I'm really
afraid that it could break some other stuff that relies on this (broken?)
behaviour and our test coverage is not sufficient at this point to be
reliable enough.
---
 server/src/handler/move.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/src/handler/move.cpp b/server/src/handler/move.cpp
index 0a6c3bf..4cf9d4e 100644
--- a/server/src/handler/move.cpp
+++ b/server/src/handler/move.cpp
@@ -85,7 +85,7 @@ bool Move::parseStream()
   if ( qb.exec() ) {
     const QVector<PimItem> items = qb.result();
     if ( items.isEmpty() ) {
-      throw HandlerException( "No items found" );
+      return successResponse( "MOVE complete" );
     }
 
     // Split the list by source collection
-- 
2.4.3