Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72923
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.219.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <alpine.DEB.2.10.1403051253450.6896@whisky.home.derickrethans.nl>
References: <CAPhkiZwsndt-sC+wChyXS03+kDvSKEWoK=jKwGjuhLy1gk7Z1g@mail.gmail.com>
	<CAF+90c9e3YF09Sv-h=f72OGUEFifO8w4ZE2neV=4fijCh3AYsw@mail.gmail.com>
	<alpine.DEB.2.10.1403051253450.6896@whisky.home.derickrethans.nl>
Date: Wed, 5 Mar 2014 15:46:40 +0100
Message-ID: <CAF+90c-2=HCzQVy6-Jw-CD7Ko9GNncRhai-zVH8Q9iU2z+EKVg@mail.gmail.com>
To: Derick Rethans <derick@php.net>
Cc: Andrey Andreev <narf@devilix.net>, "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=047d7b6769e8a76e0504f3dd1687
Subject: Re: [PHP-DEV] Stricter error handling in mcrypt extension
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7b6769e8a76e0504f3dd1687
Content-Type: text/plain; charset=ISO-8859-1

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

--047d7b6769e8a76e0504f3dd1687--