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.

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

MailboxHeader.php not using the full email-adress of customers, 1.7.5.0 #11933

Closed
gorkij77 opened this Issue Dec 26, 2018 · 58 comments

Comments

7 participants
@gorkij77
Copy link

gorkij77 commented Dec 26, 2018

Describe the bug
Clean install of 1.7.5.0, migrated old 1.6.1.20 data. MailboxHeader.php doesn't seem to get a correct address - it gets cut after the @-sign.

To Reproduce
Steps to reproduce the behavior:
Good question. Anything related to sending emails to customers generates this problem - them placing an order and getting a confirmation mail, us changing order status (where it's set to mail the customer).

Quite frankly, I'm at a loss to find this. Further logs are available if needed.

Screenshots

exeption-log-edited

Additionnal information
PrestaShop version: 1.7.5.0
PHP version: 7.2.9

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

Hi @gorkij77,

I did not manage to reproduce the issue with PS1.7.5.0 after upgrade from PS1.6.1.23 with the 1-click upgrade v4.5.1.
In fact, this issue is fixed in the PS1.7.4.0 with this PR: #9080
You need to check on your server that the php-intl extension is installed.
This issue occurs also with the ps_emailalerts module, we are aware of this issue.
Here's a similar issue #9638
Thanks to check & feedback.

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Hi @khouloudbelguith

Thank you for lending a hand, losing sales now, which is annoying.

Now, this is a clean install - I looked at MailboxHeaders.php (which seems to be the problem) and I note that the fresh install is 351 lines long. The old one on my PS 1.6 installation is 354 lines? A diff check on the two shows some discrepancies, but I really can't make heads or tails of those.

(Do note, the 1.6 installation had a working mail function. Have 1.7 changed dependencies to the php-intl or what?)

Server info more:
ntl
Internationalization support enabled
version 1.1.0
ICU version 4.2.1
ICU TZData version 2009j
ICU Unicode version 5.1

ps_emailalerts is disabled as well.

I really need to get this going, and the linked errors are related to using special chars. My customers have basically name.nameson@email.com so no high-levels...

MailboxHeader1750.txt
MailboxHeader16.txt

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, did you made a fresh installation PS1.7? or PS1.6?
What is the exact address mail did you manage to reproduce the issue?
Thanks!

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Fresh installation of 1.7.5.0 then migrated my old data to a new MySQL-database.

Swift_RfcComplianceException:
Address in mailbox given [oskar.sten.nilsson81@] does not comply with RFC 2822, 3.6.2.

at vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php:345
at Swift_Mime_Headers_MailboxHeader->_assertValidAddress('oskar.sten.nilsson81@')
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php:261)
at Swift_Mime_Headers_MailboxHeader->normalizeMailboxes(array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php:106)
at Swift_Mime_Headers_MailboxHeader->setNameAddresses(array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php:63)
at Swift_Mime_Headers_MailboxHeader->setFieldBodyModel(array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/SimpleHeaderFactory.php:58)
at Swift_Mime_SimpleHeaderFactory->createMailboxHeader('To', array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/SimpleHeaderSet.php:68)
at Swift_Mime_SimpleHeaderSet->addMailboxHeader('To', array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/SimpleMessage.php:329)
at Swift_Mime_SimpleMessage->setTo(array('oskar.sten.nilsson81@' => 'Oskar Nilsson'))
(vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/SimpleMessage.php:305)
at Swift_Mime_SimpleMessage->addTo('oskar.sten.nilsson81@', 'Oskar Nilsson')
(classes/Mail.php:317)

This particular customer has oskar.sten.nilsson81@gmail.com - the same issue pops up for the last three customers I've spoken to. NONE has high-level ascii in their email.

I've checked their addresses against the MySQL tables as well. They're complete.

Checking the mail()-function in the BO does send mail. Not sure why order statuses or new carts messes this up.

