[PATCH] Fix for php://input returning duplicate data in some cases

php.internals

Jozef Hatala

21 years ago
In some cases, the php://input stream returns incorrect data. This was discovered as a data corruption when directly parsing a POSTed XML with a long text-node. The problem is between main/streams/streams.c and ext/standard/php_fopen_wrapper.c. The XML parser requests data from the stream in 4000 byte chunks. _php_stream_read requests data from the handler for php://input in 8192 byte chunks. It uses two counters stream->readpos and stream->writepos to handle the buffering. Just before returning the next 4000 byte chunk, _php_stream_read advances the stream->position. The handler for php://input in php_stream_input_read uses stream->position to address the contents of SG(request_info).raw_post_data and serves a 8192 byte chunk starting at that position. The problem shows itself on the third call of _php_stream_read. In the first iteration of the while(size>0) loop, it uses the 192 bytes left over in the buffer. In the second iteration it needs to refill to buffer, so it calls php_stream_input_read. But stream->position has not been advanced by 192 yet, so php_stream_input_read ends up returning a buffer starting with the same 192 bytes. The patch consists in simply updating stream->position immediately, instead of accumulating didread and only updating stream->position at the end. This way php_stream_input_read always sees an accurate value of the stream->position. Here it is. It is made against php-5.0.3, but it also applies to php5-200503161930. ==== begin ==== diff -ru php-5.0.3.orig/main/streams/streams.c php-5.0.3/main/streams/streams.c --- php-5.0.3.orig/main/streams/streams.c 2004-11-15 15:44:14.000000000 -0800 +++ php-5.0.3/main/streams/streams.c 2005-03-16 13:08:02.000000000 -0800 @@ -560,6 +560,7 @@ size -= toread; buf += toread; didread += toread; + stream->position += toread; } /* ignore eof here; the underlying state might have changed */ @@ -584,6 +585,7 @@ } if (toread > 0) { didread += toread; + stream->position += toread; buf += toread; size -= toread; } else { @@ -596,10 +598,6 @@ break; } - if (didread > 0) { - stream->position += didread; - } - return didread; } ==== end ====

Jani Taskinen

21 years ago
Are there any bug reports about this at bugs.php.net ? If not, enter one and put the patch online somewhere were we can download it as text file. (and the url to that into the bug report) Does this problem exist with PHP_4_3 branch? --Jani On Wed, 16 Mar 2005, Jozef Hatala wrote:
> In some cases, the php://input stream returns incorrect data. > This was discovered as a data corruption when directly parsing > a POSTed XML with a long text-node. > > The problem is between main/streams/streams.c and > ext/standard/php_fopen_wrapper.c. > > The XML parser requests data from the stream in 4000 byte > chunks. > > _php_stream_read requests data from the handler for php://input > in 8192 byte chunks. It uses two counters stream->readpos > and stream->writepos to handle the buffering. Just before > returning the next 4000 byte chunk, _php_stream_read advances > the stream->position. > > The handler for php://input in php_stream_input_read > uses stream->position to address the contents of > SG(request_info).raw_post_data and serves a 8192 byte chunk > starting at that position. > > The problem shows itself on the third call of _php_stream_read. > In the first iteration of the while(size>0) loop, it uses the > 192 bytes left over in the buffer. In the second iteration it > needs to refill to buffer, so it calls php_stream_input_read. > But stream->position has not been advanced by 192 yet, so > php_stream_input_read ends up returning a buffer starting with > the same 192 bytes. > > The patch consists in simply updating stream->position > immediately, instead of accumulating didread and only updating > stream->position at the end. This way php_stream_input_read > always sees an accurate value of the stream->position. > > Here it is. It is made against php-5.0.3, but it also applies > to php5-200503161930. > > ==== begin ==== > diff -ru php-5.0.3.orig/main/streams/streams.c php-5.0.3/main/streams/streams.c > --- php-5.0.3.orig/main/streams/streams.c 2004-11-15 15:44:14.000000000 -0800 > +++ php-5.0.3/main/streams/streams.c 2005-03-16 13:08:02.000000000 -0800 > @@ -560,6 +560,7 @@ > size -= toread; > buf += toread; > didread += toread; > + stream->position += toread; > } > > /* ignore eof here; the underlying state might have changed */ > @@ -584,6 +585,7 @@ > } > if (toread > 0) { > didread += toread; > + stream->position += toread; > buf += toread; > size -= toread; > } else { > @@ -596,10 +598,6 @@ > break; > } > > - if (didread > 0) { > - stream->position += didread; > - } > - > return didread; > } > > ==== end ==== > >
-- https://www.paypal.com/xclick/business=sniper@php.net&no_note=1&tax=0&currency_code=EUR

Jozef Hatala

21 years ago
I did not find a matching bug entry. Created #32371 for it. The exacly same code is also in php-4.3.10, I made a diff against that version (streams.c is in a different dir), too. Description and patches : http://thepathetic.com/jozef/php_input_stream_patch.html http://thepathetic.com/jozef/php-5.0.3-input_stream.patch http://thepathetic.com/jozef/php-4.3.10-input_stream.patch If curious, you can reproduce it against an unpatched php-5.0.3 like this: $ wget http://thepathetic.com/jozef/data.xml $ curl -s --data-binary '@data.xml' http://thepathetic.com/jozef/lala2.php | cmp - data.xml The reply will be: - data.xml differ: byte 8193, line 1 Here is lala2.php : $ wget -O lala2.php http://thepathetic.com/jozef/lala2.php.txt jh On Sat, Mar 19, 2005 at 01:32:09AM +0200, sniper wrote:

D . Walsh

21 years ago
On Mar 18, 2005, at 20:46, Jozef Hatala wrote:
> I did not find a matching bug entry. Created #32371 for it. > > The exacly same code is also in php-4.3.10, I made a diff > against that version (streams.c is in a different dir), too. > > Description and patches : > http://thepathetic.com/jozef/php_input_stream_patch.html > http://thepathetic.com/jozef/php-5.0.3-input_stream.patch > http://thepathetic.com/jozef/php-4.3.10-input_stream.patch > > If curious, you can reproduce it against an unpatched php-5.0.3 like > this: > > $ wget http://thepathetic.com/jozef/data.xml > $ curl -s --data-binary '@data.xml' > http://thepathetic.com/jozef/lala2.php | cmp - data.xml > > The reply will be: > - data.xml differ: byte 8193, line 1 > > Here is lala2.php : > > $ wget -O lala2.php http://thepathetic.com/jozef/lala2.php.txt > > jh > > On Sat, Mar 19, 2005 at 01:32:09AM +0200, sniper wrote: >> >> Are there any bug reports about this at bugs.php.net ? >> If not, enter one and put the patch online somewhere were >> we can download it as text file. (and the url to that into >> the bug report) >> >> Does this problem exist with PHP_4_3 branch? >> >> --Jani
Nice simple patch that gets the job done!!!!! Hopefully it will find it's way into the release of 4.3.11. -- Dale