Johannes, > Some comments on the "error behavior" part: > > E_WARNING - When CRYPT is not included in core (was disabled > compile-time, or is listed in disabled_functions declaration) > > Disabling a different function should have no effect. This is not > intuitive. If crypt is a dependency and is not available this function > shouldn't be available either.
Hrm. Well, then I guess I could re-implement against crypt internally. That would take either a slight re-implementation of the crypt() internals, or slight refactoring of the PHP_FUNCTION(crypt) function to enable c calls to it (even if it's disabled). I don't like the concept of core functions disappearing if they are not implemented. I think that does nothing but make it harder on the developers (now having to inject a function_exists(), etc). Additionally, since this is a security issue, I think that always defining the function is the better approach. Otherwise, you can wind up in a situation where someone else comes in and implements function password_verify($password, $hash) { return true; }, which would be all sorts of bad... I can see the static linking to the function (instead of the dynamic call that's there), So in this case, I personally think the warning is appropriate. > E_WARNING - When supplied an incorrect number of arguments. > E_WARNING - When supplied a non-string first parameter (password) > > This should follow common semantics of zend_parse_parameters(... "s"). > i.e. it has to support objects with __toString(). Also other scalars are > fine. (if they can be casted to string) > > E_WARNING - If a non-string salt option is provided > > As above. Yeah, I guess that's fair. Is there a macro or function like Z_REPRESENTABLE_AS_STRING_P() or something? To make it easier to check? > If any error is raise, false is returned by the function. > > In http://de.php.net/functions.internal it is documented that internal > functions return NULL on error during parameter parsing. New exceptions > for that should have a good reason. The good reason is consistency. Otherwise there would be three return values, `null`, `false` and `true` for password_verify(). Therefore, a check of `false === password_verify($foo)` would actually fail inadvertantly. My opinion is that it should do what's appropriate. The other 2 I can live with returning null (although I disagree with it significantly), but password_verify I think really should only ever return a boolean... > These things are all minor and you might consider them bad, but then > change it globally, not by adding new inconsistencies. Fair enough... Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php