Re: Stricter error handling in mcrypt extension

On Wed, Mar 5, 2014 at 1:56 PM, Derick Rethans <derick@php.net> wrote: > On Wed, 5 Mar 2014, Nikita Popov wrote: > > > On Tue, Mar 4, 2014 at 9:32 PM, Andrey Andreev <narf@devilix.net> wrote: > > > > > Speaking of bugs in MCrypt and IVs ... in ECB mode it complains if > > > you don't pass an IV, even though it is ignored afterwards. > > > > You're probably referring to mcrypt_generic here, rather than > > mcrypt_encrypt. I can bring that function in line with mcrypt_encrypt, > > i.e. add the same error checks and make the IV only required if the > > mode requires it. > > > > However I'm not sure what kind of return value I should use with this > > function. Currently it returns a long result, which is 0 on success > > and a negative number on error. However mcrypt does not define error > > codes for all possible error conditions, e.g. while it has a code for > > invalid key sizes, it doesn't have a code for invalid IV sizes. > > > > Personally I'd just switch it to true/false for success/error, as the > > warnings already tell you what kind of error occurred. Would that be > > okay with you, Derick? > > No - as that is a BC break of a deliberate (though crappy ;-)) API. The > result values of mcrypt_generic_init() (which is I think what you're > refering too) are documented too: > > The function returns a negative value on error: -3 when the key > length was incorrect, -4 when there was a memory allocation problem > and any other return value is an unknown error. If an error occurs > a > warning will be displayed accordingly. FALSE is returned if > incorrect parameters were passed. > > cheers, > Derick > Yeah, it would be kinda ugly to have "falsy" be success in one version and be failure in the next... I applied the changes to mcrypt_encrypt/mcrypt_decrypt/mcrypt_{MODE} now and left mcrypt_generic_init() alone. Most people should be using the encrypt/decrypt functions anyway. Nikita