Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87013
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.192.179 as permitted sender)
Message-ID: <55971538.5020505@gmail.com>
Date: Fri, 03 Jul 2015 16:05:28 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Nikita Popov <nikita.ppv@gmail.com>, =?UTF-8?B?Sm9oYW5uZXMgU2NobMO8dGU=?=
 =?UTF-8?B?cg==?= <johannes@schlueters.de>
CC: PHP internals <internals@lists.php.net>, 
 Dmitry Stogov <dmitry@zend.com>
References: <CAF+90c9jJVVOwXGrCYo4yWx=LtfqycccNNS3e8AvAO7PhLkiyA@mail.gmail.com>	<1435240516.15676.7.camel@kuechenschabe> <CAF+90c9beR6aFaTHiPKXA-sjydOcLSeu8+8_7N9z4MaMaVYkjw@mail.gmail.com>
In-Reply-To: <CAF+90c9beR6aFaTHiPKXA-sjydOcLSeu8+8_7N9z4MaMaVYkjw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] Allow exceptions in __toString()
From: smalyshev@gmail.com (Stanislav Malyshev)

Hi!

> a) Error handlers can (and often do) throw. E.g. it is possible to convert
> the two recoverable fatal errors that __toString() currently throws into
> exceptions and end up in the situation you describe, only with even less
> safety and more leaks. However this applies to all occurrences of
> zend_error or php_error_docref that generate a non-fatal error. From a
> quick grep I estimate we have about 4000 places in the codebase where this
> can occur and I'm sure many of them do not check for EG(exception)
> immediately after throwing the error.
> 
> b) Destructors can throw. We have approximately 2500 places in php-src
> destroying a zval, which could potentially throw. I don't think I've ever
> seen an EG(exception) check after a zval_ptr_dtor().

You don't usually use the value being destroyed after zval_ptr_dtor().
You could, however, use the value that was supposed to be converted
after __toString, and you probably are if you're converting it. The
problem is not that exception is thrown, the problem is that the code
assumes there's some workable value and there's none or at least not the
value it expected.

With error handlers, it is trickier but the code that produces the error
usually does not expect everything to be fine after the error handler is
done - on the contrary, it explicitly expects it *not* to be fine, since
it just reported that something is not fine by invoking the error
handler! Of course, we have there all kinds of issues with error
handlers messing with the environment, but it's much harder to cause
this problem by accident.

The problem, as I see it, is not the throwing of exception by itself,
but the fact that the code assumes the defined state of the variable
being converted, but when exception is thrown, we don't get that state,
but we also don't know something bad happened.

Of course, we could do something like set the value to empty string, but
since again there's no check there the code would just assume we've got
legitimate conversion to empty string - which is not the same as "we've
got error" and could lead to very unexpected consequences (such as
somebody erroneously supplying object as a password and system treating
it as empty password).

> If you want to protect against side-effects after exceptions in critical
> places like database interaction, we can introduce exception checks right
> before the operation is executed and prevent anything bad from happening

I don't see how this is possible - we have no idea which places can be
"critical" - any interaction with now-undefined variable could lead to a
disaster downstream.
-- 
Stas Malyshev
smalyshev@gmail.com