32-bit compatibility about zxcvbn-php HOT 19 CLOSED

Some more tests on Fedora build system (rawhide, with PHP 8.1.1)

• i686 https://koji.fedoraproject.org/koji/taskinfo?taskID=80070857
• armv7hl https://koji.fedoraproject.org/koji/taskinfo?taskID=80070931
• ppc64le https://koji.fedoraproject.org/koji/taskinfo?taskID=80070906
• s390x https://koji.fedoraproject.org/koji/taskinfo?taskID=80071080

all OK

from zxcvbn-php.

AFAIK, PHP use 64-bit float (double), since always

from zxcvbn-php.

@remicollet, could you check whether namepros@fc3858d works in your environment?

Edit: It should also work with PHP 7.2 and without GMP.

it raise so much errors... (with to without gmp)

+ php /usr/bin/phpunit9 --verbose
PHPUnit 9.5.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.0.13
Configuration: /builddir/build/BUILD/zxcvbn-php-5268743bffbb8cd182c98a4e79d6ed87004a6621/phpunit.xml.dist
Warning:       No code coverage driver available
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

.......................................................E.......  63 / 298 ( 21%)
EEEEE..........................EE.........EEEE..EEEEEEEEEEEEE.. 126 / 298 ( 42%)
............................................................... 189 / 298 ( 63%)
..............EEEEEEEEE................E.FFFFFFFFFFF........... 252 / 298 ( 84%)
.......................EEEEEEEE.EE......E.E...                  298 / 298 (100%)

Time: 00:00.077, Memory: 20.00 MB

There were 47 errors:

1) ZxcvbnPhp\Test\Matchers\DictionaryTest::testGuessesCapitalization
TypeError: ZxcvbnPhp\Math\Binomial::initProvider(): Return value must be of type ZxcvbnPhp\Math\BinomialProvider, string returned

/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Math/Binomial.php:56
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Math/Binomial.php:28
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Math/Binomial.php:22
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Matchers/BaseMatch.php:127
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Matchers/DictionaryMatch.php:233
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Matchers/DictionaryMatch.php:204
/builddir/build/BUILDROOT/php-bjeavons-zxcvbn-php-1.3.0-2.fc35.remi.i386/usr/share/php/ZxcvbnPhp/Matchers/BaseMatch.php:134
/builddir/build/BUILD/zxcvbn-php-5268743bffbb8cd182c98a4e79d6ed87004a6621/test/Matchers/DictionaryTest.php:237

from zxcvbn-php.

Yes using pr #62 seems OK (testing with PHP 7.4, 8.0, with and without gmp, on i686 and x86_64). Test suite passes.

from zxcvbn-php.

Likely cause: #60

from zxcvbn-php.

I'm actually surprised that ever passed on 32-bit machines. Any fix I implement is likely going to harm performance; the workaround would be to install gmp and use PHP 7.3+, that way you won't need the polyfill.

Edit: That test uses large numbers that will never actually be seen by the code in practice. We could simply refuse to handle large numbers, particularly on 32-bit machines that would resort to the polyfill.

from zxcvbn-php.

Thanks for submitting.

@remicollet Do you know if there's an easy VPS host, CI platform, etc where we can test this?

from zxcvbn-php.

Note that this code path will only be taken in one of the following environments:

• PHP < 7.3
• PHP >= 7.3 without gmp

from zxcvbn-php.

@remicollet is it feasible to add ext-gmp and PHP >= 7.3 as dependencies to your build?

from zxcvbn-php.

If not, I can have it fall back to a naïve implementation similar to what it was using previously, but I would expect it to perform poorly on embedded systems, such as those likely to be running 32-bit builds. Requiring GMP and PHP 7.3 on 32-bit builds is likely going to result in more consistent performance.

from zxcvbn-php.

Looking at this a bit closer, I don't think this is (or ever was) functional on 32-bit. binom(49, 12) should return 92,263,734,836--about 2^36, which is solidly outside the range of a 32-bit integer. This would've returned a float previously, but it would've led to different answers on 32-bit and 64-bit systems. Some matchers (e.g., L33tMatch) can realistically hit numbers that high.

@mkopinsky, we may need to take a different approach here. I think L33tMatch can fail even on 64-bit systems. Requiring GMP would be ideal, but we could probably get away with allow binom to return a float, although it would complicate testing a bit.

from zxcvbn-php.

@remicollet Do you know if there's an easy VPS host, CI platform, etc where we can test this?

Any arm distro ?
BTW, I use Fedora, and can run quickly tests in a 32-bit chroot env (using mock)

@remicollet is it feasible to add ext-gmp and PHP >= 7.3 as dependencies to your build?

Yes (especially as I don't care about EOL PHP versions)

I confirm that making gmp a mandatory dependency fix this issue on 32-bit

from zxcvbn-php.

Looking at this a bit closer, I don't think this is (or ever was) functional on 32-bit. binom(49, 12) should return 92,263,734,836--about 2^36, which is solidly outside the range of a 32-bit integer. This would've returned a float previously, but it would've led to different answers on 32-bit and 64-bit systems. Some matchers (e.g., L33tMatch) can realistically hit numbers that high.

notice that float have a 52-bit mantissa which may be acceptable to store 32 to 51-bit unsigned values.. (of course higher values will loose significant bits). This is why PHP cast such value to float.

from zxcvbn-php.

Not on 32-bit builds; I believe PHP uses 33-bit floats in that case.

The test shouldn't pass on 32-bit builds, even with GMP. You should get a different error.

from zxcvbn-php.

The test shouldn't pass on 32-bit builds, even with GMP. You should get a different error.

On Fedora 35 i686, with gmp ext.

Runtime:       PHP 8.0.13
Configuration: /builddir/build/BUILD/zxcvbn-php-5268743bffbb8cd182c98a4e79d6ed87004a6621/phpunit.xml.dist
Warning:       No code coverage driver available
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

...............................................................  63 / 284 ( 22%)
............................................................... 126 / 284 ( 44%)
............................................................... 189 / 284 ( 66%)
............................................................... 252 / 284 ( 88%)
................................                                284 / 284 (100%)

Time: 00:00.063, Memory: 18.00 MB

OK (284 tests, 1381 assertions)

from zxcvbn-php.

That's odd. I'll look into it.

Can you confirm it's using 32-bit floats?

from zxcvbn-php.

@remicollet, could you check whether namepros@fc3858d works in your environment?

Edit: It should also work with PHP 7.2 and without GMP.

from zxcvbn-php.

I'll test it more thoroughly and let you know when I've got it working.

from zxcvbn-php.

I think I've got it working now, @remicollet. Mind giving it another test? Everything is passing on my end. #62

from zxcvbn-php.