Re: [RFC] Deprecate the extract function in PHP 7.3

Hi all, On Sat, Sep 16, 2017 at 2:50 AM, Sara Golemon <pollita@php.net> wrote: > On Fri, Sep 15, 2017 at 1:35 PM, <ilija.tovilo@me.com> wrote: > > The extract function takes an associative array and > > puts it into the local symbol table. > > http://php.net/manual/en/function.extract.php > > > > I seriously doubt the usefulness of this function, > > especially looking at the potential risks. The fact > > that overwriting the local variables is the default > > behaviour doesn’t make it any better. I suggest > > deprecating it in PHP 7.3 and removing it in 8. > > > Preface: I despise extract() as well. It's breaks assumptions for > both the developer and the runtime. I save some of my frowniest of > faces for extract(). > > That said... > > > I can see it’s usefulness in this case. > > But wouldn’t it be better to implement this by hand > > in these rare cases (it’s 3 lines of code) instead of > > encouraging the pollution of the symbol table by > > unknown input? It’s also clearer since people who > > don’t know the extract function probably don’t > > expect it to mutate the local symbol table. > > > Let's be clear on what that looks like: foreach ($data as $key => > $value) { $$key = $value; } > > This is SO MUCH WORSE for several reasons, no least of all what > happens when $data contains keys named 'data', 'key', or 'value'. > > I'd like to kill extract(), but it does have a reason for being, and I > couldn't in any good conscience support removing it without a > replacement that's at least marginally better. > The security issue regarding extract() are: - Unintended variable creation. e.g. $admin_flag = ture - Unintended variable modification. e.g. $admin_flag = ture Instead of removing/deprecating extract(), - Add EXTR_EXCEPTION flag that throws exception for overwriting. (There are many flags, but no error/exception flag. This isn't good) - Require prefix. (User may still set '' as prefix) We may do: - Add EXTR_EXCEPTION flag in 7.3 - Make all three parameters required parameter set EXTR_ECEPTION a default flag in 8.0 With this way, we'll have better compatibility with older PHP and better security with extract(). https://github.com/php/php-src/blob/master/ext/standard/array.c#L2459 Since it checks valid flags, we wouldn't have compatibility for current versions unless released versions code is modified for it. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net