Core

I am not sure what the usecase would be to have a "default" hashing algorithm that can be configured. That would mean that upon change of configuration all already created hashes are invalid.

I would prefer to leave the decision on the hash to the one that is actually using the "hash" property in stdWrap, so that it's more explicit.

What do you think? What are the use-cases of having it configurable?

I agree that encryption should just "fail" if a certain crypto is not available. Neither is leaving it unencrypted nor having it encrypted differently (just for the sake of enrypting :-)) much useful.
So I'd say return FALSE would be a good option. Together with stdWrap I guess that would result in an "empty" value then.

How can we have a PHP error? I covered this case with a unit test and we have an explicit test to see if $algorithm is available.

The current behavior is to return the text as-this if it could not be hashed because of an invalid, empty or non-existing hashing algorithm. I'm not sure it really makes sense to automatically fall back to another hashing algorithm because in the end, you certainly have to know the hashing algorithm being used and automatic fallback, even if explicitly defined by the administrator really sounds odd to me...

Stefan Neufeind wrote:

I agree that encryption should just "fail" if a certain crypto is not available. Neither is leaving it unencrypted nor having it encrypted differently (just for the sake of enrypting :-)) much useful.
So I'd say return FALSE would be a good option. Together with stdWrap I guess that would result in an "empty" value then.

I like the idea of not returning it unhashed, would help much in stdWrap conditions

Using an unknown algo gives a PHP-warning. I guess that's what was meant to be suppressed by checking first. Imho fetching the list of available algos (maybe storing it via a singleton - not sure if that's really needed) would be all it takes.

Singleton for the list? Or just uncached access to the hash_algos()?

$ cat hash.php
echo hash('myalgo', 'abc123');

$ php hash.php
PHP Warning: hash(): Unknown hashing algorithm: myalgo in /home/sn/asd.php on line 2

Stefan Neufeind wrote:

Using an unknown algo gives a PHP-warning. I guess that's what was meant to be suppressed by checking first. Imho fetching the list of available algos (maybe storing it via a singleton - not sure if that's really needed) would be all it takes.

Singleton for the list? Or just uncached access to the hash_algos()?

Before coming up with a cache for hash_algos() I'd like to see numbers showing that it is slow call worth some caching mechanism.

$ cat hash.php
echo hash('myalgo', 'abc123');

$ php hash.php
PHP Warning: hash(): Unknown hashing algorithm: myalgo in /home/sn/asd.php on line 2

Sorry but I still don't understand. We are explicitly checking that both the 'hash' function and the requested algorithm are available so are you really sure we get a PHP warning anyway? Does not make much sense.

Of course your example has this pitfall but our stdWrap implementation should not.

Hmm, by the subject "add check" I assumed there was no check yet. I just reviewed the diff - of course you are right. And I think it's fine as is. Maybe we should make it return nothing/false if the algo is unavailable (avoiding disclosing unhashed information). But the rest is fine imho.

I'd prefer "nothing" instead of FALSE because then we would have either a boolean or a string as return value, which I don't like. So OK for a follow-up to return '' but make sure to fix the unit test as well.

Patch set 1 of change Id25b85de039797aa7e39225fb0e2b1c75207a505 has been pushed to the review server.
It is available at http://review.typo3.org/3324

Patch submitted for not disclosing (potentially sensitive) information. Somebody might want to update this issues headline, please.

Patch set 2 of change Id25b85de039797aa7e39225fb0e2b1c75207a505 has been pushed to the review server.
It is available at http://review.typo3.org/3324