See includes/file.inc, line 191

Original function:

function file_check_location($source, $directory = '') {
  $check = realpath($source);
  if ($check) {
    $source = $check;
  }
  else {
    // This file does not yet exist
    $source = realpath(dirname($source)) .'/'. basename($source);
  }
  $directory = realpath($directory);
  if ($directory && strpos($source, $directory) !== 0) { // <-- this is wrong
    return 0;
  }
  return $source;
}

According to php manual [ http://php.net/strpos ] you must type compare with FALSE instead of 0 as 0 can be a valid position.

So that line must be changed to:

if ($directory && strpos($source, $directory) !== FALSE) {

Comments

prine-1’s picture

prine-1 commented

Based on my experiences, this may be a critical bug for running Drupal on some servers.

I had been attempting to build a site using Drupal, but I was unable to attach files to nodes using the Upload module. I would get the error:

* warning: move_uploaded_file(): Unable to move '/tmp/phpZp3hUa' to '' in example.com/includes/file.inc on line 572.
* File upload error. Could not move uploaded file.

The destination was being set as blank (with the empty single quotes: ''), and so it could not create the proper file.

In trying to debug this, I traced the problem through the file.inc file: I tracked it through file_save_upload, to file_create_path, and finally to file_check_location. Then, I came across this bug report, which matched up perfectly to my issue.

I changed that line from '0' to 'FALSE'--and now the uploads work perfectly!

In addition, I had been getting the same error when I tried to change the color scheme in Garland, since that also requires an upload of the 'color' files. And now the color scheme change works as well!

Also, I hadn't been able to get SUPHP to work with Drupal--and now it does work.

All those magical improvements just from one little change! So, it seems this could solve similar problems for other people as well.

Are there other places in Drupal core where it compares to '0' when it should be comparing to 'FALSE'?

In any case, thanks very much to CoolCow for pointing this out!

markusrado’s picture

markusrado commented

I don't think this is a bug, I think the intension is to compare with the start of the string. The bug is probably in the module using this function.

brianV’s picture

brianV commented
Status: Active » Closed (works as designed)

markusrado is correct. That is how it is supposed to operate.