Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
When using PHP 5.3, the above error is generated when field_file_save_file() is called.
The attached patch fixes this (patch is against 6.x-3.2, but it will probably work on latest dev too).
Comment | File | Size | Author |
---|---|---|---|
#15 | 755558-morecomments.patch | 833 bytes | roderik |
#13 | 755558.NOT_.patch | 1.22 KB | roderik |
#13 | 755558.patch | 754 bytes | roderik |
filefield-php-5.3.patch | 591 bytes | yhager |
Comments
Comment #1
quicksketchThis seems like a very strange way to fix this problem, since it depends on a calltime pass-by-reference. I think we could fix it without causing warnings in other PHP installations just by initializing $file as
$file = array()
. Making something by reference with an & sign accomplishes the same thing indirectly, but it would cause a warning on most PHP installations.Comment #2
yhager CreditAttribution: yhager commentedI am not sure what you suggest here.
filefield_validate_associate_field()
modifies the$file
object. For that to work it must get a reference to the object, as the function signature shows.However, in PHP 5.3, calling the validator using
call_user_func_array()
will only pass arguments by value, regardless of the function signature. The patch above does just that - sends a reference of the object.Can you be more specific on how you suggest to solve this instead?
Comment #3
quicksketchSorry I was making the assumption that $file was undefined; I didn't actually look at the code with the patch applied.
What I'm worried about here is that putting an ampersand in front of a variable when you're calling a function results in, "Warning: Call time pass-by-reference depreciated" error, at least in every version of PHP up to 5.3. I haven't used 5.3 so I'm not sure if it occurs there also. Getting rid of a warning in 5.3 but causing one in 5.2 isn't going to be acceptable. I find it highly unusual that 5.3 would require you to explicitly cast the reference when all objects are already passed by reference in PHP 5 and higher.
Comment #4
yhager CreditAttribution: yhager commentedI agree that a solution should not generate errors in 5.2.
From call_user_func_array documentation:
I have made a short test program:
the first call is what used today by filefield calling its validators, the second is how it looks after the patch is applied. I ran it on PHP 5.2, and PHP 5.3:
No error on PHP 5.2, and it works correctly.
Seeing the behavior of the first version in PHP 5.2 makes me think this is a real bug in all PHP versions, albeit does not generate an error message with 5.2.
Comment #5
quicksketchYou should also check your php.ini file and see what this option is set to:
The default, afaik, is "Off", meaning that a call like
function_name(&$variable)
will throw a warning.Comment #6
yhager CreditAttribution: yhager commentedTrue, default is off, but it seem call_user_func_array is not affected.
I have changed the value to off on PHP 5.2 (it does not exist in PHP 5.3) with the same results as above (no errors).
Comment #7
quicksketchImmediately upon visiting node/add/story (with a FileField enabled on this type) I receive:
This seems to indicate exactly what I had described: This patch causes an error in PHP 5.2 and lower when allow_call_time_pass_reference is off (which is the default).
Comment #8
Reg CreditAttribution: Reg commentedI was looking at the patch and it looks like a similar problem as here: http://drupal.org/node/734556 .
You should probably effect a similar solution as theirs - the line:
$params[0] = &$image;
because a line like:
array_unshift($args, &$file);
as used in the above patch will throw a depreciated warning in PHP 5.3 because it uses call-time pass by reference.
Comment #9
quicksketchThanks for that suggestion Reg. If using that approach, we'd first need to unshift something onto the array like an empty string, since there is already something in position 0. Then we can use your approach and it should work in all situations. Still it seems like there should be a reliable and easy way to accomplish a simple task like this.
Comment #10
justintime CreditAttribution: justintime commentedsubscribing
Comment #11
imclean CreditAttribution: imclean commentedHow's this coming along? I'm seeing the problem with Filefield 6.x-3.7 and Filefield Sources: #843152: warning: Parameter 1 to filefield_validate_associate_field() expected to be a reference
Comment #12
quicksketchPatches appreciated, I'm pretty busy with a lot of things.
Comment #13
roderik@ #9: I don't think there is a reliable and easy way to accomplish this. (As far as I understand, this is one of the reasons why the drupal_alter() call signature is somewhat odd. There is just no 5.2-and-5.3-generic way to call a function with a variable number of arguments, if some of these arguments are references.)
Just FYI: in other cases I have worked around call_user_func_array(), like in the first patch included here.
But that seems inconvenient & overkill here, if we know that:
- the first argument ($file) must always be passed as a reference
- no other arguments are ever passed as a reference
...because then we can keep using call_func_array().
The second attachment is the original patch, incorporating solution #8 to problem #7. As far as I know, there's no better solution than this.
Comment #14
quicksketchThanks roderik. So to clarify, "755558.patch" is the suggested patch correct? It looks good to me, though the comments aren't very clear. I think explicitly stating that array_unshift() in PHP 5.3 breaks referenced variables would be good clarity.
Comment #15
roderikYes, "755558.patch" is the suggested one. Credit to yhager - and yourself for testing.
I can never get comments right (last time I had a php5.3 patch I was told my comment was too long :p) and it depends on personal taste too. I think we should then state that any function except call_user_func* breaks referenced variables. How about this one?
Comment #16
quicksketchThanks committed. This will be included in the 3.8 version.