zytzagoo / smtp-validate-email

A PHP library for performing email addresses validation via SMTP
GNU General Public License v3.0
446 stars 157 forks source link

Fix locale issue #58

Closed BenMorel closed 4 years ago

BenMorel commented 4 years ago

When the current locale is not set to an English locale, sprintf() may not use a dot as decimal separator (nor does float-to-string conversion until PHP 8).

For example, my PHP installation came with fr_FR.UTF-8 as a locale, and sprintf('%.f', microtime(true)) returns:

1606241584,856880

Demo on 3v4l

Which makes DateTime::createFromFormat() return false and PHP trigger an error on ->format():

PHP Fatal error: Uncaught Error: Call to a member function format() on bool in (...)/vendor/zytzagoo/smtp-validate-email/src/Validator.php:942

This fixes the issue by temporarily setting the locale to C, then reverting to its original value.

I had the same issue on one of my libraries.

zytzagoo commented 4 years ago

Thank you very much for your (very thorough) report, isolated repro/demo, and the proposed solution.

While thinking and reading through it today I started thinking whether it would be possible to somehow avoid the issue altogether. Came up with this: d5e4967 (#59)

The test confirms it working in the mentioned fr_FR.UTF-8 locale, so I think I'll go with that. Unless you perhaps know of some potential gotchas with using gettimeofday() in this context.

Once again, many thanks for the very detailed report/pr!

BenMorel commented 4 years ago

Hi, it's a good point if you can avoid the problem entirely!

However, I think there's a bug in your commit. If usec is < 900000 (less than 6 digits):

Array
(
    [sec] => 1606329143
    [usec] => 1309
    [minuteswest] => -60
    [dsttime] => 0
)

Concatenating $parts['sec'] . '.' . $parts['usec'] will yield 1606329143.1309, when it should be 1606329143.001309.

Suggestions:

I would probably go with str_pad() myself, I agree that temporarily changing the locale is suboptimal. Working implementation: https://3v4l.org/bj6Xv

Note that the float-to-string conversion has been fixed in PHP 8, it's now locale-independent: https://3v4l.org/BeNhD ... if you want to write a todo for when you'll drop support for PHP 7 ;)

zytzagoo commented 4 years ago

Ah, you are absolutely right! I was afraid of some dark corner like that. Thanks again, will update.

zytzagoo commented 4 years ago

Looks like there might be a "simpler" solution which avoids all the drama:

\DateTime::createFromFormat('0.u00 U', microtime());

Edit: forgot to link to the source: https://stackoverflow.com/questions/33691428/datetime-with-microseconds#comment81576416_37336227

BenMorel commented 4 years ago

Nice one. Whatever works is fine!