Re: extending ignore_errors inside http_fopen_wrappers.c

... I don't know why I never scrolled down to the changelog section of the documentation where it mentions that this was introduced in 5.3.0 onwards (ahem); still, since the introduction of this parameter was only inside this wrapper it could still be backported to 5.2.10 right? ;-) On 5/17/09, Tjerk Anne Meesters <datibbaw@php.net> wrote: > Since I'll be out most of today, I'll just post the patch first; it'd > be great if it could also be applied to the 5_2 branch of course, but > that's not my call ;-) > > As for compatibility, by default the patch would not change anything > to the current 5.2 behaviour, so I'm not sure how this poses a > problem. In addition, the documentation already states the > "ignore_errors" settings without mentioning the version it was > introduced in; a simple "introduced in 5.2.10" would work, no? > > Reference documentation: > http://sg.php.net/manual/en/context.http.php > > > Best, > Tjerk > > On Sun, May 17, 2009 at 8:42 AM, Tjerk Anne Meesters <datibbaw@php.net> > wrote: >> 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 >> > > > > -- > -- > Tjerk > -- -- Tjerk