nextLine() hangs (infinite loop)

[sergiy-petrov]

Describe the bug When i try to search all folders using $client->getFolders() method the script just gets into infinite loop. I tried to debug it by myself and find out that when reading next char of the stream always returning an empty string.

This happens right after i received ' BYE Connection closed.' After closing connection i expect to receive 'logout' or something like that, but instead the script just hangs infinitely.

Used config

$cm = new ClientManager(['options' => [
            'debug' => true,
        ]]);

        $client = $cm->make([
            'host'           => 'outlook.office365.com',
            'port'           => 993,
            'encryption'     => 'ssl',
            'validate_cert'  => true,
            'protocol'       => 'imap',
            'username'       => $username,
            'password'       => $token,
            'authentication' => 'oauth',
        ]);

Code to Reproduce The troubling code section which produces the reported bug.

\Webklex\PHPIMAP\Connection\Protocols\ImapProtocol::nextLine()

    public function nextLine(): string {
        $line = "";
        while (($next_char = fread($this->stream, 1)) !== false && $next_char !== "\n") {
           dump($next_char); // i added this by myself to debug - always return empty string (screenshot below)
            $line .= $next_char; // always return empty string ''
        }
        if ($line === "") {
            throw new RuntimeException('empty response');
        }
        if ($this->debug) echo "<< ".$line."\n";
        return $line . "\n";
    }

Expected behavior

Hard to say what is expected result, but i assume this should be handled somehow.

Screenshots

Desktop / Server (please complete the following information):

• OS: [Ubuntu]
• PHP: [8.0.23] (cli)
• Version [v4.0.2]
• Provider [Outlook]

[AzzaAzza69]

I would personally use stream_get_line() as it will be more efficient and just check the resulting $line===false for an empty response as a $line="" indicates an empty line (stream_get_line doesn't include the cr)

[thin-k-design]

I have the same issue, but only from time to time. I pinpointed the infitnite loop to the while loop in the ImapProtocol::nextLine() method as @sergiy-petrov described. I'm provisionally doing a check to see if there are other problematic chars:

    public function nextLine(): string {
        $line = "";
        $beginningTime = time();
        while (($next_char = fread($this->stream, 1)) !== false && !in_array($next_char, ["","\n"])) {
            $line .= $next_char;
            if (time() > $beginningTime + 30) {
                throw new RuntimeException('reading line for longer than 30s (last char was "'.$next_char.'"): '.$line);
            }
        }
        if ($line === "") {
            throw new RuntimeException('empty response');
        }
        if ($this->debug) echo "<< ".$line."\n";
        return $line . "\n";
    }

[sergiy-petrov]

@thin-k-design yeah for me it also happens from time to time

[laurent-rizer]

I have the same issue, our script sometimes timeouts while reading next line. Since this occurs quite randomly for us, we added a && !feof($this->stream) to the while condition, and we'll see if that makes the trick.

[thin-k-design]

@laurent-rizer not sure if your solution will work, fread() and feof() should behave the same, I think. changing while (($next_char = fread($this->stream, 1)) !== false && $next_char !== "\n") { to while (($next_char = fread($this->stream, 1)) !== false && !in_array($next_char, ["","\n"])) { has worked for me. Since then I haven't had any more issues.

See my pull request

[laurent-rizer]

@thin-k-design thx, I'll see if feof does change anything in the next days and if not I'll test your solution

[thin-k-design]

I got the next problem, I got again an endless loop, but this time over 500.000 times a PHP Warning was written to the logs: "fread(): SSL: An existing connection was forcibly closed by the remote host.".

I am wondering, wouldn't it be easier just to use fgets? Are there any downsides I am not aware of?

    public function nextLine(): string {
        $line = fgets($this->stream);
        if ($line === false) {
            throw new RuntimeException('no response');
        }
        if ($this->debug) echo "<< ".$line;
        return $line;
    }

[laurent-rizer]

@thin-k-design I went for the following which partially resolves the issue:

    public function nextLine(): string {
        $line = "";
        while (($next_char = fread($this->stream, 1)) !== false && $next_char !== "\n" && !feof($this->stream)) {
            $line .= $next_char;
        }
        if (
            $line === "" &&
            ($next_char === false || feof($this->stream))
        ) {
            throw new RuntimeException('empty response');
        }
        if ($this->debug) echo "<< ".$line."\n";
        return $line . "\n";
    }

I said partially since we now from time to time get the following error: Warning: fwrite(): SSL operation failed with code 1. OpenSSL Error messages: error:1420C0CF:SSL routines:ssl_write_internal:protocol is shutdown

For us this seems to always happen after a ImapProtocol::logout() I need to dig deeper in our code, but I wonder if we don't try to re-connect after an not properly closed pre-existing connection.

[laurent-rizer]

Concerning fgets that was the case a few versions before

[thin-k-design]

Concerning fgets that was the case a few versions before

Do you know what the problem was? I have changed my code to use fgets and seems to work pretty fine (in my case mostly Microsoft and Gmail mailboxes).

[laurent-rizer]

I have no clue Blame (https://github.com/Webklex/php-imap/blame/master/src/Connection/Protocols/ImapProtocol.php) gives the following commit: https://github.com/Webklex/php-imap/commit/f0fcdb728ed8285789e7a3d0cca96f7f93a30ec5

Which doesn't gives more information than "IMAP Connection debugging improved"

[thin-k-design]

If the change was really only for debugging purposes, we should revert to fgets(). There is no advantage (as I understand it) in reading character after character for debugging. fgets() is much simpler and the while loop is buggy. @Webklex?