I commited your change as it fixes a bug, thanks for the patch :) I also followed your idea of a server to test http streams (ftp extension does this, too) and wrote a test for this bug. Other http tests are welcome if you want to write some.
Regards, Arnaud On Sat, 2009-05-16 at 16:42 +0800, Tjerk Anne Meesters wrote: > On a related note, I've been working on a phpt to verify the behaviour > during which I've noticed a lack of test cases for networked functions > in general. > > The solution I have in mind depends on pcntl_fork(), which practically > takes out Windows test targets. It essentially forks out a child to > perform the test while the parent starts a socket_server. > > I suppose it would be nice if the test framework could somehow be > extended to pose as a webserver to verify typical http behaviour > inside stream wrappers =D > > This can't be the first time that this has been a point of discussion; > my apologies if this has been outright rejected before ;-) > > On 5/16/09, Arnaud Le Blanc <lbarn...@php.net> wrote: > > > > On Fri, 2009-05-15 at 11:49 -0400, Ilia Alshanetsky wrote: > >> the patch looks good. > > > > Same here. Lukas, Johannes, can this be commited to 5.3 ? > > > > Regards, > > > > Arnaud > > > > > >> > >> > >> Ilia Alshanetsky > >> > >> > >> > >> > >> On 15-May-09, at 11:36 AM, Tjerk Anne Meesters wrote: > >> > >> > Hi Arnaud, > >> > > >> > Thanks for the tip, please find patch attached. > >> > > >> > > >> > Best, > >> > Tjerk > >> > > >> > On Fri, May 15, 2009 at 10:58 PM, Arnaud Le Blanc <lbarn...@php.net> > >> > wrote: > >> >> Hi, > >> >> > >> >> On Fri, 2009-05-15 at 22:16 +0800, Tjerk Anne Meesters wrote: > >> >>> Hi, > >> >>> > >> >>> I've been extending the pecl/oauth code to work with php stream > >> >>> wrappers in cases whereby curl is not enabled, but I've hit a snag. > >> >>> > >> >>> The documentation states that enabling the "ignore_errors" flag will > >> >>> fetch the content even on failure status codes. At the same time, > >> >>> setting "max_redirects" to <= 1 indicates that no redirects will be > >> >>> followed. > >> >>> > >> >>> It does not state that setting "max_redirects" <= 1 would actually > >> >>> trigger an error, though in the code it returns (php_stream *)NULL > >> >>> making it impossible to even get the headers out of the wrapperdata > >> >>> hash unless STREAM_ONLY_GET_HEADERS is used. > >> >>> > >> >>> So, either setting "max_redirects" <= 1 should not throw NULL back > >> >>> into the caller code or "ignore_error" behaviour should be > >> >>> extended to > >> >>> prevent that from happening. > >> >>> > >> >>> My attached patch favours to fix the latter, since the > >> >>> "ignore_errors" > >> >>> is a relatively new flag. Hope it will be accepted for the 5.3 > >> >>> release. > >> >> > >> >> max_redirects's purpose is to avoid infinite loops, etc. Stream > >> >> functions are expected to return an error in case the limit is > >> >> exceeded. > >> >> If they do not, scripts will get an empty body without noticing the > >> >> error (changing this may break existing scripts). > >> >> > >> >> ignore_errors is a new flag, it forces scripts to check HTTP status > >> >> code > >> >> (so that scripts will notice non-200 ones) and changing it to ignore > >> >> redirects when max_redirects is exceeded seems to be a good solution. > >> >> > >> >>> > >> >>> (not sure whether attachments will be allowed ... give me a ping > >> >>> if it > >> >>> doesn't make it through) > >> >> > >> >> The list accepts only text/plain attachments (try renaming your patch > >> >> to .txt). > >> >> > >> >> > >> >> Regards, > >> >> > >> >> Arnaud > >> >> > >> >> > >> >> > >> > > >> > > >> > > >> > -- > >> > -- > >> > Tjerk > >> > <http_fopen_wrapper.c.patch.txt>-- > >> > PHP Internals - PHP Runtime Development Mailing List > >> > To unsubscribe, visit: http://www.php.net/unsub.php > >> > > > > > > -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
