Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87311
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain paragonie.com from 209.85.192.46 cause and error)
MIME-Version: 1.0
In-Reply-To: <05f101d0c838$fcdab5e0$f69021a0$@belski.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>
	<05f101d0c838$fcdab5e0$f69021a0$@belski.net>
Date: Mon, 27 Jul 2015 02:57:47 -0400
Message-ID: <CAKws9z0yjxWisObfQ323rk3fA78yqJtRG=6zYDvjuPHEKH4=ag@mail.gmail.com>
To: Anatol Belski <anatol.php@belski.net>
Cc: Aaron Piotrowski <aaron@icicle.io>, Sammy Kaye Powers <me@sammyk.me>, 
	Larry Garfield <larry@garfieldtech.com>, =?UTF-8?B?SmFrdWIgS3Viw63EjWVr?= <kelerest123@gmail.com>, 
	Stanislav Malyshev <smalyshev@gmail.com>, rowan.collins@gmail.com, 
	Pierre Joye <pierre.php@gmail.com>, Dean Eigenmann <dean.eigenmann@icloud.com>, 
	Yasuo Ohgaki <yohgaki@ohgaki.net>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
From: scott@paragonie.com (Scott Arciszewski)

My only concern is that, we have a fixed timetable for the 7.0.0
release. It's certainly a good idea to develop a cogent strategy
before moving forward, but I worry that bikeshedding will get involved
and we won't make the deadline.

I propose that, if a better strategy cannot be presented in time for
RC1, consistency should take a back seat to security. Fail hard, throw
an Error, deal with the details later.
Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises


On Mon, Jul 27, 2015 at 2:53 AM, Anatol Belski <anatol.php@belski.net> wrot=
e:
> 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=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.ne=
t>
>> Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
>>
>>
>> > 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
>> >
>>
>> I believe some were suggesting triggering an E_ERROR, though most E_ERRO=
Rs
>> in the engine have been replaced with thrown Error exceptions, so I thin=
k
> 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 valu=
e.
> 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 w=
hen
> calling
>> strpos().
>>
>> 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 befo=
re
>> the next beta release.
>>
> 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 exceptio=
ns
> 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 th=
at
> 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. Bu=
t
> 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 conceptualizat=
ion
> as first. The more special cases exist, the harder it will be in the futu=
re
> 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 p=
age
> still exists, but it's being continued pushing on a special case. Solving=
 an
> immediate issue might be tactically justifiable, but without a strategica=
l
> 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
>
>