From 8512e5d3c3d760ccb27a87987d969fea922a4c63 Mon Sep 17 00:00:00 2001 From: Vlad Safronov Date: Fri, 26 Feb 2021 13:13:10 +0300 Subject: [PATCH 01/16] Fold long lines following RFC 5322 section-2.2.3 To deal with the 998/78 character limitations per line, long lines can be split into a multiple-line representation separated by CRLF + WSP; this is called "folding". Correct folding is particularly important for long header fields. Signed-off-by: Vlad Safronov --- src/Protocol/Smtp.php | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 05f23381..00104973 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -325,12 +325,18 @@ public function data($data) rewind($fp); // max line length is 998 char + \r\n = 1000 - while (($line = stream_get_line($fp, 1000, "\n")) !== false) { + // read the line up to PHP_SOCK_CHUNK_SIZE + while (($line = stream_get_line($fp, 0, "\n")) !== false) { $line = rtrim($line, "\r"); if (isset($line[0]) && $line[0] === '.') { // Escape lines prefixed with a '.' $line = '.' . $line; } + if (strlen($line) > 998) { + // Long lines are "folded" by inserting "" + // https://tools.ietf.org/html/rfc5322#section-2.2.3 + $line = substr(chunk_split($line, 998, "\r\n "), 0, -3); + } $this->_send($line); } fclose($fp); From 7bc2deb6fcef6c3f4632337e91d0d3c12fb92152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sat, 27 Feb 2021 14:37:03 +0200 Subject: [PATCH 02/16] Use Headers::FOLDING when folding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 00104973..5a11fadb 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -8,6 +8,8 @@ namespace Laminas\Mail\Protocol; +use Laminas\Mail\Headers; + /** * SMTP implementation of Laminas\Mail\Protocol\AbstractProtocol * @@ -335,7 +337,7 @@ public function data($data) if (strlen($line) > 998) { // Long lines are "folded" by inserting "" // https://tools.ietf.org/html/rfc5322#section-2.2.3 - $line = substr(chunk_split($line, 998, "\r\n "), 0, -3); + $line = substr(chunk_split($line, 998, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); } $this->_send($line); } From 44c4053b0ebd01c3b51df7fd8758f8c429b03c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sat, 27 Feb 2021 14:32:57 +0200 Subject: [PATCH 03/16] Use fgets and detect partial line reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 5a11fadb..e8bbc453 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -326,21 +326,42 @@ public function data($data) unset($data); rewind($fp); - // max line length is 998 char + \r\n = 1000 - // read the line up to PHP_SOCK_CHUNK_SIZE - while (($line = stream_get_line($fp, 0, "\n")) !== false) { - $line = rtrim($line, "\r"); + $chunkSize = 4096; + $line = ''; + while (($buffer = fgets($fp, $chunkSize)) !== false) { + $line .= $buffer; + + // partial read, continue loop to read again to complete the line + if (strlen($buffer) === $chunkSize - 1 && $buffer[$chunkSize - 2] !== "\n") { + continue; + } + + $line = rtrim($line, "\r\n"); if (isset($line[0]) && $line[0] === '.') { // Escape lines prefixed with a '.' $line = '.' . $line; } + + // max line length is 998 char + \r\n = 1000 + if (strlen($line) > 998) { + // Long lines are "folded" by inserting "" + // https://tools.ietf.org/html/rfc5322#section-2.2.3 + $line = substr(chunk_split($line, 998, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); + } + $this->_send($line); + $line = ''; + } + if ($line) { + // max line length is 998 char + \r\n = 1000 if (strlen($line) > 998) { // Long lines are "folded" by inserting "" // https://tools.ietf.org/html/rfc5322#section-2.2.3 $line = substr(chunk_split($line, 998, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); } + $this->_send($line); } + fclose($fp); $this->_send('.'); From 009eed6472e5af26ec207fddf45cd8be03796697 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sat, 27 Feb 2021 13:32:37 +0200 Subject: [PATCH 04/16] Add unit test for long header wrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index 07a1f9c9..2c00e46a 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -199,6 +199,33 @@ public function testReceivesMailArtifacts(): void $this->assertStringContainsString("\r\n\r\nThis is only a test.", $data, $data); } + /** + * Fold long lines during smtp communication in Protocol\Smtp class. + * Test folding of long lines following RFC 5322 section-2.2.3 + * + * @see https://github.com/laminas/laminas-mail/pull/140 + */ + public function testLongLinesFoldingRFC5322(): void + { + $message = $this->getMessage(); + // Create buffer of 8192 bytes (PHP_SOCK_CHUNK_SIZE) + $buffer = str_repeat('0123456789abcdef', 512); + $this->assertEquals(8192, strlen($buffer)); + + $headerValue = substr($buffer, 0, 1000); + $bufferSizeHeaderValue = substr($buffer, 0, 8192 - strlen('X-Exact-Length') - 2); + $message->getHeaders()->addHeaders([ + 'X-Ms-Exchange-Antispam-Messagedata' => $headerValue, + 'X-Exact-Length' => $bufferSizeHeaderValue, + ]); + + $this->transport->send($message); + $data = $this->connection->getLog(); + // The original header can't be present if it's wrapped + $this->assertStringNotContainsString($headerValue, $data, "May not contain headerValue"); + $this->assertStringNotContainsString($bufferSizeHeaderValue, $data, "May not contain bufferSizeHeaderValue"); + } + public function testCanUseAuthenticationExtensionsViaPluginManager(): void { $options = new SmtpOptions([ From 3dab0b3aefe08811efe3f9885b960a1cc895948c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sun, 28 Feb 2021 00:27:48 +0200 Subject: [PATCH 05/16] Refactor data reader in Protocol\Smtp to use Generator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This isolates the logic of handling incomplete reads to own unit. Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 53 +++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index e8bbc453..9ce927fe 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -317,41 +317,42 @@ public function data($data) $this->_send('DATA'); $this->_expect(354, 120); // Timeout set for 2 minutes as per RFC 2821 4.5.3.2 - if (($fp = fopen("php://temp", "r+")) === false) { - throw new Exception\RuntimeException('cannot fopen'); - } - if (fwrite($fp, $data) === false) { - throw new Exception\RuntimeException('cannot fwrite'); - } - unset($data); - rewind($fp); + $chunkReader = static function (string $data, int $chunkSize = 4096) { + if (($fp = fopen("php://temp", "r+")) === false) { + throw new Exception\RuntimeException('cannot fopen'); + } + if (fwrite($fp, $data) === false) { + throw new Exception\RuntimeException('cannot fwrite'); + } + rewind($fp); + + $line = null; + while (($buffer = fgets($fp, $chunkSize)) !== false) { + $line .= $buffer; - $chunkSize = 4096; - $line = ''; - while (($buffer = fgets($fp, $chunkSize)) !== false) { - $line .= $buffer; + // partial read, continue loop to read again to complete the line + if (strlen($buffer) === $chunkSize - 1 && $buffer[$chunkSize - 2] !== "\n") { + continue; + } - // partial read, continue loop to read again to complete the line - if (strlen($buffer) === $chunkSize - 1 && $buffer[$chunkSize - 2] !== "\n") { - continue; + yield $line; + $line = null; } + if ($line !== null) { + yield $line; + } + + fclose($fp); + }; + + foreach ($chunkReader($data) as $line) { $line = rtrim($line, "\r\n"); if (isset($line[0]) && $line[0] === '.') { // Escape lines prefixed with a '.' $line = '.' . $line; } - // max line length is 998 char + \r\n = 1000 - if (strlen($line) > 998) { - // Long lines are "folded" by inserting "" - // https://tools.ietf.org/html/rfc5322#section-2.2.3 - $line = substr(chunk_split($line, 998, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); - } - $this->_send($line); - $line = ''; - } - if ($line) { // max line length is 998 char + \r\n = 1000 if (strlen($line) > 998) { // Long lines are "folded" by inserting "" @@ -362,8 +363,6 @@ public function data($data) $this->_send($line); } - fclose($fp); - $this->_send('.'); $this->_expect(250, 600); // Timeout set for 10 minutes as per RFC 2821 4.5.3.2 $this->data = true; From 662433d889dac4cfb10f5edc606b3df888e001ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sun, 28 Feb 2021 00:36:26 +0200 Subject: [PATCH 06/16] Optimize out strlen call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As the buffer max size is known, can check last byte with isset Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 9ce927fe..f4610d2f 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -331,7 +331,7 @@ public function data($data) $line .= $buffer; // partial read, continue loop to read again to complete the line - if (strlen($buffer) === $chunkSize - 1 && $buffer[$chunkSize - 2] !== "\n") { + if (isset($buffer[$chunkSize - 2]) && $buffer[$chunkSize - 2] !== "\n") { continue; } From fb82a1264fde9019f7d4e3cf95ac53933ba5258a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Sun, 28 Feb 2021 22:55:11 +0200 Subject: [PATCH 07/16] Test for max length of all SMTP data lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index 2c00e46a..a31e7e80 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -221,6 +221,13 @@ public function testLongLinesFoldingRFC5322(): void $this->transport->send($message); $data = $this->connection->getLog(); + + $lines = explode("\r\n", $data); + $maxLen = max(array_map(static function ($line) { + return strlen($line); + }, $lines)); + $this->assertLessThan(1000, $maxLen, "No line can be longer than 1000 bytes"); + // The original header can't be present if it's wrapped $this->assertStringNotContainsString($headerValue, $data, "May not contain headerValue"); $this->assertStringNotContainsString($bufferSizeHeaderValue, $data, "May not contain bufferSizeHeaderValue"); From 0deb254fb4b824230373fe3bf7bcadd4dd6c16fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 09:07:19 +0200 Subject: [PATCH 08/16] Move chunkedReader to a method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit he method has no state, so therefore declared static. Ideally this should be a standalone class, perhaps in laminas-stdlib. Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 71 +++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 30 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index f4610d2f..a5a5ab21 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -8,6 +8,7 @@ namespace Laminas\Mail\Protocol; +use Generator; use Laminas\Mail\Headers; /** @@ -172,6 +173,44 @@ public function setUseCompleteQuit($useCompleteQuit) return $this->useCompleteQuit = (bool) $useCompleteQuit; } + /** + * Read $data as lines terminated by "\n" + * + * @param string $data + * @param int $chunkSize + * @return Generator|string[] + * @author Elan Ruusamäe + */ + private static function chunkedReader(string $data, int $chunkSize = 4096): Generator + { + if (($fp = fopen("php://temp", "r+")) === false) { + throw new Exception\RuntimeException('cannot fopen'); + } + if (fwrite($fp, $data) === false) { + throw new Exception\RuntimeException('cannot fwrite'); + } + rewind($fp); + + $line = null; + while (($buffer = fgets($fp, $chunkSize)) !== false) { + $line .= $buffer; + + // partial read, continue loop to read again to complete the line + if (isset($buffer[$chunkSize - 2]) && $buffer[$chunkSize - 2] !== "\n") { + continue; + } + + yield $line; + $line = null; + } + + if ($line !== null) { + yield $line; + } + + fclose($fp); + } + /** * Whether or not send QUIT command * @@ -317,36 +356,8 @@ public function data($data) $this->_send('DATA'); $this->_expect(354, 120); // Timeout set for 2 minutes as per RFC 2821 4.5.3.2 - $chunkReader = static function (string $data, int $chunkSize = 4096) { - if (($fp = fopen("php://temp", "r+")) === false) { - throw new Exception\RuntimeException('cannot fopen'); - } - if (fwrite($fp, $data) === false) { - throw new Exception\RuntimeException('cannot fwrite'); - } - rewind($fp); - - $line = null; - while (($buffer = fgets($fp, $chunkSize)) !== false) { - $line .= $buffer; - - // partial read, continue loop to read again to complete the line - if (isset($buffer[$chunkSize - 2]) && $buffer[$chunkSize - 2] !== "\n") { - continue; - } - - yield $line; - $line = null; - } - - if ($line !== null) { - yield $line; - } - - fclose($fp); - }; - - foreach ($chunkReader($data) as $line) { + $reader = self::chunkedReader($data); + foreach ($reader as $line) { $line = rtrim($line, "\r\n"); if (isset($line[0]) && $line[0] === '.') { // Escape lines prefixed with a '.' From a3e57fd0dc1f5e3e04c052309aeabc661da9bb62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 09:25:30 +0200 Subject: [PATCH 09/16] Update chunk_split to use 997 for correct result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 2 +- test/Transport/SmtpTest.php | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index a5a5ab21..73aa286e 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -368,7 +368,7 @@ public function data($data) if (strlen($line) > 998) { // Long lines are "folded" by inserting "" // https://tools.ietf.org/html/rfc5322#section-2.2.3 - $line = substr(chunk_split($line, 998, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); + $line = substr(chunk_split($line, 997, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); } $this->_send($line); diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index a31e7e80..d724479a 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -223,10 +223,10 @@ public function testLongLinesFoldingRFC5322(): void $data = $this->connection->getLog(); $lines = explode("\r\n", $data); - $maxLen = max(array_map(static function ($line) { - return strlen($line); - }, $lines)); - $this->assertLessThan(1000, $maxLen, "No line can be longer than 1000 bytes"); + foreach ($lines as $line) { + // Max line length is 998 char + \r\n = 1000 + $this->assertLessThanOrEqual(998, strlen($line), sprintf('Line is too long: ' . $line)); + } // The original header can't be present if it's wrapped $this->assertStringNotContainsString($headerValue, $data, "May not contain headerValue"); From b40d5d4d14be75291c0b94e3ce9616598f8d2924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 09:44:06 +0200 Subject: [PATCH 10/16] Update test that header line with exact length is not wrapped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index d724479a..e4a60b20 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -213,7 +213,7 @@ public function testLongLinesFoldingRFC5322(): void $this->assertEquals(8192, strlen($buffer)); $headerValue = substr($buffer, 0, 1000); - $bufferSizeHeaderValue = substr($buffer, 0, 8192 - strlen('X-Exact-Length') - 2); + $bufferSizeHeaderValue = substr($buffer, 0, 998 - strlen('X-Exact-Length') - 2); $message->getHeaders()->addHeaders([ 'X-Ms-Exchange-Antispam-Messagedata' => $headerValue, 'X-Exact-Length' => $bufferSizeHeaderValue, @@ -230,7 +230,8 @@ public function testLongLinesFoldingRFC5322(): void // The original header can't be present if it's wrapped $this->assertStringNotContainsString($headerValue, $data, "May not contain headerValue"); - $this->assertStringNotContainsString($bufferSizeHeaderValue, $data, "May not contain bufferSizeHeaderValue"); + // Header with exact length is not wrapped + $this->assertStringContainsString($bufferSizeHeaderValue, $data, "Must contain bufferSizeHeaderValue"); } public function testCanUseAuthenticationExtensionsViaPluginManager(): void From 38bc378e73faed1c112992c43bcd4359f2dea251 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 09:45:48 +0200 Subject: [PATCH 11/16] Update test to require lines not to be empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index e4a60b20..8d076d9d 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -223,6 +223,8 @@ public function testLongLinesFoldingRFC5322(): void $data = $this->connection->getLog(); $lines = explode("\r\n", $data); + $this->assertCount(21, $lines); + foreach ($lines as $line) { // Max line length is 998 char + \r\n = 1000 $this->assertLessThanOrEqual(998, strlen($line), sprintf('Line is too long: ' . $line)); From 43d8da384c62dc4f203df53c1b4f4e66cc8f76da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 11:21:04 +0200 Subject: [PATCH 12/16] Add MAX_LINE_LENGTH constant for magic 998 value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 73aa286e..2d6c1325 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -21,6 +21,12 @@ class Smtp extends AbstractProtocol { use ProtocolTrait; + /** + * RFC 5322 section-2.2.3 specifies maximum of 998 bytes per line. + * @see https://tools.ietf.org/html/rfc5322#section-2.2.3 + */ + public const MAX_LINE_LENGTH = 998; + /** * The transport method for the socket * @@ -364,11 +370,13 @@ public function data($data) $line = '.' . $line; } - // max line length is 998 char + \r\n = 1000 - if (strlen($line) > 998) { + if (strlen($line) > self::MAX_LINE_LENGTH) { // Long lines are "folded" by inserting "" // https://tools.ietf.org/html/rfc5322#section-2.2.3 - $line = substr(chunk_split($line, 997, Headers::FOLDING), 0, -strlen(Headers::FOLDING)); + // Add "-1" to stay within limits, + // because Headers::FOLDING includes a byte for space character after \r\n + $chunks = chunk_split($line, self::MAX_LINE_LENGTH - 1, Headers::FOLDING); + $line = substr($chunks, 0, -strlen(Headers::FOLDING)); } $this->_send($line); From 4105ce61f68172d8ccc2b5c2e1b4aa5386772065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 11:26:09 +0200 Subject: [PATCH 13/16] Update test to use SmtpProtocol::MAX_LINE_LENGTH constant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index 8d076d9d..eb37aef9 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -210,30 +210,35 @@ public function testLongLinesFoldingRFC5322(): void $message = $this->getMessage(); // Create buffer of 8192 bytes (PHP_SOCK_CHUNK_SIZE) $buffer = str_repeat('0123456789abcdef', 512); - $this->assertEquals(8192, strlen($buffer)); - $headerValue = substr($buffer, 0, 1000); - $bufferSizeHeaderValue = substr($buffer, 0, 998 - strlen('X-Exact-Length') - 2); + $maxLen = SmtpProtocol::MAX_LINE_LENGTH; + $headerWithLargeValue = $buffer; + $headerWithExactlyMaxLineLength = substr($buffer, 0, $maxLen - strlen('X-Exact-Length: ')); $message->getHeaders()->addHeaders([ - 'X-Ms-Exchange-Antispam-Messagedata' => $headerValue, - 'X-Exact-Length' => $bufferSizeHeaderValue, + 'X-Ms-Exchange-Antispam-Messagedata' => $headerWithLargeValue, + 'X-Exact-Length' => $headerWithExactlyMaxLineLength, ]); $this->transport->send($message); $data = $this->connection->getLog(); $lines = explode("\r\n", $data); - $this->assertCount(21, $lines); + $this->assertCount(28, $lines); foreach ($lines as $line) { - // Max line length is 998 char + \r\n = 1000 - $this->assertLessThanOrEqual(998, strlen($line), sprintf('Line is too long: ' . $line)); + $this->assertLessThanOrEqual($maxLen, strlen($line), sprintf('Line is too long: ' . $line)); } - // The original header can't be present if it's wrapped - $this->assertStringNotContainsString($headerValue, $data, "May not contain headerValue"); - // Header with exact length is not wrapped - $this->assertStringContainsString($bufferSizeHeaderValue, $data, "Must contain bufferSizeHeaderValue"); + $this->assertStringNotContainsString( + $headerWithLargeValue, + $data, + "The original header can't be present if it's wrapped" + ); + $this->assertStringContainsString( + $headerWithExactlyMaxLineLength, + $data, + "Header with exact length is not wrapped" + ); } public function testCanUseAuthenticationExtensionsViaPluginManager(): void From db947515aca4a97af67fbdaf5fe54743050580d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 14:18:09 +0200 Subject: [PATCH 14/16] Introduce local variable and exccessively document the logic in the loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 2d6c1325..1abe9a0c 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -201,8 +201,25 @@ private static function chunkedReader(string $data, int $chunkSize = 4096): Gene while (($buffer = fgets($fp, $chunkSize)) !== false) { $line .= $buffer; + // This is optimization to avoid calling length() in a loop. + // We need to match a condition that is when: + // 1. maximum was read from fgets, which is $chunkSize-1 + // 2. last byte of the buffer is not \n + // + // to access last byte of buffer, we can do + // - $buffer[strlen($buffer)-1] + // and when maximum is read from fgets, then: + // - strlen($buffer) === $chunkSize-1 + // - strlen($buffer)-1 === $chunkSize-2 + // which means this is also true: + // - $buffer[strlen($buffer)-1] === $buffer[$chunkSize-2] + // + // the null coalesce works, as string offset can never be null + $lastByte = $buffer[$chunkSize - 2] ?? null; + // partial read, continue loop to read again to complete the line - if (isset($buffer[$chunkSize - 2]) && $buffer[$chunkSize - 2] !== "\n") { + // compare \n first as that's usually false + if ($lastByte !== "\n" && $lastByte !== null) { continue; } From b93be7561c25ae8b54b415b3701a760074ca205c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 14:23:12 +0200 Subject: [PATCH 15/16] Rename MAX_LINE_LENGTH -> SMTP_LINE_LIMIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- src/Protocol/Smtp.php | 7 ++++--- test/Transport/SmtpTest.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Protocol/Smtp.php b/src/Protocol/Smtp.php index 1abe9a0c..7aaf9786 100644 --- a/src/Protocol/Smtp.php +++ b/src/Protocol/Smtp.php @@ -23,9 +23,10 @@ class Smtp extends AbstractProtocol /** * RFC 5322 section-2.2.3 specifies maximum of 998 bytes per line. + * This may not be exceeded. * @see https://tools.ietf.org/html/rfc5322#section-2.2.3 */ - public const MAX_LINE_LENGTH = 998; + public const SMTP_LINE_LIMIT = 998; /** * The transport method for the socket @@ -387,12 +388,12 @@ public function data($data) $line = '.' . $line; } - if (strlen($line) > self::MAX_LINE_LENGTH) { + if (strlen($line) > self::SMTP_LINE_LIMIT) { // Long lines are "folded" by inserting "" // https://tools.ietf.org/html/rfc5322#section-2.2.3 // Add "-1" to stay within limits, // because Headers::FOLDING includes a byte for space character after \r\n - $chunks = chunk_split($line, self::MAX_LINE_LENGTH - 1, Headers::FOLDING); + $chunks = chunk_split($line, self::SMTP_LINE_LIMIT - 1, Headers::FOLDING); $line = substr($chunks, 0, -strlen(Headers::FOLDING)); } diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index eb37aef9..cd7b20a0 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -211,7 +211,7 @@ public function testLongLinesFoldingRFC5322(): void // Create buffer of 8192 bytes (PHP_SOCK_CHUNK_SIZE) $buffer = str_repeat('0123456789abcdef', 512); - $maxLen = SmtpProtocol::MAX_LINE_LENGTH; + $maxLen = SmtpProtocol::SMTP_LINE_LIMIT; $headerWithLargeValue = $buffer; $headerWithExactlyMaxLineLength = substr($buffer, 0, $maxLen - strlen('X-Exact-Length: ')); $message->getHeaders()->addHeaders([ From 74f4242afe9610806865033b9e110cce30514405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Fri, 5 Mar 2021 14:26:00 +0200 Subject: [PATCH 16/16] Update test to expect Headers::FOLDING never change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Elan Ruusamäe --- test/Transport/SmtpTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Transport/SmtpTest.php b/test/Transport/SmtpTest.php index cd7b20a0..01a18e12 100644 --- a/test/Transport/SmtpTest.php +++ b/test/Transport/SmtpTest.php @@ -207,6 +207,9 @@ public function testReceivesMailArtifacts(): void */ public function testLongLinesFoldingRFC5322(): void { + $message = 'The folding logic expects exactly 1 byte after \r\n in folding'; + $this->assertEquals("\r\n ", Headers::FOLDING, $message); + $message = $this->getMessage(); // Create buffer of 8192 bytes (PHP_SOCK_CHUNK_SIZE) $buffer = str_repeat('0123456789abcdef', 512);