Description: ------------ I have implemented the following functions as a component of PHP_Compat library. * hash * hash_algos * hash_hmac * sha1 It also includes a PHP implementation of SHA-256 hash function. Therefore, MD5, SHA-1 and SHA-256 hash algorithms are supported. It would be nice if these components would be imported into the repository. You can download the files from here: http://download.revulo.com/PHP/PHP_Compat_Hash.tgz P.S. I wonder where we should place the file "sha256.php". Should we move it to "Function/hash" directory or somewhere?

phpseclib's Crypt_Hash includes sha256, sha384, and sha512 implementations: http://phpseclib.sourceforge.net/ Per the documentation, Crypt_Hash requires, minimally, PHP 4.3.0 due to its use of PHP's sha1 function. If PHP_Compat got its own sha1 implementation the requirements would probably be lower still.

mhash could use sha256 support as well: http://svn.php.net/viewvc/pear/packages/PHP_Compat/trunk/Compat/Function/mhash.php?revision=269597&view=markup

I see no reason not to merge the sha256.php into hash.php as a helper, since, as you know, there's no actual sha256 function in PHP5 anyway. For hmac, I wonder if it would be worth utilising the Crypt_HMAC2 PEAR package instead? If this is to be done correctly, we should ensure that all ground is covered and test against these files: http://svn.php.net/repository/php/php-src/trunk/ext/hash/tests/ I am particularly looking at hash_algos.phpt for a list of supported algorithms that would require support.

In my opinion, we should not merge sha256.php into hash.php. As pointed out above, mhash.php could also utilize sha256.php. In this case, we cannot use the Crypt_HMAC2 package since it's written in PHP5. The tests included in PHP_Compat_Hash.tgz are almost the same as in the repository of PHP. But I intentionally omitted some tests because the hash functions implemented in PHP are very slow.

Touché. What we need is a way to separate the helper functions from the existing PHP functions as to avoid confusion and to maintain integrity. I propose to either prefix the filename with helper_ or place the file in a "Helper" directory, in the trunk. With regards to the tests, my point was that, for example, your hash_algos.phpt is a far cry from the PHP hash_algos.phpt.

I think renaming the file "_sha256.php" is a simple and comprehensible solution as we have a precedent: "_magic_quotes_inputs.php" in the Environment directory. As for the tests, why do you think both of the hash_algos.phpt must be identical? While the hash extension supports about 40 hash algorithms, php_compat_hash() currently supports MD5, SHA-1 and SHA-256 hash algorithms only. For this reason, php_compat_hash_algos() should return array("md5","sha1","sha256").

Yes, using an underscore would be fine to signify a common file. They should be identical because PHP_Compat should bring the latest PHP compatibility to PHP4. Having said this, I'm not entirely sure support for this is required in PHP_Compat as PHP4 users are able to install the PECL hash package.

PHP manual http://php.net/hash-algos says: "hash_algos returns a numerically indexed array containing the list of SUPPORTED hashing algorithms." Therefore, we must not include unsupported algorithms in the list. For example, if php_compat_hash_algos() returns an array containing "sha512", it contradicts the current status of php_compat_hash() that does not support SHA-512. There would be no problem if all PHP4 users were able to install hash extension. But we should care about users who don't have permission to install extensions.

The PHP_Compat functions should match what is in PHP core as closely as possible.

You mean php_compat_hash_array() should return an incorrect answer? I can understand your policy. However, we don't yet satisfy the requirement to implement php_compat_hash_algos() in such a way.

Measures can be taken to meet the requirements. Place holders can be implemented until the support is available or a user error which explains that it is not yet supported. This avoids confusing the user.

I think your implementation much confuses users. Please think of the following code. if (!function_exists('hash_algos')) { require 'PHP/Compat/Function/hash_algos.php'; } if (!function_exists('hash')) { require 'PHP/Compat/Function/hash.php'; } foreach (hash_algos() as $algo) { echo $algo . ': ' . hash($algo, 'abc') . "\n"; } If the hash extension is installed, the above code outputs no errors. md2: da853b0d3f88d99b30283a69e6ded6bb md4: a448017aaf21d8525fc10ae87aa6729d ...... And my implementation of php_compat_hash_algos() results in: md5: 900150983cd24fb0d6963f7d28e17f72 sha1: a9993e364706816aba3e25717850c26c9cd0d89d sha256: ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad But your implementation results in a lot of warnings. Warning: hash(): Unknown hashing algorithm: md2 in PHP/Compat/Function/hash.php on line 38 md2: Warning: hash(): Unknown hashing algorithm: md4 in PHP/Compat/Function/hash.php on line 38 md4: ...... Probably the user does not expect that the above code generates an error.

The error is that the algorithm is not implemented, not that it is unknown. Not displaying the error does not mean there is no error. If a user calls an algorithm that they expect to work and it does not, an error is expected.

hash_algos() is a function that lists SUPPORTED algorithms as is documented in PHP manual. Users use this function to check preliminarily whether the algorithm is supported or not. After confirming that the algorithm is supported, the user calls hash() function without any worries. Thus, that's an unexpected result for the user if the above code reports an error, even if we alter the error message to "Not Implemented". If my opinion is not accepted, I will drop the submission of my hash_algos.php. Please commit the code after removing my hash_algos.php and hash_algos.phpt.

The issue is not exclusive to the hash_algos function. There is work to be carried out before this is committed.

I agree that hash_algos should only return the supported algorithms. What needs to be done to get this patch included in our next release?

I made an updated version of my patch that renamed the file "_sha256.php". http://download.revulo.com/PHP/PHP_Compat_Hash-20100821.zip If you point out the remaining work concretely, then I would update my patch again.

I feel we need to address why PHP_Compat cannot support the same algos as the Message Digest (hash) engine. What is here is a good start, but I think more can be done.

I think the error message "hash(): Unknown hashing algorithm: ..." sufficiently explains that php_compat_hash() doesn't support the algorithm yet. If you say so, however, how about adding the following code to hash.php? switch ($algo) { case 'md2': case 'md4': ...... case 'haval256,5': user_error('hash(): Not yet implemented: ' . $algo, E_USER_WARNING); return false; ...... }