search

OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
869 stars 436 forks source link

Upgrading from Magento 1.9 to OpenMage 19.4.23 with PHP 8.1 corrupts HTML templates #3174

Closed djixas closed 1 year ago

djixas commented 1 year ago

Preconditions (*)

  1. Magento 1.9.4.5
  2. OpenMage 19.4.23
  3. PHP 8.1

Steps to reproduce (*)

  1. Take any Magento 1.9 install and upgrade to OpenMage, then enable PHP 8.1, this might also work on default OpenMage, I am not sure
  2. Now try to place an order, click admin forgot pass or anything else, so you get email from TEMPLATES

Expected result (*)

  1. It should display emails correctly

Sample "Test" email displays this content with PHP 7.4:

Name: Email: Telephone:

Comment: This is a test template email - your email settings are correct!

Actual result (*)

  1. [Screenshots, logs or description] mail

Sample "Test" email displays this content with PHP 8.1:

=0A=0AName: =0AEmail: =0ATelephone: =0A=0AComment: This is a test template email - your email settings are correct!

Tested also with https://github.com/icecubenz-open-mage/Icecube_CustomEmailServer just to be safe, same issue, but the issue disappears if I downgrade to PHP 7.4, then email templates work properly. And it's for all emails, forgot admin pass, etc. Already tried reloading all email templates, same issue.

PHP ini settings / modules are identical.

djixas commented 1 year ago

P.S. Tested with two different Magento upgrades, one done via old school upgrade script of yours, other is by replacing all old files with OpenMage. It also keeps generating mailformed address errors:

A message that you sent contained one or more recipient addresses that were incorrectly constructed:

=?utf-8?B??= <>: missing or malformed local part

This address has been ignored. There were no other addresses in your message, and so no attempt at delivery was possible.

------ This is a copy of your message, including all the headers. ------

To: =?utf-8?B??= <> Subject: =?utf-8?B?Q29udGFjdCBGb3Jt?= X-PHP-Originating-Script: 1047:Sendmail.php From: xxx xxxxx@xxxx.xxx Date: Sun, 16 Apr 2023 14:05:12 +0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline MIME-Version: 1.0 Message-Id: E1po30C-0000dv-2d@host.xxxxx.com Date: Sun, 16 Apr 2023 14:05:12 +0000

Name: Email: Telephone:

Comment: This is a test queue email - your email settings are correct!

addison74 commented 1 year ago

The reporting is not the most appropriate to describe the issue but I confirm its existence. Here are the steps to reproduce it.

Place an order as a customer. Once you get the confirmation email check its content. Here is what I am seeing in MailHog.

1. This is for HTML content - EMPTY!

html

2. This is for Plain text

plain

3. This is the Source with Headers

source

Something went wrong at some point and I can't say when, but this issue must be fixed as quickly as possible. I don't think it's coming from the phtml templates because no changes were made in the last years. Most likely a change in a PHP file affects all the messages format sent from the website.

kiatng commented 1 year ago

May be it is related to issue #274.

fballiano commented 1 year ago

great find @kiatng!

@djixas @ADDISON74 could you try to changing vendor/shardj/zf1-future/library/Zend/Mail/Transport/Sendmail.php and replace public $EOL = PHP_EOL; with public $EOL = "\r\n"; and see if it works?

addison74 commented 1 year ago

I will test this report in more detail because I can no longer reproduce it. I didn't make any changes, I installed the latest version of OpenMage 20 and I run it with PHP 8.1 and 8.2 and PR #3181. Please try to test it and let me know what results you get. If it is coming from ZF1-Future, we must report it to be fixed so that everyone can update now that the library has been outsourced.

djixas commented 1 year ago

great find @kiatng!

@djixas @ADDISON74 could you try to changing vendor/shardj/zf1-future/library/Zend/Mail/Transport/Sendmail.php and replace public $EOL = PHP_EOL; with public $EOL = "\r\n"; and see if it works?

Replaced sendmail.php with the edited and all emails are working fine still, even after PHP 8.1 switch, same on both sites :) yayyy

addison74 commented 1 year ago

@djixas - What OS are use using? Windows?

I am asking this because digging for this issue I found that for Windows OS's the line ending must be \r\n and for Linux flavors PHP_EOL. Here some ideas

https://github.com/PHPMailer/PHPMailer/issues/953#issuecomment-276691373

kiatng commented 1 year ago

I managed to reproduced this issue on a shared host running PHP8.1. @fballiano fix on public $EOL = "\r\n"; works on Linux system.

djixas commented 1 year ago

@djixas - What OS are use using? Windows?

I am asking this because digging for this issue I found that for Windows OS's the line ending must be \r\n and for Linux flavors PHP_EOL. Here some ideas

PHPMailer/PHPMailer#953 (comment)

I'm on CentOS v7.9

fballiano commented 1 year ago

I'm wondering if this patch would create a problem on php < 8.1, anyway, I've created a PR on zf1future https://github.com/Shardj/zf1-future/pull/344

addison74 commented 1 year ago

It's about PHP. It would have been natural for the use of PHP_EOL not to produce such unwanted results. It is interesting that the issue was discussed in the PHP repository in recent years, since version 7, PRs were proposed to solve it, but it reappeared.

I don't think it will be a problem with versions < 8.1 because the proposed solution was used in the past with version 7.x. Anyway, I will test it with 7.x , even 7.4 is the minimum requested. Those who use DDEV (to advertise it once again) can test the PR with different PHP versions just editing the configuration file.

djixas commented 1 year ago

It's about PHP. It would have been natural for the use of PHP_EOL not to produce such unwanted results. It is interesting that the issue was discussed in the PHP repository in recent years, since version 7, PRs were proposed to solve it, but it reappeared.

I don't think it will be a problem with versions < 8.1 because the proposed solution was used in the past with version 7.x. Anyway, I will test it with 7.x , even 7.4 is the minimum requested. Those who use DDEV (to advertise it once again) can test the PR with different PHP versions just editing the configuration file.

I've had the same issue with PHP 8.0.

addison74 commented 1 year ago

Whoever encounters problems can make the change manually. Until the PR proposed in ZF1-Future is merged (https://github.com/Shardj/zf1-future/pull/344), we will leave this ticket open.

I would like you to test that PR and approve it if it is OK, so we can speed up its integration. This is the big problem with outsourced parts of OpenMage, we depend on other developers.