From 34158407b2ef8131b9a4223439137f5834879c99 Mon Sep 17 00:00:00 2001 From: mark_story Date: Mon, 13 Feb 2017 22:37:44 -0500 Subject: [PATCH] Exit early if SMTP connection fails. If the SMTP connection is disconnected (read() returns false) we should exit early and not wait for the read timeout. This has the added benefit of making the mocks much simpler. Refs #10221 --- lib/Cake/Network/Email/SmtpTransport.php | 6 +- .../Case/Network/Email/SmtpTransportTest.php | 176 +++++++----------- 2 files changed, 75 insertions(+), 107 deletions(-) diff --git a/lib/Cake/Network/Email/SmtpTransport.php b/lib/Cake/Network/Email/SmtpTransport.php index b0761d496..6e5e6f959 100644 --- a/lib/Cake/Network/Email/SmtpTransport.php +++ b/lib/Cake/Network/Email/SmtpTransport.php @@ -352,7 +352,11 @@ class SmtpTransport extends AbstractTransport { $response = ''; $startTime = time(); while (substr($response, -2) !== "\r\n" && ((time() - $startTime) < $this->_config['timeout'])) { - $response .= $this->_socket->read(); + $bytes = $this->_socket->read(); + if ($bytes === false || $bytes === null) { + break; + } + $response .= $bytes; } if (substr($response, -2) !== "\r\n") { throw new SocketException(__d('cake_dev', 'SMTP timeout.')); diff --git a/lib/Cake/Test/Case/Network/Email/SmtpTransportTest.php b/lib/Cake/Test/Case/Network/Email/SmtpTransportTest.php index 712ea54ae..ef5f75b5e 100644 --- a/lib/Cake/Test/Case/Network/Email/SmtpTransportTest.php +++ b/lib/Cake/Test/Case/Network/Email/SmtpTransportTest.php @@ -83,11 +83,13 @@ class SmtpTransportTest extends CakeTestCase { */ public function testConnectEhlo() { $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); - $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->any()) + ->method('read') + ->will($this->onConsecutiveCalls( + "220 Welcome message\r\n", + "250 Accepted\r\n" + )); + $this->socket->expects($this->once())->method('write')->with("EHLO localhost\r\n"); $this->SmtpTransport->connect(); } @@ -99,18 +101,14 @@ class SmtpTransportTest extends CakeTestCase { public function testConnectEhloTls() { $this->SmtpTransport->config(array('tls' => true)); $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250 Accepted\r\n")); - $this->socket->expects($this->at(5))->method('write')->with("STARTTLS\r\n"); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("220 Server ready\r\n")); - $this->socket->expects($this->at(8))->method('enableCrypto')->with('tls')->will($this->returnValue(true)); - $this->socket->expects($this->at(9))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(10))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(11))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("STARTTLS\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("220 Server ready\r\n")); + $this->socket->expects($this->at(6))->method('enableCrypto')->with('tls')->will($this->returnValue(true)); + $this->socket->expects($this->at(7))->method('write')->with("EHLO localhost\r\n"); + $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("250 Accepted\r\n")); $this->SmtpTransport->connect(); } @@ -124,14 +122,11 @@ class SmtpTransportTest extends CakeTestCase { public function testConnectEhloTlsOnNonTlsServer() { $this->SmtpTransport->config(array('tls' => true)); $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250 Accepted\r\n")); - $this->socket->expects($this->at(5))->method('write')->with("STARTTLS\r\n"); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("500 5.3.3 Unrecognized command\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("STARTTLS\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("500 5.3.3 Unrecognized command\r\n")); $this->SmtpTransport->connect(); } @@ -145,14 +140,11 @@ class SmtpTransportTest extends CakeTestCase { public function testConnectEhloNoTlsOnRequiredTlsServer() { $this->SmtpTransport->config(array('tls' => false, 'username' => 'user', 'password' => 'pass')); $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250 Accepted\r\n")); - $this->socket->expects($this->at(5))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized authentication type\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("AUTH LOGIN\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("504 5.7.4 Unrecognized authentication type\r\n")); $this->SmtpTransport->connect(); $this->SmtpTransport->auth(); } @@ -164,14 +156,11 @@ class SmtpTransportTest extends CakeTestCase { */ public function testConnectHelo() { $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); - $this->socket->expects($this->at(5))->method('write')->with("HELO localhost\r\n"); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("250 Accepted\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("HELO localhost\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250 Accepted\r\n")); $this->SmtpTransport->connect(); } @@ -184,14 +173,11 @@ class SmtpTransportTest extends CakeTestCase { */ public function testConnectFail() { $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); - $this->socket->expects($this->at(5))->method('write')->with("HELO localhost\r\n"); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("HELO localhost\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("200 Not Accepted\r\n")); $this->SmtpTransport->connect(); } @@ -202,14 +188,11 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuth() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("334 Login\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("bWFyaw==\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("334 Pass\r\n")); - $this->socket->expects($this->at(6))->method('write')->with("c3Rvcnk=\r\n"); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("235 OK\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Pass\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("c3Rvcnk=\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("235 OK\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -223,8 +206,7 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuthNotRecognized() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("500 5.3.3 Unrecognized command\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("500 5.3.3 Unrecognized command\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -238,8 +220,8 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuthNotImplemented() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("502 5.3.3 Command not implemented\r\n")); + $this->socket->expects($this->at(1))->method('read') + ->will($this->returnValue("502 5.3.3 Command not implemented\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -253,8 +235,8 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuthBadSequence() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("503 5.5.1 Already authenticated\r\n")); + $this->socket->expects($this->at(1))->method('read') + ->will($this->returnValue("503 5.5.1 Already authenticated\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -268,11 +250,9 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuthBadUsername() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("334 Login\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("bWFyaw==\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -286,14 +266,11 @@ class SmtpTransportTest extends CakeTestCase { */ public function testAuthBadPassword() { $this->socket->expects($this->at(0))->method('write')->with("AUTH LOGIN\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("334 Login\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("bWFyaw==\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("334 Pass\r\n")); - $this->socket->expects($this->at(6))->method('write')->with("c3Rvcnk=\r\n"); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("334 Login\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("bWFyaw==\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("334 Pass\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("c3Rvcnk=\r\n"); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("535 5.7.8 Authentication failed\r\n")); $this->SmtpTransport->config(array('username' => 'mark', 'password' => 'story')); $this->SmtpTransport->auth(); } @@ -323,20 +300,15 @@ class SmtpTransportTest extends CakeTestCase { $email->cc(array('mark@cakephp.org' => 'Mark Story', 'juan@cakephp.org' => 'Juan Basso')); $this->socket->expects($this->at(0))->method('write')->with("MAIL FROM:\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("RCPT TO:\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(4))->method('write')->with("RCPT TO:\r\n"); $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(6))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(9))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(10))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(11))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(12))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(13))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(14))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(6))->method('write')->with("RCPT TO:\r\n"); + $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(8))->method('write')->with("RCPT TO:\r\n"); + $this->socket->expects($this->at(9))->method('read')->will($this->returnValue("250 OK\r\n")); $this->SmtpTransport->sendRcpt($email); } @@ -353,11 +325,9 @@ class SmtpTransportTest extends CakeTestCase { $email->returnPath('pleasereply@cakephp.org', 'CakePHP Return'); $this->socket->expects($this->at(0))->method('write')->with("MAIL FROM:\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("RCPT TO:\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n")); $this->SmtpTransport->sendRcpt($email); } @@ -398,11 +368,9 @@ class SmtpTransportTest extends CakeTestCase { $data .= "\r\n\r\n.\r\n"; $this->socket->expects($this->at(0))->method('write')->with("DATA\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("354 OK\r\n")); - $this->socket->expects($this->at(3))->method('write')->with($data); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("354 OK\r\n")); + $this->socket->expects($this->at(2))->method('write')->with($data); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n")); $this->SmtpTransport->sendData($email); } @@ -443,20 +411,18 @@ class SmtpTransportTest extends CakeTestCase { $this->assertEmpty($this->SmtpTransport->getLastResponse()); $this->socket->expects($this->any())->method('connect')->will($this->returnValue(true)); - $this->socket->expects($this->at(0))->method('read')->will($this->returnValue(false)); $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("220 Welcome message\r\n")); $this->socket->expects($this->at(2))->method('write')->with("EHLO localhost\r\n"); - $this->socket->expects($this->at(3))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250-PIPELINING\r\n")); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250-SIZE 102400000\r\n")); - $this->socket->expects($this->at(6))->method('read')->will($this->returnValue("250-VRFY\r\n")); - $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("250-ETRN\r\n")); - $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("250-STARTTLS\r\n")); - $this->socket->expects($this->at(9))->method('read')->will($this->returnValue("250-AUTH PLAIN LOGIN\r\n")); - $this->socket->expects($this->at(10))->method('read')->will($this->returnValue("250-AUTH=PLAIN LOGIN\r\n")); - $this->socket->expects($this->at(11))->method('read')->will($this->returnValue("250-ENHANCEDSTATUSCODES\r\n")); - $this->socket->expects($this->at(12))->method('read')->will($this->returnValue("250-8BITMIME\r\n")); - $this->socket->expects($this->at(13))->method('read')->will($this->returnValue("250 DSN\r\n")); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250-PIPELINING\r\n")); + $this->socket->expects($this->at(4))->method('read')->will($this->returnValue("250-SIZE 102400000\r\n")); + $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250-VRFY\r\n")); + $this->socket->expects($this->at(6))->method('read')->will($this->returnValue("250-ETRN\r\n")); + $this->socket->expects($this->at(7))->method('read')->will($this->returnValue("250-STARTTLS\r\n")); + $this->socket->expects($this->at(8))->method('read')->will($this->returnValue("250-AUTH PLAIN LOGIN\r\n")); + $this->socket->expects($this->at(9))->method('read')->will($this->returnValue("250-AUTH=PLAIN LOGIN\r\n")); + $this->socket->expects($this->at(10))->method('read')->will($this->returnValue("250-ENHANCEDSTATUSCODES\r\n")); + $this->socket->expects($this->at(11))->method('read')->will($this->returnValue("250-8BITMIME\r\n")); + $this->socket->expects($this->at(12))->method('read')->will($this->returnValue("250 DSN\r\n")); $this->SmtpTransport->connect(); $expected = array( @@ -479,11 +445,9 @@ class SmtpTransportTest extends CakeTestCase { $email->to('cake@cakephp.org', 'CakePHP'); $this->socket->expects($this->at(0))->method('write')->with("MAIL FROM:\r\n"); - $this->socket->expects($this->at(1))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(2))->method('read')->will($this->returnValue("250 OK\r\n")); - $this->socket->expects($this->at(3))->method('write')->with("RCPT TO:\r\n"); - $this->socket->expects($this->at(4))->method('read')->will($this->returnValue(false)); - $this->socket->expects($this->at(5))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(1))->method('read')->will($this->returnValue("250 OK\r\n")); + $this->socket->expects($this->at(2))->method('write')->with("RCPT TO:\r\n"); + $this->socket->expects($this->at(3))->method('read')->will($this->returnValue("250 OK\r\n")); $this->SmtpTransport->sendRcpt($email);