Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87010
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 74.125.82.54 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <1435240516.15676.7.camel@kuechenschabe>
References: <CAF+90c9jJVVOwXGrCYo4yWx=LtfqycccNNS3e8AvAO7PhLkiyA@mail.gmail.com>
	<1435240516.15676.7.camel@kuechenschabe>
Date: Fri, 3 Jul 2015 17:57:32 +0200
Message-ID: <CAF+90c9beR6aFaTHiPKXA-sjydOcLSeu8+8_7N9z4MaMaVYkjw@mail.gmail.com>
To: =?UTF-8?Q?Johannes_Schl=C3=BCter?= <johannes@schlueters.de>
Cc: PHP internals <internals@lists.php.net>, Dmitry Stogov <dmitry@zend.com>
Content-Type: multipart/alternative; boundary=047d7bf0bfc225ed390519fa9dfb
Subject: Re: [PHP-DEV] Allow exceptions in __toString()
From: nikita.ppv@gmail.com (Nikita Popov)

--047d7bf0bfc225ed390519fa9dfb
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Thu, Jun 25, 2015 at 3:55 PM, Johannes Schl=C3=BCter <johannes@schlueter=
s.de>
wrote:

> On Thu, 2015-06-25 at 14:52 +0200, Nikita Popov wrote:
> > However what it doesn't do, and what I wouldn't consider feasible to do=
,
> is
> > ensure that every single string conversion in library functions is
> > exception safe. Personally I don't think this is a blocking issue, as t=
he
> > worst that can happen is usually an additional superfluous warning to b=
e
> > thrown, or something similar. If cases like this turn up, we can
> > specifically target them.
>
> I don't agree to the assesment that this isn'T a problem. Consider this
> extension pseudo-code:
>
>
>    zval *data =3D get_data();
>    convert_to_string(data);
>    store_into_database(Z_STRVAL_P(data));
>    return TRUE;
>
> This will store wrong data in the database and report to the user that
> there was an error before storing, so the user assumes nothing was
> stored.
>

I can see your concern here -- however this is nothing specific to
exceptions thrown from __toString(). There is are a number of ways you can
end up in this situation, the two most common being:

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().

Given that we rely on best-effort exception handling in so many cases, it's
not clear to me why exceptions thrown from __toString() should be expressly
forbidden.

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
regardless of where the exception came from. But as said, I think this is
only tangential to the question of __toString(), as this is a general issue=
.

Nikita

--047d7bf0bfc225ed390519fa9dfb--