Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:40245
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <48BF8790.1000504@zend.com>
Date: Thu, 04 Sep 2008 11:00:32 +0400
User-Agent: Thunderbird 2.0.0.16 (X11/20080723)
MIME-Version: 1.0
To: Diogo Neves <dafneves@gmail.com>
CC: Arnaud Le Blanc <arnaud.lb@gmail.com>, jani.taskinen@iki.fi, 
 Lukas Kahwe Smith <mls@pooteeweet.org>,
 Marcus Boerger <helly@php.net>, Stanislav Malyshev <stas@zend.com>, 
 PHP Internals List <internals@lists.php.net>
References: <48BCFF84.9080801@zend.com> <48BD0A53.2030202@sci.fi>	 <48BD1226.5010902@zend.com> <200809030030.48091.arnaud.lb@gmail.com> <87ae45950809031333w527f6bob0b8a97893ce18c1@mail.gmail.com>
In-Reply-To: <87ae45950809031333w527f6bob0b8a97893ce18c1@mail.gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] BUG #45392
From: dmitry@zend.com (Dmitry Stogov)

Hi Diogo,

Diogo Neves wrote:
> On Tue, Sep 2, 2008 at 11:30 PM, Arnaud Le Blanc <arnaud.lb@gmail.com>wrote:
> 
>> Hi,
>>
>> On Tuesday 02 September 2008 12:15:02 Dmitry Stogov wrote:
>>> Jani Taskinen wrote:
>>>>>> Dmitry Stogov wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> Attached is a proposed fix for http://bugs.php.net/bug.php?id=45392for
>>>>>>> PHP_5_3. I don't know output buffering part very well, and I'm not
>>>>>>> completely sure about this fix, so please review it.
>>>>>>>
>>>>>>> The patch changes behavior of output buffering a bit. In case of
>> fatal
>>>>>>> error all output buffers opened by ob_start() with zero (or omitted)
>>>>>>> chunk_size argument are discarded. The fix brakes two tests:
>>>>>>>
>>>>>>> Test session_module_name() function : variation
>>>>>>> [ext/session/tests/session_module_name_variation3.phpt]
>>>>>>> Test session_set_save_handler() function : error functionality
>>>>>>> [ext/session/tests/session_set_save_handler_error3.phpt]
>>>>>>>
>>>>>>> We need to make a decision about #45392.
>>>>>>>
>>>>>>> 1) Fix it with proposed (or similar) patch
>>>>>>>
>>>>>>> 2) Make it bogus because any fix requires output buffering behavior
>>>>>>> change
>>>> Sorry for top-posting. Anyway, reading the patch made me think that
>>>> maybe in the user/recoverable cases the output buffer should not be
>>>> discarded like this..?
>>>>
>>>> And IMO, it's a bug, bugs should be fixed even if it means changing the
>>>> behaviour in an _edge_ case.
>>> I'm not so sure as http://www.php.net/manual/en/function.ob-start.php
>>> doesn't make difference between successful and unsuccessful request and
>>> assumes "output buffer is flushed to the browser at the end of the
>> request".
>>
>> I think that it is critical to output this kind of buffer in case of fatal
>> error, as it may unexpectedly leak data which was not meant to be output.
>> It
>> seems Ok to me to discard this kind of buffers in this case.
>>
>>>> How do those session tests fail..?
>>> As they have ob_start() at top, the output is discarded and only error
>>> message is printed.
>>>
>>>> They expect output with fatal errors? (actually those tests failed
>>>> without your patch too, IIRC :)
>>> They works for me without the patch.
>>>
>>> Thanks. Dmitry.
>>>
>>>> --Jani
>>>>
>> Regards,
>>
>> Arnaud
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>> Hi all,
> [Warning] I don't know the insides of php and then it can be a stupid
> question
> 
> eval use ob_start? I say that because, in eval if u have a Parse Error
> inside it will output it without really stop the script... eval doesn't
> change its behavior with this patch?
> 
> If it makes no sense to you guys, simply ignore ;)
> 

Thank you for looking into the patch and your notice.
The patch is really changes behavior in this case

<?php
echo "1\n";
ob_start();
echo "2\n"; // this line is discarded with the patch
eval("1 +++++ 4");
echo "3\n"; // this line is printed with and without patch
?>

Actual output
-------------
1

Parse error: syntax error, unexpected T_INC in Command line code(1) :
eval()'d code on line 1
3
-------------

The proposed patch is not good enough.
I'll think how to fix it.

Thanks. Dmitry.