0032-Don-t-throw-exception-when-MOVE-handler-finds-no-ite.patch

   1 From abe71f46c3b2e657db25ac16c43a4c76b2212a9f Mon Sep 17 00:00:00 2001
   2 From: =?UTF-8?q?Dan=20Vr=C3=A1til?= <dvratil@redhat.com>
   3 Date: Wed, 17 Jun 2015 13:04:13 +0200
   4 Subject: [PATCH 32/34] Don't throw exception when MOVE handler finds no items
   5  to move
   6
   7 Instead return "OK MOVE complete" right away. The reason for this is that
   8 when client tries to move an Item from a folder into the same folder (it's
   9 possible in KMail, also mailfilter agent might trigger this situation) the
  10 subsequent command gets eaten by ImapStreamParser and the client's Job gets
  11 stuck waiting for response forever. According to Laurent this could also fix
  12 the Mail Filter Agent getting stuck occasionally.
  13
  14 The problem is in ImapStreamParser::atCommandEnd() method, which is called
  15 by the Move handler at some point. atCommandEnd() checks whether we reached
  16 command end in the stream by looking if the next characters in the stream
  17 are "\r\n" and if so it will consume the command end ("\r\n"), effectively
  18 moving the streaming position BEYOND the command. In case of MOVE the
  19 command has already been completely parsed so we are actually at the end of
  20 the command and so ImapStreamParser will consume the "\r\n" and position the
  21 stream beyond the command end.
  22
  23 After that the Move handler tries to get the items from DB and throws the
  24 exception (the second part of the condition in the SQL query causes that
  25 the query yields no results in this situation)  which gets us back to
  26 Connection where we then call ImapStreamParser::skipCommand(). At this point
  27 however there are no more data in the stream (because atCommandEnd() moved
  28 us beyond the end of the MOVE command) and so ImapStreamParser will block
  29 and wait for more data (with 30 seconds timeout). If client sends another
  30 command within this time the ImapStreamParser will think that this is the
  31 command to be skipped and will consume it. This means that the command never
  32 really reaches the Connection as it's consumed as soon as it's captured by
  33 ImapStreamParser. And because Akonadi never receives the command it cannot
  34 send a response and thus the Job in client will wait forever and ever...
  35
  36 Proper fix would be to make ImapStreamParser::atCommandEnd() to only peek
  37 instead of actually altering the position in the stream however I'm really
  38 afraid that it could break some other stuff that relies on this (broken?)
  39 behaviour and our test coverage is not sufficient at this point to be
  40 reliable enough.
  41 ---
  42  server/src/handler/move.cpp | 2 +-
  43  1 file changed, 1 insertion(+), 1 deletion(-)
  44
  45 diff --git a/server/src/handler/move.cpp b/server/src/handler/move.cpp
  46 index 0a6c3bf..4cf9d4e 100644
  47 --- a/server/src/handler/move.cpp
  48 +++ b/server/src/handler/move.cpp
  49 @@ -85,7 +85,7 @@ bool Move::parseStream()
  50    if ( qb.exec() ) {
  51      const QVector<PimItem> items = qb.result();
  52      if ( items.isEmpty() ) {
  53 -      throw HandlerException( "No items found" );
  54 +      return successResponse( "MOVE complete" );
  55      }
  56
  57      // Split the list by source collection
  58 --
  59 2.4.3
  60