]>
Commit | Line | Data |
---|---|---|
8a8f9fb3 AM |
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 |