Parameter 1 to wp_handle_upload_error() expected to be a reference, value given

[jbrinley]

When calling media_handle_sideload() with a failed download, the file information is passed (in _wp_handle_upload() to the upload error handler, which defaults to wp_handle_upload_error().

In, PHP7 (and maybe earlier), this throws a warning: "PHP Warning: Parameter 1 to wp_handle_upload_error() expected to be a reference, value given in /srv/www/public/wp/wp-admin/includes/file.php".

Per the notes at ​http://php.net/call_user_func, "the parameters for call_user_func() are not passed by reference", but it provides a workaround using call_user_func_array().

All of the calls to the error handler should be changed from:

call_user_func( $upload_error_handler, $file, $error_msg )

To:

call_user_func_array( $upload_error_handler, array( &$file, $error_msg ) )

[jbrinley]

Replace call_user_func with call_user_func_array

[jbrinley]

Patch attached. https://core.trac.wordpress.org/attachment/ticket/37570/37570.diff

[boonebgorges]

Related: #37295, ​https://buddypress.trac.wordpress.org/ticket/7204

[ocean90]

This is interesting. It looks like the warning was missing in PHP 7.0.0 - 7.0.8 and also in PHP 7.1 Alpha 1 + 2, see ​https://3v4l.org/fESuN.

The same function uses also a variable function (https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/file.php?rev=38151&marks=273#L245) which doesn't produce the warning.

[ocean90]

We should consider to fix this in 4.6.

The issue is not only the warning, it also prevents the execution of the function, which means that the upload gets treated as successful.

Let's say the $test_uploaded_file condition fails:

Expected:

{
  "success": false,
  "data": {
    "message": "Specified file failed upload test.",
    "filename": "test.png"
  }
}

Actual:

{
  "success": true,
  "data": {
    "id": 32,
    "title": "test",
    "filename": "",
    "url": "http:\/\/src.wordpress-develop.dev\/test\/",
    "link": "http:\/\/src.wordpress-develop.dev\/test\/",
    "alt": "",
    "author": "1",
    "description": "",
    "caption": "",
    "name": "test",
    "status": "inherit",
    "uploadedTo": 0,
    "date": 1470746608000,
    "modified": 1470746608000,
    "menuOrder": 0,
    "mime": "",
    "type": "",
    "subtype": "",
    "icon": "http:\/\/src.wordpress-develop.dev\/wp-includes\/images\/media\/default.png",
    "dateFormatted": "9. August 2016",
    "nonces": {
      "update": "b8499d3bd9",
      "delete": "19e4752d90",
      "edit": "975849a990"
    },
    "editLink": "http:\/\/src.wordpress-develop.dev\/wp-admin\/post.php?post=32&action=edit",
    "meta": false,
    "authorName": "admin",
    "compat": {
      "item": "",
      "meta": ""
    }
  }
}

@dd32, @pento, @jorbin: What do you think?

[ocean90]

Replying to ocean90:

This is interesting. It looks like the warning was missing in PHP 7.0.0 - 7.0.8 and also in PHP 7.1 Alpha 1 + 2, see ​https://3v4l.org/fESuN.

That was the wrong test script. The correct one is ​https://3v4l.org/p9E7N. It's the nested function and I'm assuming that it's because of ​https://bugs.php.net/bug.php?id=72508 which was fixed in 7.0.9, see ​https://github.com/php/php-src/blob/PHP-7.0.9/NEWS#L6.

[jorbin]

I also think this is worth fixing in 4.6. We should always run notice free.

[jorbin]

It's also worth noting that this will fail in 7.1 according to the ​upgrading file:

  . call_user_func() will now consistently fail to perform calls to functions
    that accept reference arguments. Previously this sometimes worked if
    call_user_func() was used outside a namespace.

[ocean90]

In 38235:

Media: In _wp_handle_upload() use call_user_func_array() to call the upload error handler.

The default error handler wp_handle_upload_error() expects a reference for the first parameter but call_user_func() doesn't pass parameters by reference. The current code didn't produce any issues until now. PHP 7.0.9 (and PHP 7.1) is now stricter and prevents calling the error handler with a warning:

PHP Warning: Parameter 1 to wp_handle_upload_error() expected to be a reference, value given.

To restore the error handler _wp_handle_upload() now uses call_user_func_array().

Props jbrinley.
Props jorbin for review.
Fixes #37570.

[ocean90]

In 38236:

Media: In _wp_handle_upload() use call_user_func_array() to call the upload error handler.

The default error handler wp_handle_upload_error() expects a reference for the first parameter but call_user_func() doesn't pass parameters by reference. The current code didn't produce any issues until now. PHP 7.0.9 (and PHP 7.1) is now stricter and prevents calling the error handler with a warning:

PHP Warning: Parameter 1 to wp_handle_upload_error() expected to be a reference, value given.

To restore the error handler _wp_handle_upload() now uses call_user_func_array().

Merge of [38235] to the 4.6 branch.

Props jbrinley.
Props jorbin for review.
See #37570.