Can I get you any other logs or similar?

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, I tried with this email "oskar.sten.nilsson81@gmail.com" with PS1.7.5.0 after upgrade from 1.6.1.21, but I did not manage to reproduce the issue.
I attached a video record.
https://drive.google.com/file/d/1Mvd9Asv7hSaavFQeDPTRbTlh85ehCsln/view
Could you please diff check your files with the fix: https://github.com/PrestaShop/PrestaShop/pull/9080/files
Thanks!

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, what is the exact email of your shop, you can find it in the BO => Shop Parameters => Contact => Stores Tab?
Thanks!

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

I could. But now let's assume that I'm a complete idiot : How do I perform a diff check?

Exact email of my shop, under store tab, email (req'd field) info@rsl-gaming.se

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, in your Poject_Folder/classes/Mail.php, you can check if the green correction in this PR: https://github.com/PrestaShop/PrestaShop/pull/9080/files exists on your file.

I tried also with your email shop => there is no issue.
Thanks!

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

I'm hand-checking it now. For some reason, line 588 is the first deviation I've come across:
(These are lines 586-599)

        $send = $swift->send($message);
        ShopUrl::resetMainDomainCache();

        if ($send && Configuration::get('PS_LOG_EMAILS')) {
            $mail = new Mail();
            $mail->template = Tools::substr($template, 0, 62);
            $mail->subject = Tools::substr($subject, 0, 255);
            $mail->id_lang = (int) $idLang;
            $recipientsTo = $message->getTo();
            $recipientsCc = $message->getCc();
            $recipientsBcc = $message->getBcc();
            if (!is_array($recipientsTo)) {
                $recipientsTo = [];

Would it be easiest to simply back up my old file, then somehow copy the 9080-file to my directory and see if that helps?

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, it is the same file here https://github.com/PrestaShop/PrestaShop/blob/1.7.5.0/classes/Mail.php
It is easier to copy it.
Thanks!

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Right, tried that, and did an order update to "being processed" or whatever that translates to. Got this after replacing the entire contents of Mail.php:

(Stack trace from debug, need anything else from Debug, ask away)

`Symfony\Component\Debug\Exception\FatalThrowableError:
Parse error: syntax error, unexpected '2018' (T_LNUMBER)

at classes/Mail.php:818
at PrestaShopAutoload->load('Mail')
(vendor/symfony/symfony/src/Symfony/Component/Debug/DebugClassLoader.php:151)
at Symfony\Component\Debug\DebugClassLoader->loadClass('Mail')
at spl_autoload_call('Mail')
(classes/order/OrderHistory.php:533)
at OrderHistoryCore->sendEmail(object(Order), array())
(classes/order/OrderHistory.php:468)
at OrderHistoryCore->addWithemail(true, array())
(controllers/admin/AdminOrdersController.php:544)
at AdminOrdersControllerCore->postProcess()
(classes/controller/Controller.php:270)
at ControllerCore->run()
(classes/Dispatcher.php:509)
at DispatcherCore->dispatch()
(admin7811831ft/index.php:99)`

Thank you for your time so far.

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, you are welcome!
So the issue is fixed?
Thanks!

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Hi,

No. Problem remains the same, it still clips the email addresses after the @-sign.

I'm seriously at a loss here. What other information can I give you? PHP-info? Server-side-access?

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, Could you please follow this link: https://github.com/PierreRambaud/phppsinfo
phppsinfo file provides an equivalent to the phpinfo() function that reports PrestaShop Requirements information about the PHP/MySQL/Apache environment and offers suggestions for improvement.

  1. Upload the file to the server. You should upload your file to the exact directory you want to test. Typically, this will be your httpdocs (/var/www/vhosts/example.com/httpdocs/) directory, although you can upload it to any subdirectory on your server as well. Use FTP to upload the file.
  2. Visit the page in your browser. If you uploaded it to your html directory, you should now visit http://www.example.com/phppsinfo.php, replacing example.com with your own domain name.
  3. Now you can view all of the information about PHP for your server for that particular directory.
    image
    Thanks to check and feedback.
@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Hi, tried this. Using the default login in the code prestashop/prestashop it refuses to connect or display any information.

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, in the phpinfo file,

Thanks to replace this code line

public function __construct($login = self::DEFAULT_LOGIN, $password = self::DEFAULT_PASSWORD)
   {
       if (!empty($_SERVER['PS_INFO_LOGIN'])) {
           $this->login = $_SERVER['PS_INFO_LOGIN'];
       }
       if (!empty($_SERVER['PS_INFO_PASSWORD'])) {
           $this->password = $_SERVER['PS_INFO_PASSWORD'];
       }
       $this->login = !empty($login) ? $login : $this->login;
       $this->password = !empty($password) ? $password : $this->password;
   }

with this

public function __construct($login = self::DEFAULT_LOGIN, $password = self::DEFAULT_PASSWORD)
    {
      $this->login = $login;
      $this->password = $password;
    }
        

THanks to check & feedback.

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

Thank you, tried that. Still doesn't accept password, drops a 401 Unauthorized on me.

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

phpinfo.pdf
Here's mine with mods as well. As well as my basic PHP-info file in pdf format.

phppsinfo.zip

Update : This might be related to Red Hat, found this but I'm shaky at implementing things :
https://central.owncloud.org/t/address-in-mailbox-given-hostmaster-does-not-comply-with-rfc-2822-3-6-2/15981

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

Hi @gorkij77,

In your comment, you said that you migrated your old data to a new MySQL-database, how did you migrated your data, with a custom tool or manually?
Thanks!

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

khouloudbelguith commented Dec 27, 2018

@gorkij77, As it's not the module provided by Prestashop, we can't do anything with this.
Despite our several trials, we could not reproduce your issue with the provided information.
It seems that your issue is not a PrestaShop's core bug but most likely a server configuration or customization problem.
You should contact the module author via your addons.prestashop.com account, they are the only ones to help you with it.
Thanks!

@Pekk3n

This comment has been minimized.

Copy link

Pekk3n commented Dec 27, 2018

I have exactly the same issue with Prestashop 1.7.5.0 after upgrade.

PHP 7.1, CentOS 6

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 27, 2018

I'm aware that this issue has been closed.

However, doing a FRESH install, no migration no nothing and registering myself as a new customer this happens:

(1/1) ContextErrorExceptionWarning: Use of undefined constant INTL_IDNA_VARIANT_UTS46 - assumed 'INTL_IDNA_VARIANT_UTS46' (this will throw an Error in a future version of PHP)
--
in Mail.php line 878
at MailCore::toPunycode('martin.hogman@gmail.com')in Mail.php line 317
at MailCore::send(1, 'account', 'Welcome!', array('{firstname}' => 'Martin', '{lastname}' => 'Högman', '{email}' => 'martin.hogman@gmail.com'), 'martin.hogman@gmail.com', '=?UTF-8?B?TWFydGluIEjDtmdtYW4=?=')in CustomerPersister.php line 225
at CustomerPersisterCore->sendConfirmationMail(object(Customer))in CustomerPersister.php line 196
at CustomerPersisterCore->create(object(Customer), 'PASSWORD')in CustomerPersister.php line 59
at CustomerPersisterCore->save(object(Customer), 'PASSWORD', null, true)in CustomerForm.php line 194
at CustomerFormCore->submit()in AuthController.php line 61
at AuthControllerCore->initContent()in Controller.php line 281
at ControllerCore->run()in Dispatcher.php line 509
at DispatcherCore->dispatch()in index.php line 28

And this from php-error.log, also from a fresh install.

[27-Dec-2018 16:50:16 Europe/Stockholm] PHP Warning:  Use of undefined constant INTL_IDNA_VARIANT_UTS46 - assumed 'INTL_IDNA_VARIANT_UTS46' (this will throw an Error in a future version of PHP) in /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/classes/Mail.php on line 878
[27-Dec-2018 16:50:16 Europe/Stockholm] PHP Warning:  idn_to_ascii() expects parameter 3 to be integer, string given in /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/classes/Mail.php on line 878
[27-Dec-2018 16:50:16 Europe/Stockholm] PHP Fatal error:  Uncaught Swift_RfcComplianceException: Address in mailbox given [martin.hogman@] does not comply with RFC 2822, 3.6.2. in /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php:345
Stack trace:
#0 /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php(261): Swift_Mime_Headers_MailboxHeader->_assertValidAddress('martin.hogman@')
#1 /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php(106): Swift_Mime_Headers_MailboxHeader->normalizeMailboxes(Array)
#2 /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php(63): Swift_Mime_Headers_MailboxHeader->setNameAddresses(Array)
#3 /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/ in /storage/content/50/221250/shop.rsl-gaming.se/public_html/test/vendor/swiftmailer/swiftmailer/lib/classes/Swift/Mime/Headers/MailboxHeader.php on line 345

Now, if this is a migration problem, then why does it occur on a new installation?

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 29, 2018

The fact is that both my working site and my fresh but defective site run on the same server, same php version. So it can't be the ICU.

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 29, 2018

I figured as much as well, but I've set them to check. We're still running 2.4.1 I think. ICU's up to 57-something now. Bar that, it's the code.

Checking my hosts settings I'm supposed to be running PHP 7.2.9, in reality it's 7.2.10. Not that it'd matter here.

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 29, 2018

Another fun fact: sending a test mail from the email configuration page works, even with PHP mail() function. But sending an email from the order page is kaput.
image

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 29, 2018

Question : Does the email setup page in BO use Swiftmailer or is that just a mail() call?

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 29, 2018

I think id does, but I'm not sure.

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 30, 2018

Ok, I have spent most of today testing different scenarios:

Case 1: local machine

  • Installed 1.7.4.4 -> Mail works correctly as a fresh install
  • Updated 1.7.4.4 to 1.7.5.0 via 1 Click Upgrade 4.5.1 -> Mail works correctly after upgrade

SERVER INFO FOR CASE 1:
image

Case 2: production server

  • Installed 1.7.4.4 -> Mail works correctly as a fresh install
  • Updated 1.7.4.4 to 1.7.5.0 via 1 Click Upgrade 4.5.1
    -> MailboxHeader.php Error when sending mail from order page;
    -> ContextErrorException
    Notice: Use of undefined constant INTL_IDNA_VARIANT_UTS46 - assumed 'INTL_IDNA_VARIANT_UTS46' - when trying to register from the FO

SERVER INFO FOR CASE 2:
image

Case 3: production server, production shop started from PS 1.7.2 and upgraded each time up to PS 1.7.5.0 (I have skipped the 1.7.4.3 and 1.7.4.4 upgrades thou)
-> Mail works correctly

The fact is, I don't know why does mail work in Case 3 but not in Case 2 (it's the same server)

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 30, 2018

Now, me being somewhat of a fool on code, but have we run diffchecks on the MailboxHeader.php for the different releases? My 1.6 install (now dormant, also now running on a 7.2.10 PHP since it's set in a subdir of my mail URL) manages mail just fine. What changed between 1.6.1.20 -> 1.7.4.3 -> 1.7.5.0?

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 30, 2018

I don't think the MailboxHeader.php changed. It must be something else. The fact is, 1.7.5 runs well regardin email sending on my local machine. It also runs well on my production site that was last upgraded from 1.7.4.2 to 1.7.5.0. There has to be something related to PS 1.7.5.0 and LiteSpeed server. But then, why my production shop runs well on the same LiteSpeed server but not the fresh installs?

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 30, 2018

Oh, and just tried an upgrade from 1.7.4.2 to 1.7.5.0 and the error is present. This remains a mistery.

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 31, 2018

Hi @gorkij77
Just made another test installation on my LiteSpeed server, this time 1.7.5.0 Beta 1. This version works fine regarding mail sending. So whatever broke is something in PrestaShop 1.7.5.0 Release Candidate 1 or PrestaShop 1.7.5.0 changelog. #11533 in PrestaShop 1.7.5.0 Release Candidate 1 changelog looks most likely to have anything to do with the mail function. WDYT?

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 31, 2018

@gorkij77 I finally cracked it. This is the PR that broke e-mail sending for some of us: Remove deprecated functions #11579
Reverting the change in classes/Mail.php makes e-mail sending work again:
image
Give it a try. It worked for me. It seems that some servers are still running ICU<4.6. And there's where the problem is:

Support for UTS 46 Unicode IDNA Compatibility Processing was added in ICU 4.6 and released on 2010-12-02.

@Matt75

This comment has been minimized.

Copy link
Contributor

Matt75 commented Dec 31, 2018

Problem come from an undefined constant INTL_IDNA_VARIANT_UTS46 due to server configuration issue.

This constant has been introduced in PHP 5.4 but depend of the php-intl extension, this constant require ICU > 4.6 but @gorkij77 use ICU 4.2.1.

So you have to upgrade your ICU version, contact your hosting provider.

For information currently the ICU version is 63.1 ^^

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Dec 31, 2018

Hi @rdy4ever,

Thanks for the hard debugging work 😉

Parameter INTL_IDNA_VARIANT_UTS46 is available since ICU 4.6 (see php official doc http://php.net/manual/en/intl.constants.php). It was added in order to comply with php7.2 requirements (see php official doc http://php.net/manual/fr/function.idn-to-ascii.php)

It looks like this is a known issue about upgrading to php7.2 : https://externals.io/message/101626 🤔

@rdy4ever

This comment has been minimized.

Copy link
Contributor

rdy4ever commented Dec 31, 2018

Hi @matks
I know this is actually a server issue, but it seems there are a lot of hosting providers that didn't bother to upgrade ICU on their servers. This means that in the next days or weeks (when people will strat to upgrade to 1.7.5 after the hollydays) more and more people will be affected by this issue. The question is: shouldn't this requirement be posponed for a while? Or at least add a mandatory chechk for the ICU version at install and upgrade?

@gorkij77

This comment has been minimized.

Copy link
Author

gorkij77 commented Dec 31, 2018

@rdy4ever Excellent, that sorted it. Thanks a bundle!

@Matt75 I've been in contact with them. However, not all providers will do so for, you know, reasons. Is there a way to add redundancy for systems running old releases - perhaps to issue a notification in installation/back office? That would remove a lot of headaches for non-troubleshooting users - y'know, the normal joe scmoue who basically just wants things to work with a minimal hassle?

@matks And that gives the explaination I've been looking for. Now, I can link this into the forum thread, marking it as closed but redundancy checks needs to be made too.

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Dec 31, 2018

@rdy4ever Yes, I was not saying "this is not an issue" 😅 I was rather hooking into the discussion

Yes you're right : at least we need a warning message to tell people about this requirement. However we're going to first try to find a way to handle it without bothering people ;) because it's just a constant, there might be a workaround here.

@marionf marionf added this to To do in PrestaShop 1.7.5 via automation Jan 2, 2019

@marionf marionf added this to the 1.7.5.1 milestone Jan 2, 2019

@marionf marionf moved this from To do to To be tested in PrestaShop 1.7.5 Jan 2, 2019

@marionf marionf added Fixed and removed NMI labels Jan 2, 2019

@marionf marionf moved this from To be tested to To be merged in PrestaShop 1.7.5 Jan 2, 2019

@marionf marionf closed this Jan 2, 2019

PrestaShop 1.7.5 automation moved this from To be merged to Done Jan 2, 2019

@marionf marionf added the Regression label Jan 4, 2019

@khouloudbelguith khouloudbelguith pinned this issue Jan 10, 2019

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