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