Page MenuHomePhorge
Files F117878972

D4950.1775342458.diff
No OneTemporary
Actions

Authored By
Unknown
Size
6 KB
Referenced Files
None
Subscribers
None

D4950.1775342458.diff
View Options

diff --git a/lib/ext/Syncroton/Command/MoveItems.php b/lib/ext/Syncroton/Command/MoveItems.php
--- a/lib/ext/Syncroton/Command/MoveItems.php
+++ b/lib/ext/Syncroton/Command/MoveItems.php
@@ -59,11 +59,11 @@
$response = $moves->appendChild($this->_outputDom->createElementNS('uri:Move', 'Response'));
$response->appendChild($this->_outputDom->createElementNS('uri:Move', 'SrcMsgId', $move['srcMsgId']));
- try {
- if ($move['srcFldId'] === $move['dstFldId']) {
- throw new Syncroton_Exception_Status_MoveItems(Syncroton_Exception_Status_MoveItems::SAME_FOLDER);
- }
+ if ($this->_logger instanceof Zend_Log) {
+ $this->_logger->info(__METHOD__ . '::' . __LINE__ . " Moving from " . $move['srcFldId'] . " to " . $move['dstFldId']);
+ }
+ try {
try {
$sourceFolder = $this->_folderBackend->getFolder($this->_device, $move['srcFldId']);
} catch (Syncroton_Exception_NotFound $e) {
@@ -76,19 +76,30 @@
throw new Syncroton_Exception_Status_MoveItems(Syncroton_Exception_Status_MoveItems::INVALID_DESTINATION);
}
+ if ($move['srcFldId'] === $move['dstFldId']) {
+ throw new Syncroton_Exception_Status_MoveItems(Syncroton_Exception_Status_MoveItems::SAME_FOLDER);
+ }
+
$dataController = Syncroton_Data_Factory::factory($sourceFolder->class, $this->_device, $this->_syncTimeStamp);
$newId = $dataController->moveItem($move['srcFldId'], $move['srcMsgId'], $move['dstFldId']);
-
if (!$newId) {
+ // We don't actually know what the reason was that this failed, but from the resolution description this error seems to make the most sense,
+ // and we rule out most other reasons before.
throw new Syncroton_Exception_Status_MoveItems(Syncroton_Exception_Status_MoveItems::INVALID_SOURCE);
}
$response->appendChild($this->_outputDom->createElementNS('uri:Move', 'Status', Syncroton_Command_MoveItems::STATUS_SUCCESS));
$response->appendChild($this->_outputDom->createElementNS('uri:Move', 'DstMsgId', $newId));
- } catch (Syncroton_Exception_Status $e) {
+ } catch (Syncroton_Exception_Status_MoveItems $e) {
+ if ($this->_logger instanceof Zend_Log) {
+ $this->_logger->warn(__METHOD__ . '::' . __LINE__ . " Move failed: " . $e->getMessage());
+ }
$response->appendChild($this->_outputDom->createElementNS('uri:Move', 'Status', $e->getCode()));
} catch (Exception $e) {
- $response->appendChild($this->_outputDom->createElementNS('uri:Move', 'Status', Syncroton_Exception_Status::SERVER_ERROR));
+ if ($this->_logger instanceof Zend_Log) {
+ $this->_logger->warn(__METHOD__ . '::' . __LINE__ . " Move failed: " . $e->getMessage());
+ }
+ $response->appendChild($this->_outputDom->createElementNS('uri:Move', 'Status', Syncroton_Exception_Status_MoveItems::FOLDER_LOCKED));
}
}
diff --git a/lib/kolab_sync_storage.php b/lib/kolab_sync_storage.php
--- a/lib/kolab_sync_storage.php
+++ b/lib/kolab_sync_storage.php
@@ -1042,6 +1042,12 @@
// Use COPYUID feature (RFC2359) to get the new UID of the copied message
if (empty($this->storage->conn->data['COPYUID'])) {
+ // Check if the source item actually exists. Cyrus IMAP reports
+ // OK on a MOVE with an invalid UID, But COPYUID will be empty.
+ // This way we only incur the cost of the extra check once the move fails.
+ if (!$this->storage->get_message_headers($uid, $src_name)) {
+ throw new Syncroton_Exception_Status_MoveItems(Syncroton_Exception_Status_MoveItems::INVALID_SOURCE);
+ }
throw new Syncroton_Exception_Status(Syncroton_Exception_Status::SERVER_ERROR);
}
diff --git a/tests/Sync/MoveItemsTest.php b/tests/Sync/MoveItemsTest.php
--- a/tests/Sync/MoveItemsTest.php
+++ b/tests/Sync/MoveItemsTest.php
@@ -134,6 +134,64 @@
$this->assertSame('test sync', $xpath->query("ns:Commands/ns:Add/ns:ApplicationData/Email:Subject", $root)->item(0)->nodeValue);
}
+ public function testInvalidMove()
+ {
+ $this->emptyTestFolder('INBOX', 'mail');
+ $this->emptyTestFolder('Trash', 'mail');
+ $uid = $this->appendMail('INBOX', 'mail.sync1');
+ $this->registerDevice();
+ $inbox = array_search('INBOX', $this->folders);
+ $trash = array_search('Trash', $this->folders);
+
+ // Move item that doesn't exist
+ $request = <<<EOF
+ <?xml version="1.0" encoding="utf-8"?>
+ <!DOCTYPE AirSync PUBLIC "-//AIRSYNC//DTD AirSync//EN" "http://www.microsoft.com/">
+ <MoveItems xmlns="uri:Move">
+ <Move>
+ <SrcMsgId>foobar::99999</SrcMsgId>
+ <SrcFldId>foobar</SrcFldId>
+ <DstFldId>foobar</DstFldId>
+ </Move>
+ </MoveItems>
+ EOF;
+
+ $response = $this->request($request, 'MoveItems');
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $dom = $this->fromWbxml($response->getBody());
+ $xpath = $this->xpath($dom);
+ $xpath->registerNamespace('Move', 'uri:Move');
+
+ $root = $xpath->query("//Move:MoveItems/Move:Response")->item(0);
+ $this->assertSame('1', $xpath->query("Move:Status", $root)->item(0)->nodeValue);
+
+ // Move item that doesn't exist
+ $request = <<<EOF
+ <?xml version="1.0" encoding="utf-8"?>
+ <!DOCTYPE AirSync PUBLIC "-//AIRSYNC//DTD AirSync//EN" "http://www.microsoft.com/">
+ <MoveItems xmlns="uri:Move">
+ <Move>
+ <SrcMsgId>{$inbox}::99999</SrcMsgId>
+ <SrcFldId>{$inbox}</SrcFldId>
+ <DstFldId>{$trash}</DstFldId>
+ </Move>
+ </MoveItems>
+ EOF;
+
+ $response = $this->request($request, 'MoveItems');
+
+ $this->assertEquals(200, $response->getStatusCode());
+
+ $dom = $this->fromWbxml($response->getBody());
+ $xpath = $this->xpath($dom);
+ $xpath->registerNamespace('Move', 'uri:Move');
+
+ $root = $xpath->query("//Move:MoveItems/Move:Response")->item(0);
+ $this->assertSame('1', $xpath->query("Move:Status", $root)->item(0)->nodeValue);
+ }
+
/**
* Test moving a contact
*/

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 4, 10:40 PM (19 h, 37 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
18831457
Default Alt Text
D4950.1775342458.diff (6 KB)

Event Timeline