Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100653
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAESVnVqi6O7UNmX=jA3nwCLDYXqrPRwZKWYsuoeitjhAX0AGfQ@mail.gmail.com>
References: <097578bf-ab74-44cf-a465-dc6fdd50930f@Spark> <CADyq6sKQ5RqOQvcRK_dSnu+wtHBqwMkoH_Vhxw8NZCi2tk7YPQ@mail.gmail.com>
 <7d703ad8-5596-44e7-95dc-23c2cc058408@Spark> <CAESVnVqi6O7UNmX=jA3nwCLDYXqrPRwZKWYsuoeitjhAX0AGfQ@mail.gmail.com>
Date: Sat, 16 Sep 2017 06:16:17 +0900
Message-ID: <CAGa2bXbbEqRJmcLF7G-UMzOZyu9FJvOcNuXJyp+20C+YkQhHMA@mail.gmail.com>
To: Sara Golemon <pollita@php.net>
Cc: Ilija Tovilo <ilija.tovilo@me.com>, Marco Pivetta <ocramius@gmail.com>, 
	PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a114a9c66cb141e055940e900"
Subject: Re: [PHP-DEV] [RFC] Deprecate the extract function in PHP 7.3
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a114a9c66cb141e055940e900
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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=E2=80=99t 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=E2=80=99s usefulness in this case.
> > But wouldn=E2=80=99t it be better to implement this by hand
> > in these rare cases (it=E2=80=99s 3 lines of code) instead of
> > encouraging the pollution of the symbol table by
> > unknown input? It=E2=80=99s also clearer since people who
> > don=E2=80=99t know the `extract` function probably don=E2=80=99t
> > expect it to mutate the local symbol table.
> >
> Let's be clear on what that looks like: foreach ($data as $key =3D>
> $value) { $$key =3D $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 =3D ture
 - Unintended variable modification. e.g. $admin_flag =3D 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

--001a114a9c66cb141e055940e900--