LegacyProtocol connects multiple times due to logic error

[zssarkany]

Describe the bug

LegacyProtocol connects multiple times due to logic error in LegacyProtocol::connected

Used config

Using package default with modification affecting only account host, username and password.

Code to Reproduce

First of all, prepend echo __METHOD__ . " CALLED\n"; line to the following methods in LegacyProtocol.php:

• __destruct
• connect
• connect
• login
• logout

<?php

use Webklex\PHPIMAP\ClientManager;

require_once __DIR__ . '/vendor/autoload.php';

$cm = new ClientManager(__DIR__ . '/config.php');

$client = $cm->account('default');

$client->connect();

for ($i = 1; $i <= 3; $i++) {
    echo "Test #$i\n";
    $client->getFolder('INBOX')->getStatus();
}

Output of the test code:

Test #1
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Test #2
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Test #3
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED

imap_open() is called every time, when login is called.

Expected behavior

Imap_open should be called only once.

Screenshots

n/a

Desktop / Server (please complete the following information):

• OS: Ubuntu 18.04.5
• PHP: 7.3.27-1+ubuntu18.04.1+deb.sury.org+1
• Version v2.5.0

Additional context

The root cause of the issue is in LegacyProtocol::connected method:

    public function connected(){
        return !$this->stream;
    }

The value of the returned statement is obviously faulty. This code should be refactored to:

    public function connected() {
        return boolval($this->stream);
    }

After this change, logout gets called multiple times (via Client and LegacyProtocol destructor), therefore logout should be fixed as well to avoid multiple imap_close invoke on the same resource identifier. The code below handles such case properly.

    public function logout() {
        if ($this->stream && \imap_close($this->stream, IMAP::CL_EXPUNGE)) {
            $this->stream = false;
            return true;
        }
        return false;
    }

After all the mentioned changes, the debug output becomes:

Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::connect CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::login CALLED
Test #1
Test #2
Test #3
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::__destruct CALLED
Webklex\PHPIMAP\Connection\Protocols\LegacyProtocol::logout CALLED

[zssarkany]

Please note, that new Client instance is created every time, therefore active_folder value of Client becomes null during Message instantiation, which at the end will result invalid Message operations.