On Tue, Feb 5, 2019 at 5:55 PM Zeev Suraski <z...@php.net> wrote: > On Tue, Feb 5, 2019 at 5:17 PM Nikita Popov <nikita....@gmail.com> wrote: > >> On Mon, Nov 26, 2018 at 10:42 PM Nikita Popov <nikita....@gmail.com> >> wrote: >> >> >> I'd like to move forward with this change. I think the overall reception >> here has been positive, although in the discussion some other >> possibilities >> that avoid/reduce the BC aspect have been discussed. I think now that we >> have a PHP 8 branch, it would make sense to apply this as-is. The BC break >> is quite minor (compared to the other changes in PHP 8) and I think this >> is >> the cleanest way to solve the problem, as it only changes the list of >> silences errors, without introducing any new error handling concepts or >> mechanisms. > > > I don't think that other changes that may or may not make it into PHP 8 > should influence our decision - BC breaks accumulate and the more you have > of them, the more difficult it is to migrate. I'm also present unaware of > anything we already decided to 'break' in PHP 8 as of now (with the > exception of the removal of deprecated 7.x features). > > That said - I think we all agree the BC breakage scope is small - but at > the same time, it may be quite fatal for those that are affected. Those > who have display_errors on (which is both horrible for production and at > the same time fairly popular) are risking exposing sensitive data that > beforehand, they were safely and explicitly hiding using @. >
I can see the general concern here, but I'm having a hard time imagining that this will be an issue in practice. If you have display_errors=on you are at risk of leaking information in error messages with or without this change. The prime example of leaking information, which is passwords contained in the exception stack trace of a failed PDO connection, isn't even affected by this, because the silencing will be removed as part of exception unwinding anyway. > Is there any reason *not* to do it in such a way that provides a gentler > migration path? Introduce a new error level that would be a part of E_ALL > in 7.4, but outside of E_ALL in 8.0 - that would govern whether fatal > errors are silenced or not. > The reason not to do it is pretty much the same as always: Adding that new error level is basically the same as adding a new ini setting (and if we do want that, then I think it should be it's own ini setting and not hacked in as part of error_reporting), and you know our usual opinion on that topic. Furthermore, in this particular case it would also defeat the purpose of the change. If we set the option such that fatals are silenced by default, then we may as well not make the change, because the people who benefit most from it (non-expert users) are not going to change that default. Conversely, if we do not silence fatals by default, then we don't solve the issue with display_errors=on you mentioned above (as it requires explicitly setting an option, in which case they could just set display_errors=off instead.) Nikita
