php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #66107 func_get_args change breaks by-reference in ReflectionClass::newInstanceArgs
Submitted: 2013-11-17 15:24 UTC Modified: 2014-02-11 12:43 UTC
Votes:21
Avg. Score:4.8 ± 0.6
Reproduced:18 of 19 (94.7%)
Same Version:18 (100.0%)
Same OS:2 (11.1%)
From: sjon at hortensius dot net Assigned: dmitry (profile)
Status: Closed Package: Arrays related
PHP Version: 5.5.6 OS: archlinux
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: sjon at hortensius dot net
New email:
PHP Version: OS:

 

 [2013-11-17 15:24 UTC] sjon at hortensius dot net 
Description:
------------
The changelog states:

> Improved performance of array_merge() and func_get_args() by eliminating useless copying.

I think I found a bug related to this change. If it's not fixed it should at least be documented.

Test script:
---------------
From http://3v4l.org/U40T0

<?php
class f
{
    function f(&$e){}
}

function getNew()
{
    $a = func_get_args();
    $c = array_shift($a);

    $r = new ReflectionClass($c);
    return $r->newInstanceArgs($a);
}

$e = new stdClass;

var_dump(getNew('f', $e));

Expected result:
----------------
object(f)#3 (0) {
}

Actual result:
--------------
Warning: Parameter 1 to f::f() expected to be a reference, value given in /in//in/U40T0 on line 14

Warning: ReflectionClass::newInstanceArgs(): Invocation of f's constructor failed in /in//in/U40T0 on line 14
NULL

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2013-11-18 02:25 UTC] laruence@php.net
-Assigned To: +Assigned To: dmitry
 [2013-11-18 09:32 UTC] sjon at hortensius dot net 
Since $e is an object it is already passed by reference; so the error is rather strange. The code seems to check for an explicit &$e by-reference instead of an implicit variable=object=reference
 [2013-11-25 12:40 UTC] dmitry@php.net 
Actually, this code worked not on purpose but because of luck.
Previously, func_get_args() copied each argument making refcount == 1 and it allowed to pass it by reference.

The following code demonstrates the real reason of the bug. The first call to call_user_func_array() works but the second doesn't.

<?php
function foo(&$e) {
	var_dump($e);
}
call_user_func_array("foo", array(2));
$a = array(2);
call_user_func_array("foo", $a);
?>

The problem may be related to zend_fcall_info.no_separation handling.
 [2013-11-29 18:41 UTC] neufeind@php.net 
This is a regression introduced from 5.5.5 to 5.5.6. Reading the news for 5.5.7RC1 now, it looks like this won't be fixed anytime soon? :-(

Bug at TYPO3 for this. All current TYPO3-releases are affected and only work until 5.5.5.
http://forge.typo3.org/issues/53682
 [2013-12-07 12:05 UTC] bugs dot php dot net at philippgampe dot info 
This is a really nasty bug, because you cannot work around it.
Changing the interface breaks all 3rd-party code that makes use of your code.
 [2013-12-10 12:19 UTC] dmitry@php.net 
I've reverted changes in PHP-5.5 (not in PHP-5.6 and above).
 [2013-12-14 04:53 UTC] Christian dot Stadler at der-bloggsberg dot de 
Seems, like this fix didn't make it into PHP 5.5.7 in time. Does that mean, that this will be fixed in PHP 5.5.8?
 [2013-12-16 08:36 UTC] dmitry@php.net 
It's pushed into PHP-5.5 branch so it must be fixed in the next 5.5 release.
 [2014-02-11 10:16 UTC] sjon at hortensius dot net 
This bug is indeed fixed in 5.5.8; it is however also broken in 5.6 (a1). Should I report a new bug or leave this one open?
 [2014-02-11 12:12 UTC] dmitry@php.net 
I think it's not going to be "fixed" in 5.6.

It was fixed in 5.5 to not introduce a break in minor release.
But it's OK to break unintended behaviour in major release.
As I wrote, it was never designed or expected to work in that way.
 [2014-02-11 12:14 UTC] sjon at hortensius dot net 
Okay, that seems fine. In that case you can close this issue as fixed.
 [2014-02-11 12:43 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Wed Apr 24 12:01:29 2024 UTC