# [PATCH] fix extract w.r.t GLOBALS again

- Thread: https://externals.io/message/50393
- Original archive: https://news-web.php.net/php.internals/50393
- Started by Joe Orton on 2010-11-19T15:43:01+00:00
- Messages: 4
- 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

---

## #50393 — Joe Orton — 2010-11-19T15:43:01+00:00

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

The check to prevent `extract()` overwriting $GLOBALS got broken at some 
point - here's a fix:

Index: ext/standard/array.c
===================================================================
--- ext/standard/array.c	(revision 305556)
+++ ext/standard/array.c	(working copy)
@@ -1389,10 +1389,10 @@
 
 			case EXTR_OVERWRITE:
 				/* GLOBALS protection */
-				if (var_exists && var_name_len == sizeof("GLOBALS") && !strcmp(var_name, "GLOBALS")) {
+				if (var_exists && var_name_len == sizeof("GLOBALS")-1 && !strcmp(var_name, "GLOBALS")) {
 					break;
 				}
-				if (var_exists && var_name_len == sizeof("this")  && !strcmp(var_name, "this") && EG(scope) && EG(scope)->name_length != 0) {
+				if (var_exists && var_name_len == sizeof("this")-1 && !strcmp(var_name, "this") && EG(scope) && EG(scope)->name_length != 0) {
 					break;
 				}
 				ZVAL_STRINGL(&final_name, var_name, var_name_len, 1);
Index: ext/standard/tests/array/extract_safety.phpt
===================================================================
--- ext/standard/tests/array/extract_safety.phpt	(revision 0)
+++ ext/standard/tests/array/extract_safety.phpt	(revision 0)
@@ -0,0 +1,24 @@
+--TEST--
+Test `extract()` for overwrite of GLOBALS
+--FILE--
+<?php
+$str = "John";
+debug_zval_dump($GLOBALS["str"]);
+
+/* Extracting Global Variables */
+$splat = array("foo" => "bar");
+var_dump(extract(array("GLOBALS" => $splat, EXTR_OVERWRITE)));
+
+unset ($splat);
+
+debug_zval_dump($GLOBALS["str"]);
+
+echo "\nDone";
+?>
+
+--EXPECTF--
+string(4) "John" refcount(2)
+int(0)
+string(4) "John" refcount(2)
+
+Done

---

## #50395 — Chris Stockton — 2010-11-19T21:25:58+00:00

URL: https://externals.io/message/50393#50395 · In reply to: #50393

> The check to prevent `extract()` overwriting $GLOBALS got broken at some
> point - here's a fix:

I remember http://bugs.php.net/47409 for this some time ago and seeing
it marked applied. After taking a peak it looks like the patch in the
bug report was ignored. One more reason why I stopped contributing I
guess.

-Chris

---

## #50397 — Felipe Pena — 2010-11-19T22:07:47+00:00

URL: https://externals.io/message/50393#50397 · In reply to: #50393

Hi,

2010/11/19 Joe Orton <jorton@redhat.com>

> The check to prevent `extract()` overwriting $GLOBALS got broken at some
> point - here's a fix:
>
> Index: ext/standard/array.c
> ===================================================================
> --- ext/standard/array.c        (revision 305556)
> +++ ext/standard/array.c        (working copy)
> @@ -1389,10 +1389,10 @@
>
>                        case EXTR_OVERWRITE:
>                                /* GLOBALS protection */
> -                               if (var_exists && var_name_len ==
> sizeof("GLOBALS") && !strcmp(var_name, "GLOBALS")) {
> +                               if (var_exists && var_name_len ==
> sizeof("GLOBALS")-1 && !strcmp(var_name, "GLOBALS")) {
>                                        break;
>                                }
> -                               if (var_exists && var_name_len ==
> sizeof("this")  && !strcmp(var_name, "this") && EG(scope) &&
> EG(scope)->name_length != 0) {
> +                               if (var_exists && var_name_len ==
> sizeof("this")-1 && !strcmp(var_name, "this") && EG(scope) &&
> EG(scope)->name_length != 0) {
>                                        break;
>                                }
>                                ZVAL_STRINGL(&final_name, var_name,
> var_name_len, 1);
> Index: ext/standard/tests/array/extract_safety.phpt
> ===================================================================
> --- ext/standard/tests/array/extract_safety.phpt        (revision 0)
> +++ ext/standard/tests/array/extract_safety.phpt        (revision 0)
> @@ -0,0 +1,24 @@
> +--TEST--
> +Test `extract()` for overwrite of GLOBALS
> +--FILE--
> +<?php
> +$str = "John";
> +debug_zval_dump($GLOBALS["str"]);
> +
> +/* Extracting Global Variables */
> +$splat = array("foo" => "bar");
> +var_dump(extract(array("GLOBALS" => $splat, EXTR_OVERWRITE)));
> +
> +unset ($splat);
> +
> +debug_zval_dump($GLOBALS["str"]);
> +
> +echo "\nDone";
> +?>
> +
> +--EXPECTF--
> +string(4) "John" refcount(2)
> +int(0)
> +string(4) "John" refcount(2)
> +
> +Done
>
> --
> 
> 
>
>
I committed your fix in the 5.2, 5.3 and trunk.

Thanks for the patch!

-- 
Regards,
Felipe Pena

---

## #50398 — Felipe Pena — 2010-11-20T00:14:43+00:00

URL: https://externals.io/message/50393#50398 · In reply to: #50395

Hi Chris,

2010/11/19 Chris Stockton <chrisstocktonaz@gmail.com>

> 
> > […]
>
> I remember http://bugs.php.net/47409 for this some time ago and seeing
> it marked applied. After taking a peak it looks like the patch in the
> bug report was ignored. One more reason why I stopped contributing I
> guess.
>
>
The fix committed to the related bug was just like your patch, except that
you did the right `sizeof()` - 1.

Then as you see, we need contributions... making patches... testing fixes...
reviewing fixes...

As it's said in the bugsweb... "Help us make PHP better." :)

-- 
Regards,
Felipe Pena