# [RFC] Deprecate the extract function in PHP 7.3

- Thread: https://externals.io/message/100637
- Original archive: https://news-web.php.net/php.internals/100637
- Started by ilija.tovilo@me.com on 2017-09-15T17:20:59+00:00
- Messages: 32
- Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL)
- Next page: https://externals.io/message/100637.md?page=2
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #100637 — ilija.tovilo@me.com — 2017-09-15T17:20:59+00:00

URL: https://externals.io/message/100637#100637

Hi!

The `extract` function takes an associative array and puts it into the local symbol table.
http://php.net/manual/en/function.extract.php

```
$array = [
    ‘foo’ => ‘foo’,
    ‘bar’ => ‘bar’,
];

extract($array);

print $foo; // "foo"
```

As a second parameter the `extract` function takes some options to make this function less dangerous, like `EXTR_SKIP` that prevents an existing local variable of being overwritten. There’s a few more options, go ahead and take a look at the documentation. `EXTR_OVERWRITE` is the default one though. You can also pass a prefix for the variable names as a third argument.

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.

In a whole Symfony-Stack (3.4) with all of it’s dependencies I could only find two usages of this function, both of which could be easily rewritten in vanilla PHP:
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Templating/PhpEngine.php#L148
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Templating/PhpEngine.php#L158

Only downside: A polyfill is probably impossible since you cannot mutate the local symbol table of the callee (as far as I’m aware).

Any thoughts?

Regards

---

## #100638 — Marco Pivetta — 2017-09-15T17:26:11+00:00

URL: https://externals.io/message/100637#100638 · In reply to: #100637

Heya,

This is typically used in templating engines.
The one I worked on is
https://github.com/zendframework/zend-view/blob/5523511b6771cb6c060a77f6777426526a8db5ab/src/Renderer/PhpRenderer.php#L491-L492


Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/



> Hi!
>
> The `extract` function takes an associative array and puts it into the
> local symbol table.
> http://php.net/manual/en/function.extract.php
>
> ```
> $array = [
>     ‘foo’ => ‘foo’,
>     ‘bar’ => ‘bar’,
> ];
>
> extract($array);
>
> print $foo; // "foo"
> ```
>
> As a second parameter the `extract` function takes some options to make
> this function less dangerous, like `EXTR_SKIP` that prevents an existing
> local variable of being overwritten. There’s a few more options, go ahead
> and take a look at the documentation. `EXTR_OVERWRITE` is the default one
> though. You can also pass a prefix for the variable names as a third
> argument.
>
> 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.
>
> In a whole Symfony-Stack (3.4) with all of it’s dependencies I could only
> find two usages of this function, both of which could be easily rewritten
> in vanilla PHP:
> https://github.com/symfony/symfony/blob/master/src/
> Symfony/Component/Templating/PhpEngine.php#L148
> https://github.com/symfony/symfony/blob/master/src/
> Symfony/Component/Templating/PhpEngine.php#L158
>
> Only downside: A polyfill is probably impossible since you cannot mutate
> the local symbol table of the callee (as far as I’m aware).
>
> Any thoughts?
>
> Regards

---

## #100639 — Ryan Pallas — 2017-09-15T17:30:44+00:00

URL: https://externals.io/message/100637#100639 · In reply to: #100637

Hi!

The `extract` function takes an associative array and puts it into the
local symbol table.
http://php.net/manual/en/function.extract.php

```
$array = [
    ‘foo’ => ‘foo’,
    ‘bar’ => ‘bar’,
];

extract($array);

print $foo; // "foo"
```

As a second parameter the `extract` function takes some options to make
this function less dangerous, like `EXTR_SKIP` that prevents an existing
local variable of being overwritten. There’s a few more options, go ahead
and take a look at the documentation. `EXTR_OVERWRITE` is the default one
though. You can also pass a prefix for the variable names as a third
argument.

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.

In a whole Symfony-Stack (3.4) with all of it’s dependencies I could only
find two usages of this function, both of which could be easily rewritten
in vanilla PHP:
https://github.com/symfony/symfony/blob/master/src/
Symfony/Component/Templating/PhpEngine.php#L148
https://github.com/symfony/symfony/blob/master/src/
Symfony/Component/Templating/PhpEngine.php#L158

Only downside: A polyfill is probably impossible since you cannot mutate
the local symbol table of the callee (as far as I’m aware).

