Safe compatibility with PHP 7.4 breaking changes for password_hash

[bolmstedt]

PHP 7.4 will make a breaking change for password_hash(), changing PASSWORD_DEFAULT to null and allowing the algorithm (second parameter $algo) to be null and also accept strings while throwing deprecation warnings for usage of integers. Source: https://wiki.php.net/rfc/password_registry

It's worth thinking about how to solve this and other similar issues. One way is to try to patch functions and keep them backwards compatible, another is to release a new minor version which only supports PHP 7.4+ (easier to generate automatically).

If we were to make functions backwards compatible, we would need to widen the type hinting and throw errors based on provided type and PHP version, which feels like a hack:

/**
 * [...]
 * @param null|int|string $algo A password algorithm constant denoting the algorithm to use when hashing the password.
 * [...]
 */
function password_hash(string $password, $algo, array $options = null): string
{
    error_clear_last();
    if (PHP_VERSION_ID < 70400 && is_int($algo) === false) {
        throw new \TypeError('Argument 2 passed to Safe\password_hash() must be of the type int, '.gettype($algo).' given'); // needs to detect line number and file also
    }
    if ($options !== null) {
        $result = \password_hash($password, $algo, $options);
    } else {
        $result = \password_hash($password, $algo);
    }
    if ($result === false) {
        throw PasswordException::createFromPhpError();
    }
    return $result;
}

What is the best solution?

[moufmouf]

Hi Marcus,

This is a very very good question.

We could definitely create a new minor version that is PHP 7.4+ only. I think it will really depend on how stable Safe is when PHP 7.4 is released. There have been quite a number of issues solved in the last months and I don't have time to start maintaining 2 separate branches when an issue is raised.

Your solution with checking the PHP version would also work. It feels a bit of a hack but I'm not shy of hacks :)

Now, there is a last solution that might be the simplest one. Safe functions are generated from the PHP documentation. When PHP 7.4 is out, I'll regenerate Safe functions and we will have this:

function password_hash(string $password, $algo = null, array $options = null): string
{
    error_clear_last();
    if ($options !== null) {
        $result = \password_hash($password, $algo, $options);
    } elseif ($algo !== null) {
        $result = \password_hash($password, $algo;
    } else {
        $result = \password_hash($password);
    }
    if ($result === false) {
        throw PasswordException::createFromPhpError();
    }
    return $result;
}

This will work in PHP 7.4. The only issue is that in PHP <7.4, a user could call:

Safe\password_hash('foo');

That would trigger an exception message:

password_hash(): Unknown password hashing algorithm: 0 in ...

So maybe the best solution is not to do anything and simply regenerate the Safe functions when PHP 7.4 is out :)

[bolmstedt]

Hi David,

I didn't really think about how the generated function for 7.4 might look like, but it looks like it would be fully backwards compatible. The best solution here is to do nothing. I might have been worried over nothing!

[kralmichal]

Hi,

now that 7.4 is at last beta and RC1 should be released on 5th of September. Is there a chance we can reopen this? Right now there is a problem with testing projects on 7.4 in CI as password_hash has a different signature.

Thanks

[moufmouf]

Hey @kralmichal ,

Safe signatures are deduced from PHPStan's signature map. I opened a PR to update this file. Once it is updated, we can regenerate the Safe file and make a new release.

See https://github.com/phpstan/phpstan/pull/2401

[kralmichal]

Hi @moufmouf ,

so based on Ondrej's response in https://github.com/phpstan/phpstan/pull/2401, there needs to be a change made to https://github.com/thecodingmachine/safe/blob/master/generator/src/PhpStanFunctions/PhpStanFunctionMapReader.php.

Am I correct?

[Kharhamel]

#146

[andreasschroth]

Well PHP 7.4 is stable now and released. What's the current state about this issue?

[Kharhamel]

Hello andreasschroth. We are still not sure how to approach this. I would simply widen the type of $algo to int|string with our custom type map file and be done with it, but it is kind of lazy.

[Kharhamel]

#161

[andreasschroth]

Tbh, I would just go for the most pragmatic solution. It can still be solved in a more proper way and after more thought / discussion have been put into it, in case you don't want to decide for the approach to go yet.

Because right now, with PHP 7.4, the Safe version of password_hash() is unusable.

[Kharhamel]

type widen is now in the 1.0.0-beta2. I leave this issue open so we can think about a better and less specific solution for this problem.