Unable to retrieve SMTP server response error information after EHLO command in SMTP->hello() function

[fHYeemRLTX]

Problem description

The server receives the EHLO command.
The server rejects the input from the client by responding with a 421 error and breaking the connection.
The hello() function immediately tries to fallback with the HELO command, which fails, because the connection is gone.
The error information from the EHLO response is overwritten with the HELO failure.

Code to reproduce

require "SMTP.php";
$smtp = new PHPMailer\PHPMailer\SMTP();
$smtp->setDebugLevel(2);
$smtp->setDebugoutput("echo");
$smtp->connect("mx.tb.mail.iss.as9143.net", 25, 12);
var_dump($smtp->hello("hostname.that.does.not.match.ptr.record.in.dns"));
var_dump($smtp->getLastReply());
var_dump($smtp->getError());

Debug output

2020-11-06 14:23:47     SERVER -> CLIENT: 220 mx1.tb.mail.iss.as9143.net mx1.tb.mail.iss.as9143.net ESMTP server ready
2020-11-06 14:23:47     CLIENT -> SERVER: EHLO hostname.that.does.not.match.ptr.record.in.dns
2020-11-06 14:23:47     SERVER -> CLIENT: 421 EHLO MXIN201 Your HELO/EHLO hostname.that.does.not.match.ptr.record.in.dns is not matching your DNS configuration ...
2020-11-06 14:23:47     SMTP ERROR: EHLO command failed: 421 EHLO MXIN201 Your HELO/EHLO servername.that.does.not.match.ptr.record.in.dns is not matching your DNS configuration ...
2020-11-06 14:23:47     CLIENT -> SERVER: HELO hostname.that.does.not.match.ptr.record.in.dns
2020-11-06 14:23:47     SERVER -> CLIENT:
2020-11-06 14:23:47     SMTP ERROR: HELO command failed:
Command line code:1:
bool(false)
Command line code:1:
string(0) ""
Command line code:1:
array(4) {
  'error' =>
  string(19) "HELO command failed"
  'detail' =>
  bool(false)
  'smtp_code' =>
  int(0)
  'smtp_code_ex' =>
  NULL
}

Proposed solution

Add an option to the public function hello to skip the HELO command. By not executing the HELO command you will be able to see the correct error when connecting to mailservers that have this feature enabled.

    public function hello($host = '', $sendhelo = true)
    {
        if ($sendhelo)
        {
            //Try extended hello first (RFC 2821)
            return $this->sendHello('EHLO', $host) or $this->sendHello('HELO', $host);
        }
        else
        {
            return $this->sendHello('EHLO', $host);
        }
    }

[Synchro]

Well, it does show the issue clearly in debug output, however, suppressing the call to HELO manually is not an appropriate solution, not least because you don't know whether it's going to happen or not – the server could legitimately not support EHLO. Better to detect the 421 error and stop immediately, and not attempt the HELO because of that. That said, RFC5321 says that servers should only respond with 5xx codes for EHLO commands, so I suspect this is a non-standard response, so it's hard to recommend acting on slim evidence. For example, I can connect to gmail like this, just lying about who I am, and get away with it:

telnet smtp.gmail.com 25
Trying 173.194.76.109...
Connected to smtp.gmail.com.
Escape character is '^]'.
220 smtp.gmail.com ESMTP h128sm3045871wme.38 - gsmtp
EHLO apple.com
250-smtp.gmail.com at your service, [x.x.x.x]
250-SIZE 35882577
250-8BITMIME
250-STARTTLS
250-ENHANCEDSTATUSCODES
250-PIPELINING
250-CHUNKING
250 SMTPUTF8
QUIT
221 2.0.0 closing connection h128sm3045871wme.38 - gsmtp

[fHYeemRLTX]

Good point, I didn't consider the RFCs.

The 421 response code is mentioned as a valid EHLO response in RFC1869.
It is allowed in RFC5321 after the server receives any command.

This should fix the issue while complying with the RFCs and keeping the fallback functionality in tact:

    public function hello($host = '')
    {
        //Try extended hello first (RFC 2821)
        if ($this->sendHello('EHLO', $host))
        {
            return true;
        }

        //Some servers shut down the SMTP service here (RFC 5321)
        if (substr($this->helo_rply, 0, 3) == '421')
        {
            return false;
        }

        return $this->sendHello('HELO', $host);
    }

[Synchro]

The sendHello method already checks the status code of the command for both HELO and EHLO, and records an error if it's anything other than 250, however if EHLO fails, it will always attempt HELO regardless of why EHLO failed. It will return false if they both fail, exactly as your example does. The output of each of them is recorded in helo_reply, so I'm not sure what else is needed here.

[Synchro]

Ignore that last comment – your fix is correct, so I've applied it. Thanks!