If I create a node with a Filefield field on it, save the node then delete the node, I will receive the following error message:

warning: Parameter 1 to upload_replace_file_delete() expected to be a reference, value given in /someplace/includes/module.inc on line 497.

Tracked this down to upload_replace_file_update() in upload_replace.module. At the beginning of the function we have:

  if (!$new_file->fid) {
    //Nothing to do if no fileid
    return;
  }

Changed it to:

  if (empty($new_file->fid)) {
    //Nothing to do if no fileid
    return;
  }

And the error is gone.

CommentFileSizeAuthor
#1 upload-replace-1780258.txt715 bytesdpearcefl

Comments

dpearcefl’s picture

dpearcefl commented
Title: Parameter error on node delete » Parameter error on node update and node delete
Status: Active » Needs review
StatusFileSize
new upload-replace-1780258.txt715 bytes

Found another error that needs to be fixed. Here is the fixed function declaration.

function upload_replace_file_delete($file) {
  $file->filepath = db_result(db_query("SELECT filepath FROM {files} WHERE fid = %d", $file->fid));
}

The function parameter should not be a reference.

Patch for both issues is attached.

cybermache’s picture

cybermache commented

So far your changes seem to be working for me.
Thanks

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

We've got the same error. Is there an active maintainer who can create a new release, if we fix this?

Anonymous’s picture

Anonymous (not verified) commented

Is there any progress?

developerchris’s picture

developerchris commented
Issue summary: View changes

This is still an issue as of the date of this post.

The problem is the module incorrectly uses the hook_file_update() hook and assumes quite incorrectly that the parameter is a reference

The reason it uses a reference is because it alters the $file object. 

$new_file->uri = $desired_destination;

I assume this is so future function calls have the correct filename. Unfortunately that does not work and therefore can adversely effect other modules.This means that there may be unintended consequences of using this module.

I cannot see in drupals hooks how you can alter the destination filename during the save process.

This line in upload_replace.module 

if (!strpos($new_file->uri, $new_file->uri))

is equivalent to 

if(true)

so is entirely redundant and does not work as the author intended

For the reasons above I strongly suggest not using this module

Dru18’s picture

Dru18 commented

It seems the issue has been resolved with 7.x-1.0-beta1+2-dev.

Regarding the above commenter's advice not to use the module because of "unintended consequences." I wonder if there is a clear evidence of it.
When file updating, file_save() function invokes these hooks.

// Inform modules that the file has been updated.
module_invoke_all('file_update', $file);
module_invoke_all('entity_update', $file, 'file');

entity_update simply updates (updated) date. I don't see anything that may create consequences. Am I missing something here?

developerchris’s picture

developerchris commented

The unintended consequences is that other modules hooks do not get the new filename instead they get the unchanged filename this may result in other hooks operating on the wrong file.

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
commented
Status: Needs review » Closed (won't fix)

Drupal 6 has reached end of support. Close this issue.