Hi,
The attached patch implements automatic decoding of chunked
transfer-encoding.
It fixes http://bugs.php.net/bug.php?id=47021 but also affects all php
stream functions (e.g. file_get_contents("http://...";);)
Some PHP applications which check for Transfer-Encoding HTTP header and
perform manual decoding might be broken.
Any objections against committing the patch into PHP_5_3?
My be someone has ideas about patch improvements?
Thanks. Dmitry.
Hi!
The attached patch implements automatic decoding of chunked
transfer-encoding.
From what I see in the patch, it requires chunked data prefix to be:
XXX\r\n
where XXX are from 1 to 3 hex digits. However RFC 2616 defines chunk
size as arbitrarily long sequence of HEX digits, and also allows for
some stuff (extensions) to be added to the end. I wonder if it'd be
better to treat all hex digits as size and then ignore everything until
\r\n.
Also, I understand that by the RFC chunked encoding should never be sent
to HTTP/1.0 clients. So, by default this should never happen unless you
manually set protocol version to 1.1 (or the server is broken).
--
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com
The attached patch implements automatic decoding of chunked
transfer-encoding.Any objections against committing the patch into PHP_5_3?
I didn't have objections when I offered this filter several years ago,
and I still don't. I do recall Andi (or perhaps it was someone else)
saying it was adding unnecessary complexity to the http wrapper and
such things should be left for cURL though, and this is why it didn't
go into 5.1
Maybe someone has ideas about patch improvements?
I'd add a context option to disable automatic application of the
filter, and register the filter with the streams layer so that it can
be applied manually. (And of course, I'd second Stas' comment wrt
chunk-header length)
The issue wrt 1.0 versus 1.1 is fairly moot. If the caller doesn't
override the version then the server won't send chunked encoding.
They are capable of doing so however, and they may already be doing it
(e.g. for a streaming client). So the concern about potential BC
breakage for apps still exists, it's just less severe (since I doubt
many apps are explicitly setting the version to 1.1)
-Sara
Hi,
Thanks for comments.
The updated patch fixes RFC incompatibilities. It just ignores
extensions and trailer.
The patch also exports "chunked" filter into user space so now it can be
used with any streams (see test in the patch).
In case user opens http stream and server responds with
"Transfer-Encoding: chunked" header, php applies chunked filter
automatically and doesn't bypass "Transfer-Encoding" header to user.
The automatic decoding may be disabled using stream_context.
stream_context_create(array("http"=>array("auto_decode"=>0)));
Any objections against applying it into 5.3?
I think it's very easy to add automatic decompression of HTTP responses
with HTTP headers "Content-Encoding: gzip" (and others) using similar
patch (except for filters are already implemented and the patch will
need to update only http_fopen_wrapper.c).
Thanks. Dmitry.
Sara Golemon wrote:
The attached patch implements automatic decoding of chunked
transfer-encoding.Any objections against committing the patch into PHP_5_3?
I didn't have objections when I offered this filter several years ago,
and I still don't. I do recall Andi (or perhaps it was someone else)
saying it was adding unnecessary complexity to the http wrapper and
such things should be left for cURL though, and this is why it didn't
go into 5.1Maybe someone has ideas about patch improvements?
I'd add a context option to disable automatic application of the
filter, and register the filter with the streams layer so that it can
be applied manually. (And of course, I'd second Stas' comment wrt
chunk-header length)The issue wrt 1.0 versus 1.1 is fairly moot. If the caller doesn't
override the version then the server won't send chunked encoding.
They are capable of doing so however, and they may already be doing it
(e.g. for a streaming client). So the concern about potential BC
breakage for apps still exists, it's just less severe (since I doubt
many apps are explicitly setting the version to 1.1)-Sara
Hi,
Some PHP applications which check for Transfer-Encoding HTTP header and
perform manual decoding might be broken.
What about not adding the Transfer-Encoding header to wrapper_data ?
(not making it visible so that scripts would not break). Also,
re-enabling the filters list in stream_get_meta_data() so that one can
know that the dechunk filter has been added.
Regards,
Arnaud
I sent a new version of patch with these features a minute ago :)
Thanks. Dmitry.
Arnaud Le Blanc wrote:
Hi,
Some PHP applications which check for Transfer-Encoding HTTP header and
perform manual decoding might be broken.What about not adding the Transfer-Encoding header to wrapper_data ?
(not making it visible so that scripts would not break). Also,
re-enabling the filters list instream_get_meta_data()so that one can
know that the dechunk filter has been added.Regards,
Arnaud