diff --git a/src/Mailer.php b/src/Mailer.php index eab1b6d..f09b132 100644 --- a/src/Mailer.php +++ b/src/Mailer.php @@ -67,6 +67,18 @@ public function setServer($host, $port, $secure=null) return $this; } + /** + * set the socket timeout in seconds, applied to both connecting and + * to each read/write while talking to the server + * @param int $timeout + * @return $this + */ + public function setTimeout($timeout) + { + $this->smtp->setTimeout($timeout); + return $this; + } + /** * auth with server * @param string $username diff --git a/src/Mailer/SMTP.php b/src/Mailer/SMTP.php index 50bc3c1..65c697a 100644 --- a/src/Mailer/SMTP.php +++ b/src/Mailer/SMTP.php @@ -41,6 +41,11 @@ class SMTP */ protected $port; + /** + * socket timeout in seconds (connect + read/write) + */ + protected $timeout; + /** * smtp secure ssl tls tlsv1.0 tlsv1.1 tlsv1.2 */ @@ -102,6 +107,7 @@ class SMTP public function __construct(LoggerInterface $logger=null) { $this->logger = $logger; + $this->timeout = (int) ini_get('default_socket_timeout'); } /** @@ -160,6 +166,18 @@ public function setEhlo($ehlo) return $this; } + /** + * set the socket timeout in seconds, applied to both connecting and + * to each read/write while talking to the server + * @param int $timeout + * @return $this + */ + public function setTimeout($timeout) + { + $this->timeout = (int) $timeout; + return $this; + } + /** * Send the message * @@ -219,13 +237,14 @@ protected function connect() $host.':'.$this->port, $error_code, $error_message, - ini_get('default_socket_timeout'), + $this->timeout, STREAM_CLIENT_CONNECT, $context ); if (!$this->smtp){ throw new SMTPException("Could not open SMTP Port to $host:{$this->port}"); } + stream_set_timeout($this->smtp, $this->timeout); $code = $this->getCode(); if ($code !== '220'){ throw new CodeException('220', $code, array_pop($this->resultStack)); @@ -449,28 +468,81 @@ protected function quit() protected function pushStack($string) { $this->commandStack[] = $string; - fputs($this->smtp, $string, strlen($string)); + if (fputs($this->smtp, $string, strlen($string)) === false) { + throw new SMTPException("Failed to write to SMTP socket"); + } $this->logger && $this->logger->debug('Sent: '. $string); return $this->getCode(); } /** - * get smtp response code - * once time has three digital and a space - * @return string - * @throws SMTPException + * Read one complete SMTP reply and return its 3-digit status code. + * + * An SMTP reply may span several lines (RFC 5321 §4.2.1): continuation + * lines have the form "NNN-text", the final line "NNN text" (or "NNN"). + * All lines are read and reassembled into a single reply, which is pushed + * as one entry onto $this->resultStack -- so it stays aligned with + * $this->commandStack (one result per command) and CodeException can + * surface the whole reply, not just its last line. + * + * @return string the 3-digit reply code + * @throws SMTPException on a closed/timed-out connection or an unparseable reply */ protected function getCode() { - while ($str = fgets($this->smtp, 515)) { - $this->logger && $this->logger->debug("Got: ". $str); - $this->resultStack[] = $str; - if(substr($str,3,1) == " ") { - $code = substr($str,0,3); - return $code; + $code = null; + $reply = ''; + + do { + $line = $this->readLine(); + if ($line === null) { + $meta = stream_get_meta_data($this->smtp); + if (!empty($meta['timed_out'])) { + throw new SMTPException("SMTP read timed out after {$this->timeout} seconds"); + } + throw new SMTPException("SMTP Server did not respond with anything I recognized"); + } + + $this->logger && $this->logger->debug("Got: " . $line); + $reply .= $line; + + // RFC 5321 §4.2.1: "NNN " (or bare "NNN") is the final line, + // "NNN-" marks a continuation line that must be followed by more. + if (!preg_match('/^(\d{3})([ -]?)/', $line, $m)) { + $this->resultStack[] = $reply; + throw new SMTPException("SMTP Server did not respond with anything I recognized"); + } + + $code = $m[1]; + $separator = $m[2]; + } while ($separator === '-'); + + $this->resultStack[] = $reply; + + return $code; + } + + /** + * Read a single complete line (terminated by LF) from the socket, + * regardless of how long it is or how the OS chunks it. Reassembling the + * whole line here is what keeps a long reply from leaving a tail in the + * buffer that would desync the next getCode() call. + * + * @return string|null the line (including its trailing CRLF), or null on EOF/timeout + */ + protected function readLine() + { + $line = ''; + while (true) { + $chunk = fgets($this->smtp, 1024); + if ($chunk === false) { + return ($line === '') ? null : $line; // EOF / timeout + } + $line .= $chunk; + if (substr($line, -1) === "\n") { + return $line; // got the full line } } - throw new SMTPException("SMTP Server did not respond with anything I recognized"); } } diff --git a/tests/SMTPParserTest.php b/tests/SMTPParserTest.php new file mode 100644 index 0000000..450deb6 --- /dev/null +++ b/tests/SMTPParserTest.php @@ -0,0 +1,168 @@ +setAccessible(true); + $getCode->setAccessible(true); + $stack->setAccessible(true); + } + + $prop->setValue($smtp, $fp); + + $invoke = function () use ($getCode, $smtp) { + return $getCode->invoke($smtp); + }; + $resultStack = function () use ($stack, $smtp) { + return $stack->getValue($smtp); + }; + + return array($invoke, $fp, $resultStack); + } + + public function testSingleLineReply() + { + list($getCode) = $this->smtpReading("250 OK\r\n"); + $this->assertSame('250', $getCode()); + } + + /** + * Every continuation line is consumed, leaving the stream at EOF so the + * next command reads its own reply. + */ + public function testMultiLineReplyIsFullyConsumed() + { + list($getCode, $fp) = $this->smtpReading( + "250-mx.example.com\r\n" . + "250-PIPELINING\r\n" . + "250-SIZE 1000000\r\n" . + "250 8BITMIME\r\n" + ); + $this->assertSame('250', $getCode()); + $this->assertFalse(fgets($fp)); + } + + /** + * A multi-line reply is recorded as a single reassembled resultStack entry, + * keeping resultStack aligned with commandStack (one result per command). + */ + public function testMultiLineReplyIsReassembledIntoOneStackEntry() + { + $reply = + "250-mx.example.com\r\n" . + "250-PIPELINING\r\n" . + "250-SIZE 1000000\r\n" . + "250 8BITMIME\r\n"; + list($getCode, , $resultStack) = $this->smtpReading($reply); + $this->assertSame('250', $getCode()); + + $stack = $resultStack(); + $this->assertCount(1, $stack); + $this->assertSame($reply, $stack[0]); + } + + /** + * The whole multi-line reply is preserved in the single resultStack entry + * callers pop for CodeException, so an explanatory line survives alongside + * the final one. + */ + public function testMultiLineErrorReplyKeepsExplanatoryText() + { + $reply = + "535-5.7.8 Username and Password not accepted.\r\n" . + "535 5.7.8 https://support.google.com/mail/?p=BadCredentials\r\n"; + list($getCode, , $resultStack) = $this->smtpReading($reply); + $this->assertSame('535', $getCode()); + + $stack = $resultStack(); + $popped = array_pop($stack); + $this->assertSame($reply, $popped); + $this->assertStringContainsString('Username and Password not accepted', $popped); + } + + /** + * A reply line longer than readLine()'s fgets() chunk is reassembled + * byte-for-byte instead of being truncated, leaving nothing buffered. + */ + public function testLongLineIsNotTruncated() + { + // > the 1024-byte fgets() chunk, so reassembly must stitch >1 chunk + $line = "250 " . str_repeat("X", 2000) . "\r\n"; + list($getCode, $fp, $resultStack) = $this->smtpReading($line); + $this->assertSame('250', $getCode()); + $captured = $resultStack(); + $this->assertSame($line, $captured[0]); + $this->assertFalse(fgets($fp)); + } + + /** + * After a line longer than the fgets() chunk is reassembled, the stream is + * left at the start of the next reply, so the following command reads its + * own code. + */ + public function testTwoRepliesStayInSyncAfterLongLine() + { + // first line > the 1024-byte fgets() chunk, forcing reassembly + $first = "250 " . str_repeat("X", 2000) . "\r\n"; + list($getCode) = $this->smtpReading($first . "354 go ahead\r\n"); + $this->assertSame('250', $getCode()); + $this->assertSame('354', $getCode()); + } + + public function testBareCodeLine() + { + list($getCode) = $this->smtpReading("220\r\n"); + $this->assertSame('220', $getCode()); + } + + public function testEmptyReplyThrows() + { + list($getCode) = $this->smtpReading(""); + $this->expectException(SMTPException::class); + $getCode(); + } + + public function testGarbageReplyThrows() + { + list($getCode) = $this->smtpReading("not an smtp reply\r\n"); + $this->expectException(SMTPException::class); + $getCode(); + } +}