Any thoughts?


I see no gain by removing this function. I've also seen it used for
templating quite often. Yes the functionality could be changed not to use
extract and end up working the same to the consumer but why make people
rewrite these things for no apparent gain (and likely a small performance
hit)?


Regards

---

## #100640 — ilija.tovilo@me.com — 2017-09-15T17:35:57+00:00

URL: https://externals.io/message/100637#100640 · In reply to: #100638

Hi Marco

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.

Cheers



> Heya,
>
> This is typically used in templating engines.
> The one I worked on is
> https://github.com/zendframework/zend-view/blob/5523511b6771cb6c060a77f6777426526a8db5ab/src/Renderer/PhpRenderer.php#L491-L492
>
>
> Marco Pivetta
>
> http://twitter.com/Ocramius
>
> http://ocramius.github.com/
>
> 
>
> > […]

---

## #100642 — ilija.tovilo@me.com — 2017-09-15T17:38:22+00:00

URL: https://externals.io/message/100637#100642 · In reply to: #100639

Hi Ryan

I can see your argument. The reasoning behind it is that a function in the standard library should not encourage unsafe code. Admittedly, since this function is rarely used except for templating systems one could call this a non-issue. I just wanted to bring it up.

Regards



>
>
> > […]
>
> I see no gain by removing this function. I've also seen it used for templating quite often. Yes the functionality could be changed not to use extract and end up working the same to the consumer but why make people rewrite these things for no apparent gain (and likely a small performance hit)?
>
> > […]

---

## #100641 — Marco Pivetta — 2017-09-15T17:41:25+00:00

URL: https://externals.io/message/100637#100641 · In reply to: #100640

Hi,



Hi Marco

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.

Cheers




Heya,

This is typically used in templating engines.
The one I worked on is
https://github.com/zendframework/zend-view/blob/
5523511b6771cb6c060a77f6777426526a8db5ab/src/Renderer/
PhpRenderer.php#L491-L492


Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/



Hi!

The `extract` function takes an associative array and puts it into the
local symbol table.
http://php.net/manual/en/function.extract.php

```
$array = [
‘foo’ => ‘foo’,
‘bar’ => ‘bar’,
];

extract($array);

print $foo; // "foo"
```

As a second parameter the `extract` function takes some options to make
this function less dangerous, like `EXTR_SKIP` that prevents an existing
local variable of being overwritten. There’s a few more options, go ahead
and take a look at the documentation. `EXTR_OVERWRITE` is the default one
though. You can also pass a prefix for the variable names as a third
argument.

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.

In a whole Symfony-Stack (3.4) with all of it’s dependencies I could only
find two usages of this function, both of which could be easily rewritten
in vanilla PHP:
https://github.com/symfony/symfony/blob/master/src/
Symfony/Component/Templating/PhpEngine.php#L148
https://github.com/symfony/symfony/blob/master/src/
Symfony/Component/Templating/PhpEngine.php#L158

Only downside: A polyfill is probably impossible since you cannot mutate
the local symbol table of the callee (as far as I’m aware).

Any thoughts?

Regards

Absolutely, can be replaced with a loop indeed.

---

## #100643 — Sara Golemon — 2017-09-15T17:50:35+00:00

URL: https://externals.io/message/100637#100643 · In reply to: #100640

> 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.

-Sara

---

## #100644 — Ryan Pallas — 2017-09-15T17:52:51+00:00

URL: https://externals.io/message/100637#100644 · In reply to: #100642

> Hi Ryan
>
> I can see your argument. The reasoning behind it is that a function in the
> standard library should not encourage unsafe code. Admittedly, since this
> function is rarely used except for templating systems one could call this a
> non-issue. I just wanted to bring it up.
>

That makes sense. What about performance difference? On a reasonable sized
array (not 10k items and not 2) which is faster?

extract($array);

- or -

foreach ($array as $key => $value)
    $$key = $value;

Honestly, I don't know (ps 2 lines without braces instead of 3!)


