Skip to content

SendmailMailer: use CRLF endings for php >= 8.0 #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 16, 2021
Merged

SendmailMailer: use CRLF endings for php >= 8.0 #83

merged 1 commit into from
Apr 16, 2021

Conversation

ondrejcech
Copy link
Contributor

@ondrejcech ondrejcech commented Apr 9, 2021 •
edited
Loading

Version: PHP 8.
OS: Not Windows! Must be one where PHP_EOL is not set to \r\n.

Bug Description

Since this update in PHP8 https://git.php.net/?p=php-src.git;a=commit;h=6983ae751cd301886c966b84367fc7aaa1273b2d which addressed bug https://bugs.php.net/bug.php?id=47983 mail headers must be separated by CRLF. Without CRLF the headers are separated by space.

Steps To Reproduce

Run the following code:

<?php

require __DIR__ . '/../vendor/autoload.php';

$mail = new Nette\Mail\Message;
$mail->setFrom('GLAMI <info@glami.cz>')
    ->addTo('customer@email.cz')
    ->setSubject('Order Confirmation')
    ->setBody("Hello, Your order has been accepted.");

$mail->setHtmlBody('<p>Hello,</p><p>Your order has been accepted.</p>');

$mailer = new Nette\Mail\SendmailMailer;
$res = $mailer->send($mail);

Expected Behavior

An HTML-formatted email.

Hello,

Your order has been accepted.

Actual result

The email is missing the header and is treated as plaintext. The boundaries and the encoded mime-part contents show up in the message:

----------afprpktz9g
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hello, Your order has been accepted.
----------afprpktz9g
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 7bit

<p>Hello,</p><p>Your order has been accepted.</p>
----------afprpktz9g--

TBH there's no test as I'm not able to create a simple test case for sending and receiving email.

dg
dg reviewed Apr 15, 2021
View reviewed changes
src/Mail/SendmailMailer.php
@@ -53,7 +53,7 @@ public function send(Message $mail): void
str_replace(Message::EOL, PHP_EOL, (string) $mail->getEncodedHeader('To')),
str_replace(Message::EOL, PHP_EOL, (string) $mail->getEncodedHeader('Subject')),
str_replace(Message::EOL, PHP_EOL, $parts[1]),
str_replace(Message::EOL, PHP_EOL, $parts[0]),
str_replace(Message::EOL, PHP_VERSION_ID >= 80000 ? "\r\n" : PHP_EOL, $parts[0]),
Copy link
Member

@dg dg Apr 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also apply to other parameters? Then it would probably be best to remove the workaround and use: (I mean in master)

		$args = [
			(string) $mail->getEncodedHeader('To'),
			(string) $mail->getEncodedHeader('Subject'),
			$parts[1],
			$parts[0],
		];

Copy link
Contributor Author

@ondrejcech ondrejcech Apr 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, this issue is related only to $additional_headers.

@dg dg changed the title Use CRLF endings for php version 8.0 or newer SendmailMailer: use CRLF endings for php >= 8.0 Apr 16, 2021
@dg dg merged commit 1c9dde8 into nette:master Apr 16, 2021
dg pushed a commit that referenced this pull request Apr 16, 2021
@ondrejcech @dg
SendmailMailer: use CRLF endings for php >= 8.0 (#83)
fa85a33 
Co-authored-by: Ondřej Čech <ondrej.cech@glami.cz>
dg pushed a commit that referenced this pull request Apr 16, 2021
@ondrejcech @dg
SendmailMailer: use CRLF endings for php >= 8.0 (#83)
3c80399 
Co-authored-by: Ondřej Čech <ondrej.cech@glami.cz>
@lukaspijak
Copy link
Contributor

Hello, can you release the new version with fix this bug? :-)

@dg
Copy link
Member

dg commented Jun 2, 2021

done

@lukaspijak
Copy link
Contributor

Thanks! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg dg dg left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ondrejcech @lukaspijak @dg