Flexinode_file_download incorrectly returns false, breaking other file system-based modules

I tried to reproduce this and wasn't able to. I'm on Win32 XAMPP (php5/mysql5).

gallery field is *intentionally* not released for 4.7.

I had the same sympton occur when I upgraded a site from 4.6 to 4.7. The images uploaded with image.module stopped appearing. After investigating, I discovered the following:

1. the image files still existed and were not corrupted. I could view the files fine by opening them manually in an image viewer.
2. When I right-clicked on the broken image markers in IE, selected "properties" then copied the path into the browser address, what I got back was not the image, but a mass of symbols which appeared to be the contents of the image file
3. When I checked the header of the page with the symbols, I saw that the mime type was text and not an image type as it should have been
4. Using a debugger, I found that while drupal was constructing the headers for the image, the flexinode module was including a null string just ahead of the proper image mime type that was being supplied by the image module
5. So I turned off flexinode and, sure enough, when I navigated to the image file again, I got the picture back instead of the mass of symbols

It looks like a modification to flexinode would be the easiest way to fix this bug, but I'm no expert on flexinode. Can anyone else duplicate this bug?

subscribing.

The flexinode_file_download function was returning a value of false when called for files not uploaded by flexinode. From what I can tell, looking at other implementations of hook_file_download, it should return nothing at all if the file is unrelated to flexinode. I've modified the 4.7.3 flexinode_file_download function to do just that, and it has solved the problem with images uploaded by the image module not displaying correctly. The modified code follows:

function flexinode_file_download($file) {
  if ($file) {
	  $result = db_fetch_object(db_query("SELECT f.* FROM {flexinode_data} f WHERE f.textual_data = '%s'", $file));
	  if ($result) {
		  $filedb = unserialize($result->serialized_data);
		  if ($filedb->type) {
		    return array('Content-type: '. $filedb->type, 'Content-Disposition: attachment; filename="'. $file .'"');
		  }
		  if ($path = file_create_path($file)) {
		    list($width, $height, $type, $attr) = getimagesize($path);
		    $types = array(
		      IMAGETYPE_GIF => 'image/gif',
		      IMAGETYPE_JPEG => 'image/jpeg',
		      IMAGETYPE_PNG => 'image/png',
		      IMAGETYPE_SWF => 'application/x-shockwave-flash',
		      IMAGETYPE_PSD => 'image/psd',
		      IMAGETYPE_BMP => 'image/bmp',
		      IMAGETYPE_TIFF_II => 'image/tiff',
		      IMAGETYPE_TIFF_MM  => 'image/tiff',
		      IMAGETYPE_JPC => 'application/octet-stream',
		      IMAGETYPE_JP2 => 'image/jp2',
		      IMAGETYPE_JPX => 'application/octet-stream',
		      IMAGETYPE_JB2 => 'application/octet-stream',
		      IMAGETYPE_SWC => 'application/x-shockwave-flash',
		      IMAGETYPE_IFF => 'image/iff',
		      IMAGETYPE_WBMP => 'image/vnd.wap.wbmp',
		      IMAGETYPE_XBM => 'image/xbm'
		    );
		    if (isset($types[$type])) {
		      return array('Content-type: '. $types[$type], 'Content-Disposition: attachment; filename="'. $file .'"');
		    }
		    else {
		      $type = (function_exists('mime_content_type') ? mime_content_type($path) : 'application/x-download');
		      return array('Content-type: '. $type, 'Content-Disposition: attachment; filename="'. $file .'"');
		    }
		  }
	  }
  }
}

sha, sorry to be nagging after all the hard work you already did. But would it be possible to make a real patch from this? I cannot see what you changed, nor see what the impact is!

More on patches at http://drupal.org/diffandpatch

FYI...

This issue appears to be the root cause to a problem I've experienced with the Disk Node module, as well. The related Disk Node issue is here:

http://drupal.org/node/68297

Apparently a lot of modules incorrectly return false, and that module's maintainer has filed an issue against core, since the problem could be fixed there by not creating an entry in the $headers array when a module returns false:

http://drupal.org/node/78721

Turns out making a "real" patch isn't as hard as I feared... here it is.

Hi sja,

Thanks, and congrats on your first patch!

One thing I noticed off the bat is that it the code indentation no longer meets with Drupal's coding standards, which call for two spaces and no tabs (http://drupal.org/node/318).

Aside from readability, one reason this is important is because it will allow us to review your patch and identify which lines of code are truly different. Right now they all show up as different because the spacing is unnecessarily changed.

Can you fix this and re-roll your patch?