>
> Regards
>
>
> 
>
>
>
> 
>
> Hi!
>
> The `extract` function takes an associative array and puts it into the
> local symbol table.
> http://php.net/manual/en/function.extract.php
>
> ```
> $array = [
>     ‘foo’ => ‘foo’,
>     ‘bar’ => ‘bar’,
> ];
>
> extract($array);
>
> print $foo; // "foo"
> ```
>
> As a second parameter the `extract` function takes some options to make
> this function less dangerous, like `EXTR_SKIP` that prevents an existing
> local variable of being overwritten. There’s a few more options, go ahead
> and take a look at the documentation. `EXTR_OVERWRITE` is the default one
> though. You can also pass a prefix for the variable names as a third
> argument.
>
> 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.
>
> In a whole Symfony-Stack (3.4) with all of it’s dependencies I could only
> find two usages of this function, both of which could be easily rewritten
> in vanilla PHP:
> https://github.com/symfony/symfony/blob/master/src/Symfony/
> Component/Templating/PhpEngine.php#L148
> https://github.com/symfony/symfony/blob/master/src/Symfony/
> Component/Templating/PhpEngine.php#L158
>
> Only downside: A polyfill is probably impossible since you cannot mutate
> the local symbol table of the callee (as far as I’m aware).
>
> Any thoughts?
>
>
> I see no gain by removing this function. I've also seen it used for
> templating quite often. Yes the functionality could be changed not to use
> extract and end up working the same to the consumer but why make people
> rewrite these things for no apparent gain (and likely a small performance
> hit)?
>
>
> Regards

---

## #100645 — Ryan Pallas — 2017-09-15T17:54:19+00:00

URL: https://externals.io/message/100637#100645 · In reply to: #100643

> 
> > […]
> 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...
>
> > […]
> 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'.
>

This is a very good point. I hadn't thought about keys being named that
way, but obviously they could! *grumble*


>
> 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.
>
> -Sara

---

## #100646 — Dustin Wheeler — 2017-09-15T17:58:57+00:00

URL: https://externals.io/message/100637#100646 · In reply to: #100645

Hi, (accidentally replied to only OP)...

> 
> 
> Hi Marco
> 
> 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.

This argument is predicated on the assumption that my input is unknown. This assumption is not universal to all codebases as implied. 

There are many "foot guns" in every language and framework. I don't think it is necessary (or wise) for the language to assume responsibility for abuse. 

This is the same line of reasoning that could be used to justify removing serialize/deserialize because a developer could "accidentally a tent" or we could build safer options into the feature (as you've already described)

Just my 2c

---

## #100647 — Arvids Godjuks — 2017-09-15T18:27:31+00:00

URL: https://externals.io/message/100637#100647 · In reply to: #100644

2017-09-15 20:52 GMT+03:00 Ryan Pallas <derokorian@gmail.com>:

> 
>
> > […]
> the
> > […]
> this a
> > […]
>
> That makes sense. What about performance difference? On a reasonable sized
> array (not 10k items and not 2) which is faster?
>
> extract($array);
>
> - or -
>
> foreach ($array as $key => $value)
>     $$key = $value;
>
> Honestly, I don't know (ps 2 lines without braces instead of 3!)
>
>
> > […]
>

Hi Ryan,
well, basically, none. Results are from a Q6600 machine and under windows,
so your mileage probably gonna be quite better :)

C:\Users\psihius\Documents\web>php -v
PHP 7.1.5 (cli) (built: May  9 2017 19:48:36) ( NTS MSVC14 (Visual C++
2015) x64 )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
C:\Users\psihius\Documents\web>php -d memory_limit=-1 test.php
6.884626150131226 - Creating arrays
2.035606861114502 - foreach
2.128609180450439 - extract

The code:

define('ITERATIONS', 10000000);

$__time = microtime(true);

$__array = $__array2 = [];
for ($__i = 0; $__i < ITERATIONS; ++$__i) {
    $__array['a'.$__i] = $__i;
    $__array2['b'.$__i] = $__i;
}
echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;

$__time = microtime(true);
foreach ($__array as $__key => $__variable) {
    $$__key = $__variable;
}
echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;

$__time = microtime(true);
foreach ($__array2 as $__key => $__variable) {
    $$__key = $__variable;
}
echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;

-- 
Arvīds Godjuks

+371 26 851 664
arvids.godjuks@gmail.com
Skype: psihius
Telegram: @psihius https://t.me/psihius

---

## #100648 — Ilija Tovilo — 2017-09-15T19:00:24+00:00

URL: https://externals.io/message/100637#100648 · In reply to: #100643

Hi Sara

