Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:79059
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ajf.me designates 192.64.116.200 as permitted sender)
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 8.1 \(1993\))
In-Reply-To: <CADajX4Ctcn7HHu_YbxunT1TALbx8Koe1F4pOO_YwjKv5n8=Vgg@mail.gmail.com>
Date: Fri, 21 Nov 2014 02:06:56 +0000
Cc: Levi Morrison <levim@php.net>,
 PHP Internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <5A250A28-F5C2-4FF1-B144-C19B2220EADC@ajf.me>
References: <D7C8CEF6-625E-4489-8EFC-8FCAA16302D2@ajf.me> <CAFMT4NodhUcS3f-nnCpxGuR=AA6-g9wTchzeS7faTeu9SSL5xw@mail.gmail.com> <CADajX4Ctcn7HHu_YbxunT1TALbx8Koe1F4pOO_YwjKv5n8=Vgg@mail.gmail.com>
To: Adam Harvey <aharvey@php.net>
Subject: Re: [PHP-DEV] [VOTE][RFC] Safe Casting Functions
From: ajf@ajf.me (Andrea Faulds)


> On 21 Nov 2014, at 00:45, Adam Harvey <aharvey@php.net> wrote:
>=20
> My -1 is pretty much the same as Levi's:
>=20
> On 19 November 2014 13:57, Levi Morrison <levim@php.net> wrote:
>>  - The RFC does not address how this is different from
>> FILTER_VALIDATE_* from ext/filter. I know there was a mention of this
>> on the mailing list, but the RFC should say why a tool that already
>> exists to solve this kind of problem is insufficient, especially when
>> it is enabled by default.

While that was true when Levi said it, the RFC *does* address this now.

>=20
> I'm also somewhat concerned that these functions are conflating two
> concerns (validation and conversion).

Ideally, usage of these functions should be accompanied by some sort of =
validation logic if necessary for the application. They aren=E2=80=99t =
validation functions in themselves, they just prevent certain =
conversions that don=E2=80=99t make sense. Say someone fails to do =
proper validation and does this:

    $user =3D User::get((int)$_GET[=E2=80=98id=E2=80=99]);

With this code, absolutely anything could be passed for =E2=80=9Cid=E2=80=9D=
 in the URL parameter and it=E2=80=99d be converted to a valid integer, =
transforming garbage into apparently valid values.

On the other hand, say they did this:

    $user =3D User::get(to_int($_GET[=E2=80=98id=E2=80=99]));

If I pass something that is not a number (like =E2=80=9Cfoobar=E2=80=9D), =
it would throw an exception.

That might not be the best example, I suppose, but it does show where it =
would hopefully lead to less broken software. A better example might be =
conversion from an integer to a float within an application. If (int) is =
used, the data can be horribly mangled beyond recognition (overflow, =
INF/NAN handling) without so much as a warning. If to_int() is used, =
then it will throw an exception in this circumstance.

Basically, these functions aren=E2=80=99t meant to be substitutes for =
validation. Instead, they prevent dangerous conversions. The hope is =
that people, when needing to convert a value between types, will use =
these functions rather than the more dangerous explicit casts, and their =
applications can failsafe if something goes wrong.

>=20
>>  - PHP suffers a lot from function bloat and this RFC provides
>> multiple functions that do the same thing but differ only in how they
>> handle errors. A simple validation of "can this be safely cast to an
>> integer without dataloss?" avoid the issue entirely and would be =
fewer
>> functions.
>=20
> +1: the idea of adding two sets of identical functions except for how
> they signal errors isn't one I like.

The problem is that it=E2=80=99s the least bad of the available options.

> Derick raised a good point elsethread: this is really tied into how we
> want to signal errors in PHP 7. If we switch to exceptions, then let's
> figure out a plan of attack and switch to exceptions everywhere, not
> just in the odd function here and there. If we don't, then the to_*
> functions shouldn't be added.

This isn=E2=80=99t a switch, though, these are new additions. We already =
have exceptions in a few places in core, and there=E2=80=99s nothing =
wrong with new additions using exceptions if and when appropriate. The =
question is over how to deal with legacy code.

In this specific case, I suppose the other option would have been to =
throw an E_RECOVERABLE_ERROR. But exceptions can be handled easier, and =
so I don=E2=80=99t see a particular reason why it shouldn=E2=80=99t =
throw an exception.

--
Andrea Faulds
http://ajf.me/