Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving a Mail removes original and creates a copy of another, old message #29

Closed
DasTobbel opened this issue Oct 14, 2020 · 5 comments
Closed
Labels
bug Something isn't working validated

Comments

@DasTobbel
Copy link
Contributor

Describe the bug
While using the $message->move("someFolder, true) method i saw that the mail i wanted to move is deleted and in the target folder a old mail is duplicated.

To Reproduce
Steps to reproduce the behavior:
I realy dont know if you can reproduce this, or if thats a "messed up" mail server ...

  1. recieve a new mail , we call it $message
  2. Move $message it to ( in my case nested ) folder: from INBOX to my/target
  3. Origin Message is gone, and another message has been duplicated.

Expected behavior
see Additional for context
The correct "next_uid" should be picked and actually copy the origin mail to the folder insted of copying another mail inside the target folder... Or at least thats what i think should happen, dont know the "COPY" command of imap - need to check it

Desktop / Server (please complete the following information):

  • OS: ubuntu 20
  • PHP: 7.4
  • Version 2.1.12

Additional context

My logging:

[somewhere in my mail handling job]
\Log::debug(print_r(['origin message ' => $message], true)); //[0]
$message->move('my/target');

src/Message.php :661

    public function copy($folder) {
        $this->client->openFolder($this->folder_path);
        \Log::debug(print_r(['copy status of inbox ' => $this->client->getConnection()->examineFolder($this->folder_path)], true)); //[1]
        $status = $this->client->getConnection()->examineFolder($folder);
        /** @var Folder $folder */
        $folder = $this->client->getFolder($folder);
        if (isset($status["uidnext"]) && $folder !== null) {
            \Log::debug(print_r(['copy status' => $status], true)); //[2]
            \Log::debug(print_r(['uid' => $this->uid], true)); //[3]
            $next_uid = $status["uidnext"];
            if ($this->client->getConnection()->copyMessage($folder->path, $this->msgn) == true) {
                $this->client->openFolder($folder->path);
                $message_num = $this->client->getConnection()->getMessageNumber($next_uid);
                \Log::debug(print_r(['message_num' => $message_num], true)); //[4]
                $message = $folder->query()->getMessage($message_num);
                $event = $this->getEvent("message", "copied");
                $event::dispatch($this, $message);
                \Log::debug(print_r(['new message ' => $message], true)); //[6]
                return $message;
            }
        }

        return null;
    }

src/Connection/Protocols/ImapProtocol.php:799

    public function copyMessage($folder, $from, $to = null) {
        $set = (int)$from;
        \Log::debug(print_r(['folder' => $folder, 'from' => $from,'to' => $to], true)); //[5]
        if ($to !== null) {
            $set .= ':' . ($to == INF ? '*' : (int)$to);
        }

        return $this->requestAndResponse('COPY', [$set, $this->escapeString($folder)], true);
    }
[0] production.DEBUG: Array( [origin message ] => Webklex\PHPIMAP\Message Object (
            [...]
            [attributes:protected] => Array
                (
                    [message_no] =>
                    [msgn] => 1
                    [msglist] => 0
                    [uid] => 726
                )
            [folder_path:protected] => INBOX
            [...]
            [header] => Webklex\PHPIMAP\Header Object (
            ...
            Message-ID: <[email protected]>
            ...
            )

[1] production.DEBUG: Array( [copy status of inbox ] => Array (
            [...]
            [exists] => 1
            [recent] => 0
            [uidvalidity] => 1597831262
            [uidnext] => 727 
        ))
[2] production.DEBUG: Array( [copy status] => Array (
            [...]
            [exists] => 154
            [recent] => 0
            [uidvalidity] => 1597831265
            [uidnext] => 156
        ))
[3] production.DEBUG: Array( [uid] => 726 )

[4] production.DEBUG: Array([message_num] => 155)

[5] production.DEBUG: Array( [folder] => my/target, [from] => 1, [to] => )

[6] production.DEBUG: Array( [move message copy] => Webklex\PHPIMAP\Message Object (
            [...]
            [attributes:protected] => Array
                (
                    [message_no] =>
                    [msgn] => 155
                    [msglist] =>
                    [uid] => 156
                )
            )
            [folder_path:protected] => my/target
            [...]
            [header] => Webklex\PHPIMAP\Header Object (
            ...
            Message-ID: <trinity-debbb0f2-9c44-4368-9839-32d5e77161d7-1598022653825@msvc-mesg-gmx123>
            ...
            )
@Webklex Webklex added bug Something isn't working validating labels Oct 14, 2020
@DasTobbel
Copy link
Contributor Author

Okay, tried it on another mail server and got the same behavior...
Tried to log it in a somehow readable format
It looks like the step where the next uid is set may be faulty for some servers?
Message.php:copy()

// message uid is = 2800
// Folder *INBOX* has 'uidnext' = 2801
// Folder *my/target* has 'uidnext' = 496
$next_uid = $status["uidnext"];
[...]
$message_num = $this->client->getConnection()->getMessageNumber($next_uid);
//after this step the imap server responds with another message

this results in a Message.php:delete() of the "origin" message, and a carbon-copy of the $next_uid message inside the target folder...

I have no idea why this happens, my best guess is that my mailservers behave somehow different than yours, so the uid_next is not set while copying a message? I have no clue at all 😢

@DasTobbel
Copy link
Contributor Author

I can move mails with this, recieveing the copy still fails...:

ImapProtocol.php added this function

    public function moveTobbel($folder, $from, $to = null) {
        \Log::debug(print_r(['folder' => $folder, 'from' => $from,'to' => $to], true));
        $set = (int)$from;
        if ($to !== null) {
            $set .= ':' . ($to == INF ? '*' : (int)$to);
        }
        \Log::debug(print_r(['set' => $set], true));
        \Log::debug(print_r(['requestAndResponse' => ['MOVE', [$set, $this->escapeString($folder)], true]], true));
        return $this->requestAndResponse('MOVE', [$set, $this->escapeString($folder)], true);
    }

Message.php added also a moveTobbel and altered the move

    public function moveTobbel($folder) {
        $folder = $this->client->getFolder($folder);
        $copy = $this->client->getConnection()->moveTobbel($folder->path, $this->msgn);
        \Log::debug(print_r(['move tobbel' => $copy], true));
        if (is_array($copy) && (stripos($copy[0], "ok")) !== false) {
                //preg_match('/.*OK.*\[(.*)\]/i', $copy[0], $matches);
                preg_match('/.*OK.*\[(.*) (\d+)\]/i', $copy[0], $matches);
                $response = $matches[2];
                \Log::debug(print_r(['move tobbel - response' => $response], true));
                $message_num = $this->client->getConnection()->getUid($response);
                \Log::debug(print_r(['message_num' => $message_num], true));
                $message = $folder->query()->getMessage($message_num);
                // ------------ THIS FAILS *
                $event = $this->getEvent("message", "copied");
                $event::dispatch($this, $message);
                return $message;
        }
        return $copy;
    }
   
    public function move($folder, $expunge = false) {
        \Log::debug(print_r([ "message before copy"  => ['message_no' => $this->message_no, 'msgn' => $this->msgn, 'msglist' => $this->msglist, 'uid' => $this->uid, 'folder_path' => $this->folder_path, 'message_id' => $this->header->message_id]], true));

        $message = $this->moveTobbel($folder);
        \Log::debug(print_r([ "message  after moveTobbel" => $message], true));
         return $message;
}

*: in my target folder are about 140 messages, but the function getMessageNumber->getUid just fetches (exactly) 40 messages/uids - and that list does not contain my moved UID ... So the Fetch of that new message number fails ....
Very strange.... my imap server is an dovecot, version 2.3.11.3 , never had any issues

@Webklex
Copy link
Owner

Webklex commented Oct 16, 2020

Hi @DasTobbel ,
thanks for your detailed report. I was able to reproduce this and I think I found a solution. Please try to replace the Message::copy() method with this:

public function copy($folder_path) {
    $this->client->openFolder($folder_path);
    $status = $this->client->getConnection()->examineFolder($folder_path);

    if (isset($status["uidnext"])) {
        $next_uid = $status["uidnext"];

        /** @var Folder $folder */
        $folder = $this->client->getFolder($folder_path);

        $this->client->openFolder($this->folder_path);
        if ($this->client->getConnection()->copyMessage($folder->path, $this->msgn) == true) {
            $this->client->expunge();

            $this->client->openFolder($folder->path);
            $message_num = $this->client->getConnection()->getMessageNumber($next_uid);

            $message = $folder->query()->getMessage($message_num);
            $event = $this->getEvent("message", "copied");
            $event::dispatch($this, $message);

            return $message;
        }
    }

    return null;
}

..and try again.

Best regards,

@Webklex
Copy link
Owner

Webklex commented Oct 16, 2020

P.s.: you are correct there might also be a MOVE command available as specified in rfc6851.

Adding the following in \Webklex\PHPIMAP\Connection\Protocols\ImapProtocol:

public function moveMessage($folder, $from, $to = null) {
    $set = (int)$from;
    if ($to !== null) {
        $set .= ':' . ($to == INF ? '*' : (int)$to);
    }

    return $this->requestAndResponse('MOVE', [$set, $this->escapeString($folder)], true);
}

.. and using it directly should also work. The problem seemed to be that Client::getFolders() which gets called through Client::getFolder() can reset the current session "scope". If this happens the requested msgn / uid can no longer be located - fixed by opening the required folder again. This doesn't break, even if the folder is already open.

@DasTobbel
Copy link
Contributor Author

DasTobbel commented Oct 16, 2020

Hi @Webklex
can confirm that change works on my test - also getting the copied message as a return of the function - so now it works as intended. Nice work man!

The problem seemed to be that Client::getFolders() which gets called through Client::getFolder() can reset the current session "scope". If this happens the requested msgn / uid can no longer be located - fixed by opening the required folder again.

I looked a bit into the rfc3501 but i did not find any hint or function suggestion pointing to that - but, if its working i'm not going to blame anyone ;-)

Thank you for your effort and nice work with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working validated
Projects
None yet
Development

No branches or pull requests

2 participants