Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87308
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain icicle.io designates 199.38.81.6 as permitted sender)
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2102\))
In-Reply-To: <CAKws9z2PQWT7rSsM5R9TD-O69Ovy7-eifAuF-S0BQhBh3z4h1w@mail.gmail.com>
Date: Sun, 26 Jul 2015 22:32:25 -0500
Cc: Anatol Belski <anatol.php@belski.net>,
 larry@garfieldtech.com,
 =?utf-8?Q?Jakub_Kub=C3=AD=C4=8Dek?= <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>
Content-Transfer-Encoding: quoted-printable
Message-ID: <0AD335F5-35F3-4D98-8F82-800A1EDF8FD8@icicle.io>
References: <CALYT1Yo-ae+cemXaaXs5Jv5t2DSzdxdYuxuhEGbXyoo_863+bw@mail.gmail.com> <836BA21C-AE99-4E7B-AB06-EBC30E41BA0E@icicle.io> <CAKws9z2PQWT7rSsM5R9TD-O69Ovy7-eifAuF-S0BQhBh3z4h1w@mail.gmail.com>
To: Sammy Kaye Powers <me@sammyk.me>,
 Scott Arciszewski <scott@paragonie.com>
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
From: aaron@icicle.io (Aaron Piotrowski)


> I must have overlooked a detail here.
>=20
> 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.
>=20
> 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.
>=20
> Previously, I thought the suggestion was to stick to triggering errors
> (E_ERROR, E_RECOVERABLE_ERROR, etc.).
>=20
> Scott Arciszewski
> Chief Development Officer
> Paragon Initiative Enterprises <https://paragonie.com>
>=20
> --
> 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.

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.

There are several conditions that already cause the engine to throw an =
Error (or subclass thereof):

(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

Of particular interest in the above examples is intdiv(), an internal =
function that can throw an instance of Error if the denominator is zero.

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).

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.

Aaron Piotrowski