Bug #29992 [Com]: foreach by reference corrupts the array

From: Date: Wed, 04 Dec 2013 21:50:34 +0000
Subject: Bug #29992 [Com]: foreach by reference corrupts the array
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-183122@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=29992&edit=1

 ID:                 29992
 Comment by:         gray dot bowman at gmail dot com
 Reported by:        fletch at pobox dot com
 Summary:            foreach by reference corrupts the array
 Status:             Not a bug
 Type:               Bug
 Package:            Scripting Engine problem
 Operating System:   linux
 PHP Version:        5.0.1
 Block user comment: N
 Private report:     N

 New Comment:

Just wanted to chime in that 9 years on, this is still totally unexpected behavior.  Today, two
professional developers with years experience spent a couple hours trying to figure out why an array
that was demonstrably intact had its last element corrupted for apparently no reason once entering a
foreach.


Previous Comments:
------------------------------------------------------------------------
[2013-05-21 06:38:43] email at stevemann dot net

Agreed this is not a bug, it's expected behaviour. But it's dangerous as it can slip by
without being noticed. It almost certainly means there are 
thousands of sites which are exhibiting wrong behaviour because of this and no-one realises. Surely
the concept of scoping the 'as' variable to the 
foreach enclosure only can't be considered bad form. It would make so much more sense to
'opt-in' to retrieving the variable outside of the enclosure 
(by assigning to another persistent variable within the enclosure) rather than the current
'opt-out' system (using unset()) which, unless you happen 
to have read the warning is HIGHLY DANGEROUS.

------------------------------------------------------------------------
[2013-05-20 15:57:18] paul dot dillinger at gmail dot com

OK, I went over this some more.

<pre>
<?php
// Fresh array
$clean = array(1,2,3,4);

foreach($clean as &$item){
  // Nothing is modified in the array, but $item now exists
}

/*##############################################################################
 * $item persists outside of foreach and is now $clean[3] 
 * See the warning on http://php.net/manual/en/control-structures.foreach.php
 * print_r($item); // would return 4 you you uncommented this.
 * unset($item); // This would remove the pointer to $clean[3].  Expected.
 *############################################################################*/

echo "A:\n";
/*A*/       print_r($clean); // $clean is still unmodified

echo "B:\n";
            foreach($clean as $item){  
/*##############################################################################
 * Using AS $item SETS $item TO the current $item value (a.k.a. $clean[0], etc.)
 * Essentially foreach($clean as $item) is short hand for something like:
 * $x=0;while($x < count($clean)){$item=$clean[$x]; ### your code ###  $x++;}
 * The problem I had was that I did not expect foreach to be able to set on call
 *############################################################################*/

/*B*/           print_r($clean);
            }
?>
</pre>

So creating the variable is documented, and it isn't a bug.  
The ability to set the value could be made clearer though.

------------------------------------------------------------------------
[2013-05-20 09:51:33] richard at ejem dot cz

This IS a bug and SHOULD be finally fixed. It is weird, hard to debug and unexpected behavior which
took me many hours of life finding the problem. Almost every modern programming language has a
variable context, which guarrantees the foreach variable is not visible after exiting the loop.

Anyone is using it intentionally for "weird reason"? come on guys, almost every bug can be
used for some hacking or weird reasons, will it stop you from fixing other bugs? no!

Sorry for "mee-to" like post, but I do it for the good of next PHP programmer generations
who will also lose hours of their lives discovering this nonsense. Please fix it.

------------------------------------------------------------------------
[2012-10-25 17:01:03] paul dot dillinger at gmail dot com

I understand that my explanation above isn't 100% accurate (unset really doesn't 
have anything to do with it), but that doesn't change that the expected behavior 
is not working.

Rasmus said:
	"Ok, simple question then. Do you expect this to output 3?

	foreach(array(1,2,3) as $b) { }
	echo $b;

	If you do, then you don't want us to fix this "bug" 
	because fixing it would mean $b is not 3 here."

I say in the example above, would you expect a print_r of the same array to 
return 1,2,2?  My issue was that the content of the entire array.

Honestly I've been programming since I was eight years old.  That was 1985.  If 
this was confusing the hell out of me then something's wrong.

Here's an even simpler example:

<pre>
<?php
$clean = array(1,2,3,4);
foreach($clean as &$item){
  // Nothing
}
echo "A:\n";
/*A*/       print_r($clean);
echo "B:\n";
            foreach($clean as $item){
/*B*/           print_r($clean);
            }

$clean = array(1,2,3,4);
foreach($clean as &$item){
  // Nothing
}
echo "C:\n";
/*C*/       print_r($clean);
echo "D:\n";
            foreach($clean as $not_item){
/*C*/           print_r($clean);
            }
?>
</pre>

A and B output the same array differently with no modification (not expected).
C and D are the same (expected).  The only change was not re-using the name 
$item.

How can we make it so that using &$item in one foreach and then using the same 
variable name ($item) in a different foreach does not change the original array?

------------------------------------------------------------------------
[2012-10-25 15:49:27] paul dot dillinger at gmail dot com

I still say this is a bug, so here's all the code you need to re-produce it:

<pre>
<?php
$packages = array();
$library_names = array();
$ext_js =
    array(
        array(
            'name' => 'myname1',
            'attrib' => 'something1',
            'package' => true,
            'library_name' => 'package1'
            ),
        array(
            'name' => 'myname2',
            'attrib' => 'something2',
            'package' => true,
            'library_name' => 'package1'
            ),
        array(
            'name' => 'myname3',
            'attrib' => 'something3',
            'package' => false
            ),
        array(
            'name' => 'myname4',
            'attrib' => 'something4',
            'package' => false
            )
        );

foreach($ext_js as &$item){
	if(!empty($item['package'])){
                $packages[] = $item; // Not using &, so should be a copy, not a 
reference corrent?
                $library_names[] = $item['library_name'];
                unset($item);
	}
}
if(!empty($packages)){
/*A*/       print_r($ext_js);
            foreach($packages as $item){
/*B*/           print_r($ext_js);
            }
}
?>
</pre>

Look at the output on the last item.  Instead of unset removing the item from 
the array $ext_js (which is what I thought it would do), it corrupts the array.  
The array is fine though unless you go in to another foreach using another 
$item.

Changing the variable name on the second foreach to something OTHER than $item 
(I used $itemm) fixes it.

Bug.

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=29992


-- 
Edit this bug report at https://bugs.php.net/bug.php?id=29992&edit=1


Thread (43 messages)

« previous php.bugs (#183122) next »