Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87310
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
To: "'Aaron Piotrowski'" <aaron@icicle.io>,
	"'Sammy Kaye Powers'" <me@sammyk.me>,
	"'Scott Arciszewski'" <scott@paragonie.com>
Cc: <larry@garfieldtech.com>,
	=?iso-8859-2?Q?'Jakub_Kub=ED=E8ek'?= <kelerest123@gmail.com>,
	"'Stanislav Malyshev'" <smalyshev@gmail.com>,
	<scott@paragonie.com>,
	<rowan.collins@gmail.com>,
	<pierre.php@gmail.com>,
	"'Dean Eigenmann'" <dean.eigenmann@icloud.com>,
	"'Yasuo Ohgaki'" <yohgaki@ohgaki.net>,
	"'PHP Internals'" <internals@lists.php.net>
References: <CALYT1Yo-ae+cemXaaXs5Jv5t2DSzdxdYuxuhEGbXyoo_863+bw@mail.gmail.com> <836BA21C-AE99-4E7B-AB06-EBC30E41BA0E@icicle.io> <CAKws9z2PQWT7rSsM5R9TD-O69Ovy7-eifAuF-S0BQhBh3z4h1w@mail.gmail.com> <0AD335F5-35F3-4D98-8F82-800A1EDF8FD8@icicle.io>
In-Reply-To: <0AD335F5-35F3-4D98-8F82-800A1EDF8FD8@icicle.io>
Date: Mon, 27 Jul 2015 08:53:44 +0200
Message-ID: <05f101d0c838$fcdab5e0$f69021a0$@belski.net>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQDrJ9+0MU8FOQ/rKelUCzkNPL+2ggHtSrGPAjHl/tYB/wuMm5+JJJSg
Content-Language: en-us
Subject: RE: [PHP-DEV] Core functions throwing exceptions in PHP7
From: anatol.php@belski.net ("Anatol Belski")

Hi Aaron,

> -----Original Message-----
> From: Aaron Piotrowski [mailto:aaron@icicle.io]
> Sent: Monday, July 27, 2015 5:32 AM
> To: Sammy Kaye Powers <me@sammyk.me>; Scott Arciszewski
> <scott@paragonie.com>
> Cc: Anatol Belski <anatol.php@belski.net>; larry@garfieldtech.com; =
Jakub
> Kub=ED=E8ek <kelerest123@gmail.com>; Stanislav Malyshev
> <smalyshev@gmail.com>; scott@paragonie.com; rowan.collins@gmail.com;
> pierre.php@gmail.com; Dean Eigenmann <dean.eigenmann@icloud.com>;
> Yasuo Ohgaki <yohgaki@ohgaki.net>; PHP Internals =
<internals@lists.php.net>
> Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
>=20
>=20
> > I must have overlooked a detail here.
> >
> > According to
> > https://github.com/tpunt/PHP7-Reference#throwable-interface
> > there are Throwables called Error, as a separate designation from an
> > exception. I didn't see this in the engine exceptions RFC, so I was
> > unaware that was even a thing.
> >
> > In this case, yes, as long as you can wrap it in try/catch blocks,
> > SecurityError which extends Error and/or implements Throwable is an
> > excellent suggestion.
> >
> > Previously, I thought the suggestion was to stick to triggering =
errors
> > (E_ERROR, E_RECOVERABLE_ERROR, etc.).
> >
> > Scott Arciszewski
> > Chief Development Officer
> > Paragon Initiative Enterprises <https://paragonie.com>
> >
> > --
> > PHP Internals - PHP Runtime Development Mailing List To unsubscribe,
> > visit: http://www.php.net/unsub.php
> >
>=20
> I believe some were suggesting triggering an E_ERROR, though most =
E_ERRORs
> in the engine have been replaced with thrown Error exceptions, so I =
think
using
> E_ERROR in this case would be inappropriate.
>=20
> As I suggested in my prior email, I think throwing an instance of =
Error
would be
> appropriate when the functions random_bytes() and random_int() fail.
>=20
> There are several conditions that already cause the engine to throw an
Error (or
> subclass thereof):
>=20
> (1)->method(); // Throws Error
> declare(strict_types=3D1); array_map(1, 1); // Throws TypeError =
require
'file-with-
> parse-error.php'; // Throws ParseError eval("$a[ =3D 1;"); // Throws
ParseError
> 1 << -1; // Throws ArithmeticError
> intdiv(1, 0); // Throws DivisionByZeroError
> 1 % 0; // Throws DivisionByZeroError
>=20
> Of particular interest in the above examples is intdiv(), an internal
function that
> can throw an instance of Error if the denominator is zero.
>=20
> I propose that random_bytes() and random_int() should throw an =
instance of
> Error if the parameters are not as expected or if generating a random
number
> fails. (To avoid further debate about a subclass, the function should
throw just a
> plain instance of Error, it can always be subclassed later without BC
concerns).
>=20
> random_bytes() and random_int() failing closed is very important to
prevent
> misguided or lazy programmers from using false in place of a random =
value.
A
> return of false can easily be overlooked and unintentionally be cast =
to a
zero or
> empty string. A thrown instance of Error must be purposely caught and
ignored
> to produce the same behavior. As Larry pointed out, it is a very =
common
error
> for programmers to not do a strict check using =3D=3D=3D against false =
when
calling
> strpos().
>=20
> Does anyone have a strong objection to the above proposal? If not, =
then I
think
> Sammy should update his PRs to throw an Error so they can be merged =
before
> the next beta release.
>=20
Yes, there is an objection at least on my side. As we was discussing the =
PR
on the github page, the primary goal of the suggestion to move the
discussion to the intarnals list was to create an approach about =
exceptions
in functions. This seems not be the case here. It is not the question
whether exceptions should be thrown in functions - it's almost obvious =
that
they should now after it's the engine behavior. But it is a huge =
question of
the consistency with anything else. That was also the reason why me and
Kalle have changed our minds about your PR converting fatals in the
functions.

The only case where exception is thrown in a function is intdiv() now. =
But
also as mentioned elsewhere - it's something tightly coupled with the
behavior of div/mod in the engine. IMHO it is not building any base for
randomly adding exceptions elsewhere. Neither I don't see as a base that =
the
CSPRNG functions are new. Neither the argument "we can change it later =
to
xyz". The main concern is still not addressed, and even wasn't started =
to be
addressed. It is for nothing to have new "nice" functions with nice and
correct behavior while leaving the old "ugly" functions ugly.

 IMO we should stop building special cases and move to the =
conceptualization
as first. The more special cases exist, the harder it will be in the =
future
to bring the behaviors inline without BC breaks. Till now I haven't seen
even any gentle hint about such conceptualization work going on, no RFC =
page
still exists, but it's being continued pushing on a special case. =
Solving an
immediate issue might be tactically justifiable, but without a =
strategical
thinking will lead to even a bigger issue. It is not something we want =
to
leave behind us for the next generations. I would kindly call therefore, =
to
think about this in a more global way, discuss and show at least a rough
draft about what should be done regarding the existing functions, new
functions, backward compatibility, exact warning/error types and how =
they
should be converted, etc. before pushing further on this case.

Thanks

Anatol