Re: About restricting the recursive implicit calls

The overhead of the patch is negligible, so it's not a big problem to include it from perf point of view, however: 1) It won't prevent crash because of C stack overflow, in case the limit is too big. 2) Disable check with some default value (0 or -1) would be also good 3) I'm not sure about naming. It would be better to use something more clear in INI name and error message, May be instead of this, we should check C stack pointer in zend_call_function() and throw exception or error, when the pointer comes close to the limit? This check must be compiler and may be platform specific :( Thanks. Dmitry. On Fri, Jun 6, 2014 at 10:25 AM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > > > > On Wed, Feb 27, 2013 at 10:15 PM, Ferenc Kovacs <tyra3l@gmail.com> wrote: > >> >> >> >> On Wed, Feb 27, 2013 at 9:06 PM, Stas Malyshev <smalyshev@sugarcrm.com> >> wrote: >> >>> Hi! >>> >>> > May someone merge this PR (#290) as there are no arguments against it? >>> >>> I just outlined arguments against it in my last emails. If your email is >>> not working properly and you miss some emails please check the list >>> archives. >>> >>> > Or do I have to wait a little bit? (How long?) >>> >>> In my personal opinion, indefinitely long, since I think it is not a >>> good idea at all to have it in the core, and the same functionality >>> already exists in xdebug. >>> >>> >> and in suhosin. >> I think that it is a pretty handy feature, both from the security >> pov(hitting a fatal error is better than php crashing, killing the workers, >> resetting random seed, etc.) and can help with debugging the problem(as you >> will lose the non flushed output buffer on the crash). >> so if we could find an implementation which addresses your issues(no >> major perf cost, sane defaults, no edge cases left out, etc.) then I can't >> see why we shouldn't add it. >> >> -- >> Ferenc Kovács >> @Tyr43l - http://tyrael.hu >> > > sorry for resurecting the old thread, > if we are concerned about the perf cost of counting and checking the > recursion, and that the default value would break some application, maybe > we could introduce this as an optional feature: the ini setting is 0 by > default, which will disable even the checks for recursion, so people have > to opt-in to use it and it's their burden to configure it properly. > I know that this would still allow distros and hosters to misuse it, and > probably some apps will start checking for this setting to (hopefully only) > warn about the setting being too low, or even picking a different strategy > when the setting is too low or simply increasing the limit at runtime. > maybe we can do something in the next major version which would allow us > to gracefully bail from these kind of errors without imposing arbitrary > limits? > > -- > Ferenc Kovács > @Tyr43l - http://tyrael.hu >