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....@belski.net> wrote: > Hi Aaron, > >> -----Original Message----- >> From: Aaron Piotrowski [mailto:aa...@icicle.io] >> Sent: Monday, July 27, 2015 5:32 AM >> To: Sammy Kaye Powers <m...@sammyk.me>; Scott Arciszewski >> <sc...@paragonie.com> >> Cc: Anatol Belski <anatol....@belski.net>; la...@garfieldtech.com; Jakub >> Kubíček <kelerest...@gmail.com>; Stanislav Malyshev >> <smalys...@gmail.com>; sc...@paragonie.com; rowan.coll...@gmail.com; >> pierre....@gmail.com; Dean Eigenmann <dean.eigenm...@icloud.com>; >> Yasuo Ohgaki <yohg...@ohgaki.net>; PHP Internals <internals@lists.php.net> >> 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_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=1); array_map(1, 1); // Throws TypeError require > 'file-with- >> parse-error.php'; // Throws ParseError eval("$a[ = 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 === against false when > 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 before >> 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 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 > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