That is indeed a very good point. Haven't thought of that one.

Regards

> 
> 
> > […]
> 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...
> 
> > […]
> 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.
> 
> -Sara

---

## #100649 — Stephen Reay — 2017-09-15T19:42:41+00:00

URL: https://externals.io/message/100637#100649 · In reply to: #100647

> 
> 
> 2017-09-15 20:52 GMT+03:00 Ryan Pallas <derokorian@gmail.com>:
> 
> > […]
> 
> Hi Ryan,
> well, basically, none. Results are from a Q6600 machine and under windows,
> so your mileage probably gonna be quite better :)
> 
> C:\Users\psihius\Documents\web>php -v
> PHP 7.1.5 (cli) (built: May  9 2017 19:48:36) ( NTS MSVC14 (Visual C++
> 2015) x64 )
> Copyright (c) 1997-2017 The PHP Group
> Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
> C:\Users\psihius\Documents\web>php -d memory_limit=-1 test.php
> 6.884626150131226 - Creating arrays
> 2.035606861114502 - foreach
> 2.128609180450439 - extract
> 
> The code:
> 
> define('ITERATIONS', 10000000);
> 
> $__time = microtime(true);
> 
> $__array = $__array2 = [];
> for ($__i = 0; $__i < ITERATIONS; ++$__i) {
>    $__array['a'.$__i] = $__i;
>    $__array2['b'.$__i] = $__i;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;
> 
> $__time = microtime(true);
> foreach ($__array as $__key => $__variable) {
>    $$__key = $__variable;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;
> 
> $__time = microtime(true);
> foreach ($__array2 as $__key => $__variable) {
>    $$__key = $__variable;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;
> 
> -- 
> Arvīds Godjuks
> 
> +371 26 851 664
> arvids.godjuks@gmail.com
> Skype: psihius
> Telegram: @psihius https://t.me/psihius

Hi all,

If you want to reduce unintended consequences while retaining functionality, the default flag could be changed to EXTR_SKIP. Yes it affects BC but it's also much simpler to fix than if the functionality is gone completely, and in the case of templating it's probably already used that way in most cases anyway.

Cheers


Stephen

---

## #100650 — Stanislav Malyshev — 2017-09-15T20:10:36+00:00

URL: https://externals.io/message/100637#100650 · In reply to: #100637

Hi!

> As a second parameter the `extract` function takes some options to
> make this function less dangerous, like `EXTR_SKIP` that

I'd start with specifying what exactly is "dangerous" in this function.
So far I don't see any specific danger. You can shoot yourself in the
foot, so you can with many other tools in the language.

> I seriously doubt the usefulness of this function, especially looking
> at the potential risks. The fact that overwriting the local variables

Which risks? This function is used by real-life code, and unless you do
something like extract($_GET) in global scope I don't see any problem.
With extract($_GET) we could then also propose to remove all file
functions because fopen($_GET['filename']) or unlink($_GET['filename'])
are also dangerous. But if you use it properly, I don't see what "risks"
are there.

> Any thoughts?

-1 so far, I don't see what problem you are trying to solve.

-- 
Stas Malyshev
smalyshev@gmail.com

---

## #100651 — ilija.tovilo@me.com — 2017-09-15T20:41:27+00:00

URL: https://externals.io/message/100637#100651 · In reply to: #100650

Hi Stas

Dangerous meaning that if given untrusted input someone could mess with the behaviour of your code. There are risks and benefits to every solution. Certainly you’d agree that in some cases the risks outweigh the benefits.

As Sara pointed out, this might not be the case here as there’s no obvious way of mimicking `extract`s behaviour without introducing at least one local variable that could be overwritten.

Thanks for the feedback everybody!

Regards



> Hi!
>
> > […]
>
> I'd start with specifying what exactly is "dangerous" in this function.
> So far I don't see any specific danger. You can shoot yourself in the
> foot, so you can with many other tools in the language.
>
> > […]
>
> Which risks? This function is used by real-life code, and unless you do
> something like extract($_GET) in global scope I don't see any problem.
> With extract($_GET) we could then also propose to remove all file
> functions because fopen($_GET['filename']) or unlink($_GET['filename'])
> are also dangerous. But if you use it properly, I don't see what "risks"
> are there.
>
> > […]
>
> -1 so far, I don't see what problem you are trying to solve.
>
> --
> Stas Malyshev
> smalyshev@gmail.com

---

## #100652 — Stanislav Malyshev — 2017-09-15T21:05:36+00:00

URL: https://externals.io/message/100637#100652 · In reply to: #100651

Hi!

> Dangerous meaning that if given untrusted input someone could mess with
> the behaviour of your code. There are risks and benefits to every

Same as many other functions. Given untrusted input, `unlink()` could
delete files on your hard drive, and `file_put_contents()` could overwrite
your data or send it to unauthorized party. That's not the reason to
remove these functions.

> solution. Certainly you’d agree that in some cases the risks outweigh
> the benefits.

In some cases, yes. In this case, no, as there is no special risks not
existing in many other functions. Any function that has side effects
could do something unexpected when you give it unexpected input. Since
we're not converting PHP to be purely functional language just yet, the
solution is to use functions correctly, not remove them.
-- 
Stas Malyshev
smalyshev@gmail.com

---

## #100653 — Yasuo Ohgaki — 2017-09-15T21:16:17+00:00

URL: https://externals.io/message/100637#100653 · In reply to: #100643

Hi all,



> 
> > […]
> 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...
>
> > […]
> 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

---

## #100654 — ilija.tovilo@me.com — 2017-09-15T21:20:50+00:00

URL: https://externals.io/message/100637#100654 · In reply to: #100652

> no, as there is no special risks

There certainly is. No other function (as far as I’m aware) mutates your local symbol table. This means you need to know exactly what symbols are defined and what kind of data you’ll receive when calling `extract`. So basically this is only safe right at the beginning of your function, and even then it can override your other parameters. Even with trusted data this can hardly be considered safe.

```
function foo(array $data, $bar) {
    extract($data);
    print($bar);
}

$this->foo(['bar' => 'baz'], 'bar’); // “baz"
```

Regards



> Hi!
>
> > […]
>
> Same as many other functions. Given untrusted input, `unlink()` could
> delete files on your hard drive, and `file_put_contents()` could overwrite
> your data or send it to unauthorized party. That's not the reason to
> remove these functions.
>
> > […]
>
> In some cases, yes. In this case, no, as there is no special risks not
> existing in many other functions. Any function that has side effects
> could do something unexpected when you give it unexpected input. Since
> we're not converting PHP to be purely functional language just yet, the
> solution is to use functions correctly, not remove them.
> --
> Stas Malyshev
> smalyshev@gmail.com

---

## #100655 — ilija.tovilo@me.com — 2017-09-15T21:29:28+00:00

URL: https://externals.io/message/100637#100655 · In reply to: #100653

Hi Yasuo

These sound good to me!
There’s still a smaller vulnerability of defining variables beforehand:

```
$data = ['hasAccess' => true];

export($data);

if ($user->isAdmin()) {
    $hasAccess = true;
}

if (isset($hasAccess) && $hasAccess === true) {
    print 'Bingo';
}
```

but code like this should be rather rare.

Regards



> Hi all,
>
> 
>
> > […]
>
> 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

---

## #100656 — Stanislav Malyshev — 2017-09-15T21:59:13+00:00

URL: https://externals.io/message/100637#100656 · In reply to: #100654

Hi!


> > […]
> 
> There certainly is. No other function (as far as I’m aware) mutates your
> local symbol table. This means you need to know exactly what symbols are

Sure, because this is the function to mutate your local symbol table!
Why would we need more of them? It's like saying `unlink()` should be
removed because it deletes files and no other function does it, so it's
super-dangerous!

> defined and what kind of data you’ll receive when calling `extract`. So
> basically this is only safe right at the beginning of your function, and
> even then it can override your other parameters. Even with trusted data
> this can hardly be considered safe.

It does exactly what you tell it to do. Extracts array into local symbol
table. If you don't need that, don't use that function.

-- 
Stas Malyshev
smalyshev@gmail.com

---

## #100661 — lists@rhsoft.net — 2017-09-16T09:35:45+00:00

URL: https://externals.io/message/100637#100661 · In reply to: #100637

Am 15.09.2017 um 19:20 schrieb ilija.tovilo@me.com:
> Hi!
> 
> The `extract` function takes an associative array and puts it into the local symbol table.
> http://php.net/manual/en/function.extract.php
> 
> ```
> $array = [
>      ‘foo’ => ‘foo’,
>      ‘bar’ => ‘bar’,
> ];
> 
> extract($array);
> 
> print $foo; // "foo"
> ```
> 
> As a second parameter the `extract` function takes some options to make this function less dangerous, like `EXTR_SKIP` that prevents an existing local variable of being overwritten. There’s a few more options, go ahead and take a look at the documentation. `EXTR_OVERWRITE` is the default one though. You can also pass a prefix for the variable names as a third argument.
> 
> I seriously doubt the usefulness of this function

then don't use it - who are you proposing remove functionality because 
you don't use it - fine, nobody forces you to do so -  a foreach() 
replacement would miss `EXTR_OVERWRITE` - this whole thread is a "have 
solution, looking for a problem"

---

## #100663 — lists@rhsoft.net — 2017-09-16T09:37:45+00:00

URL: https://externals.io/message/100637#100663 · In reply to: #100642

Am 15.09.2017 um 19:38 schrieb ilija.tovilo@me.com:
> I can see your argument. The reasoning behind it is that a function in the standard library should not encourage unsafe code. Admittedly, since this function is rarely used

who do you think you are that you can qualify this?
breaking news: there is code outside the s*itlab/s*ithub world!

---

## #100665 — lists@rhsoft.net — 2017-09-16T09:42:12+00:00

URL: https://externals.io/message/100637#100665 · In reply to: #100647

Am 15.09.2017 um 20:27 schrieb Arvids Godjuks:
> well, basically, none. Results are from a Q6600 machine and under windows,
> so your mileage probably gonna be quite better :)

well, and now implement the `EXTR_SKIP` in PHP code - that becomes a ugly 
piece of code and that only because someone likes to remove a already 
existing function nobody is forced to use?

sadly i am not allowed to say what i think in public...

> C:\Users\psihius\Documents\web>php -v
> PHP 7.1.5 (cli) (built: May  9 2017 19:48:36) ( NTS MSVC14 (Visual C++
> 2015) x64 )
> Copyright (c) 1997-2017 The PHP Group
> Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
> C:\Users\psihius\Documents\web>php -d memory_limit=-1 test.php
> 6.884626150131226 - Creating arrays
> 2.035606861114502 - foreach
> 2.128609180450439 - extract
> 
> The code:
> 
> define('ITERATIONS', 10000000);
> 
> $__time = microtime(true);
> 
> $__array = $__array2 = [];
> for ($__i = 0; $__i < ITERATIONS; ++$__i) {
>      $__array['a'.$__i] = $__i;
>      $__array2['b'.$__i] = $__i;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;
> 
> $__time = microtime(true);
> foreach ($__array as $__key => $__variable) {
>      $$__key = $__variable;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;
> 
> $__time = microtime(true);
> foreach ($__array2 as $__key => $__variable) {
>      $$__key = $__variable;
> }
> echo number_format(microtime(true) - $__time, 15, '.', ''), PHP_EOL;

---

## #100666 — ilija.tovilo@me.com — 2017-09-16T09:44:22+00:00

URL: https://externals.io/message/100637#100666 · In reply to: #100665

Dude, calm down.

This was a request for feedback. As other people have pointed out before it may not be the best idea to remove it since there’s no good way to write the equivalent code in PHP. No reason to be rude.


>
>
> Am 15.09.2017 um 20:27 schrieb Arvids Godjuks:
> > […]
>
> well, and now implement the `EXTR_SKIP` in PHP code - that becomes a ugly
> piece of code and that only because someone likes to remove a already
> existing function nobody is forced to use?
>
> sadly i am not allowed to say what i think in public...
>
> > […]

---

## #100667 — lists@rhsoft.net — 2017-09-16T09:49:15+00:00

URL: https://externals.io/message/100637#100667 · In reply to: #100666

Am 16.09.2017 um 11:44 schrieb ilija.tovilo@me.com:
> Dude, calm down.

dude when people have no other hobbies than think about removing 
functionality for *no gain*

> This was a request for feedback

then you should withstand the feedback!
why don't you spend your time for *add* features instead cripple things?

> As other people have pointed out before 
> it may not be the best idea to remove it since there’s no good way to 
> write the equivalent code in PHP. No reason to be rude.
> 
> 
> > […]

---

Next page: https://externals.io/message/100637.md?page=2