Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:68991
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.180 as permitted sender)
MIME-Version: 1.0
Sender: mike.php.net@gmail.com
In-Reply-To: <CALJZF3hC-bb33n9RvG0zsuMTyBA1SViAjvxmuorhOrbqrsxLHg@mail.gmail.com>
References: <CALJZF3iw7uzUN2z-aBfO34c=x9tY-gCFaDHRRZq-JYpxHgdmrg@mail.gmail.com>
	<521D174D.4030802@nebm.ist.utl.pt>
	<CALJZF3hC-bb33n9RvG0zsuMTyBA1SViAjvxmuorhOrbqrsxLHg@mail.gmail.com>
Date: Tue, 10 Sep 2013 13:29:08 +0200
Message-ID: <CALJZF3gwbXs4DKA1E2PAYeHUd8WLz=ZSTrqaxqk9BS2oQ11KOw@mail.gmail.com>
To: Gustavo Lopes <glopes@nebm.ist.utl.pt>
Cc: PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] PROPOSAL: temp stream for post_data
From: mike@php.net (Michael Wallner)

On 28 August 2013 08:14, Michael Wallner <mike@php.net> wrote:
> Hi Gustavo, thank you for your review!
>
> On 27 August 2013 23:17, Gustavo Lopes <glopes@nebm.ist.utl.pt> wrote:

>> I think it's generally a good idea, but I have some concerns:
>>
>> * The whole point of PG(enable_post_data_reading) is to be able to read the
>> input of the fly. If I read your patch correctly, the data is completely
>> gobbled when you open php://input and then the temp file is reminded. This
>> will result in a serious performance degradation on large inputs, as
>> processing can only start after everything has been read. The behavior
>> should be different if PG(enable_post_data_reading) is false.
>
> Ok, *I* thought the whole point of enable_post_data_reading is not to
> swamp your memory :)  We can have that behaviour of course, but then
> the input stream is not reusable.
...
>> * The php://input streams simply forward the calls to
>> SG(request_info).request_body, so if you open two, the per-stream cursor
>> position may not match the position of the wrapped stream (even if it
>> matched, we wouldn't want one stream to affect the position of the other). I
>
> True! So my input stream handles concurrency as bad as the current
> implementation... 8)
>
>> don't see any easy way out here; all I can think of is is duping the file
>> descriptor for the temporary file in these situations. But then the book
>> keeping for php://input would be more complicated.
>
> I think that shouldn't be too hard to fix.  But it depends on concern no.1 :)

Fixed. The input stream is reusable *and* may be used JIT.

https://github.com/m6w6/php-src/compare/slim-postdata-merge

-- 
Regards,
Mike