Re: extending ignore_errors inside http_fopen_wrappers.c

But ignore_errors is not used in 5.2 right? ;-) I also have a patch for that actually if you're interested =D On 5/17/09, Arnaud Le Blanc <lbarnaud@php.net> wrote: > Yes, the fix requires "ignore_errors", which is new in 5.3. The bug > can't be fixed in 5.2 without breaking compatibility. > > Regards, > > Arnaud > > On Sun, 2009-05-17 at 02:02 +0300, Jani Taskinen wrote: >> Last 7 commits or so, none went to PHP_5_2. Intentionally? >> We usually fix bugs in the branch they're reported for also.. >> >> --Jani >> >> >> Arnaud Le Blanc kirjoitti: >> > 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 <lbarnaud@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 >> >>>>> <lbarnaud@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 >> >>> >> >> >> > >> > >> > > -- -- Tjerk