So https://github.com/php/php-src/commit/31d221f9c72f0d0322c84907c5d89a4464667244 landed just now to revert this change, per the request here. This leaves us with the worst variant... so, let's step back a bit:
The main commit that landed (for 7.2.1) is https://github.com/php/php-src/commit/c219991c77e4c68f7d62963e18a1da778de0bbe0#diff-563df88ede8d2a03e291599c46952c13. This commit *removes* checks that certain variables are below an interactive (read: recommended) level, in which case a warning was thrown. This resolves bug #75572. These checks were replaced with checks against minimum values, in which case an E_ERROR is thrown. Note that these minimums are *not* recommendations. They are hard limits imposed by the used algorithms. The libsodium functions will *already* fail if you chose parameters below the limits (see e.g. https://github.com/jedisct1/libsodium/blob/d49d7e8d4f4dd8df593beb9e715e7bc87bc74108/src/libsodium/crypto_pwhash/argon2/pwhash_argon2i.c#L160). The only thing that happened here is that these errors now get a nice, explicit error message, instead of a generic error. The second commit is https://github.com/php/php-src/commit/c05cbd1e775fa69ed9939796a908390f2bfb4459, which is what started this thread. This commit changes the E_ERRORs introduced in the previous commit (not part of any release) to be SodiumExceptions, because that's what ext/sodium does everywhere else. Now https://github.com/php/php-src/commit/31d221f9c72f0d0322c84907c5d89a4464667244 landed, which reverts that change again, based on the request here. To summarize: Previously: <Min: Generic SodiumException, <Interactive: Warning. First commit: <Min: Specific E_ERROR, <Interactive: OK. Second commit: <Min: Specific SodiumException, <Interactive: OK. Third commit: <Min: Specific E_ERROR, <Interactive: OK. In summary, the first two commits combined were not a BC break -- they only removed a warning and made an error message nicer. After the third commit we now *do* have a BC break, because what was previously a SodiumException is now an E_ERROR. I hope we can restore some sanity to this world by reverting that revert... Frank: Please correct me if my understand of what's going on here is not correct. Regards, Nikita On Tue, Nov 28, 2017 at 6:02 PM, Jakub Zelenka <bu...@php.net> wrote: > On Tue, Nov 28, 2017 at 1:17 PM, Sebastian Bergmann <sebast...@php.net> > wrote: > > > Am 28.11.2017 um 13:56 schrieb Frank Denis: > > > Commit: c05cbd1e775fa69ed9939796a908390f2bfb4459 > > > Author: Frank Denis <git...@pureftpd.org> Tue, 28 Nov 2017 > > 13:56:11 +0100 > > > Parents: c219991c77e4c68f7d62963e18a1da778de0bbe0 > > > Branches: PHP-7.2 > > > > > > Link: http://git.php.net/?p=php-src.git;a=commitdiff;h= > > c05cbd1e775fa69ed9939796a908390f2bfb4459 > > > > > > Log: > > > ext/sodium: throw exceptions instead of errors > > > > I am very much in favor of this change. However, making such a change in > > PHP 7.2.1 is a BC break. Any chance to get that into PHP 7.2.0? > > > > > I agree that this is a BC break in bug fixing release and should be > reverted from PHP-7.2 IMO and kept in master only (whatever the change is - > error or exception...) > > CC'd Frank... > > Cheers > > Jakub >
