[Mailer] Fix edge cases in STMP transports

fabpot · fabpot · commit a570e6322f01 · 2022-08-27T08:23:51.000+02:00
diff --git a/src/Symfony/Component/Mailer/Tests/Transport/Smtp/SmtpTransportTest.php b/src/Symfony/Component/Mailer/Tests/Transport/Smtp/SmtpTransportTest.php
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Mailer\Envelope;
+use Symfony\Component\Mailer\Exception\LogicException;
 use Symfony\Component\Mailer\Exception\TransportException;
 use Symfony\Component\Mailer\Transport\Smtp\SmtpTransport;
 use Symfony\Component\Mailer\Transport\Smtp\Stream\AbstractStream;
@@ -133,6 +134,37 @@ public function testWriteEncodedRecipientAndSenderAddresses()
         $this->assertContains("RCPT TO:<recipient@xn--exmple-cua.org>\r\n", $stream->getCommands());
         $this->assertContains("RCPT TO:<recipient2@example.org>\r\n", $stream->getCommands());
     }
+
+    public function testAssertResponseCodeNoCodes()
+    {
+        $this->expectException(LogicException::class);
+        $this->invokeAssertResponseCode('response', []);
+    }
+
+    public function testAssertResponseCodeWithEmptyResponse()
+    {
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Expected response code "220" but got empty code.');
+        $this->invokeAssertResponseCode('', [220]);
+    }
+
+    public function testAssertResponseCodeWithNotValidCode()
+    {
+        $this->expectException(TransportException::class);
+        $this->expectExceptionMessage('Expected response code "220" but got code "550", with message "550 Access Denied".');
+        $this->expectExceptionCode(550);
+        $this->invokeAssertResponseCode('550 Access Denied', [220]);
+    }
+
+    private function invokeAssertResponseCode(string $response, array $codes): void
+    {
+
+        $transport = new SmtpTransport($this->getMockForAbstractClass(AbstractStream::class));
+        $m = new \ReflectionMethod($transport, 'assertResponseCode');
+        $m->setAccessible(true);
+
+        $m->invoke($transport, $response, $codes);
+    }
 }
 
 class DummyStream extends AbstractStream
diff --git a/src/Symfony/Component/Mailer/Transport/Smtp/EsmtpTransport.php b/src/Symfony/Component/Mailer/Transport/Smtp/EsmtpTransport.php
@@ -94,7 +94,9 @@ public function addAuthenticator(AuthenticatorInterface $authenticator): void
 
     protected function doHeloCommand(): void
     {
-        $capabilities = $this->callHeloCommand();
+        if (!$capabilities = $this->callHeloCommand()) {
+            return;
+        }
 
         /** @var SocketStream $stream */
         $stream = $this->getStream();
@@ -123,6 +125,8 @@ private function callHeloCommand(): array
         } catch (TransportExceptionInterface $e) {
             try {
                 parent::doHeloCommand();
+
+                return [];
             } catch (TransportExceptionInterface $ex) {
                 if (!$ex->getCode()) {
                     throw $e;
diff --git a/src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php b/src/Symfony/Component/Mailer/Transport/Smtp/SmtpTransport.php
@@ -301,7 +301,8 @@ private function assertResponseCode(string $response, array $codes): void
         if (!$valid || !$response) {
             $codeStr = $code ? sprintf('code "%s"', $code) : 'empty code';
             $responseStr = $response ? sprintf(', with message "%s"', trim($response)) : '';
-            throw new TransportException(sprintf('Expected response code "%s" but got ', implode('/', $codes), $codeStr).$codeStr.$responseStr.'.', $code);
+
+            throw new TransportException(sprintf('Expected response code "%s" but got ', implode('/', $codes)).$codeStr.$responseStr.'.', $code ?: 0);
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -301,7 +301,8 @@ private function assertResponseCode(string $response, array $codes): void`
`301`	`301`	`if (!$valid \|\| !$response) {`
`302`	`302`	`$codeStr = $code ? sprintf('code "%s"', $code) : 'empty code';`
`303`	`303`	`$responseStr = $response ? sprintf(', with message "%s"', trim($response)) : '';`
`304`		`- throw new TransportException(sprintf('Expected response code "%s" but got ', implode('/', $codes), $codeStr).$codeStr.$responseStr.'.', $code);`
	`304`	`+`
	`305`	`+ throw new TransportException(sprintf('Expected response code "%s" but got ', implode('/', $codes)).$codeStr.$responseStr.'.', $code ?: 0);`
`305`	`306`	`}`
`306`	`307`	`}`
`307`	`308`