Posted by markus_petrux on March 11, 2009 at 8:17pm
2 followers
Jump to:
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | file system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | active |
Issue Summary
Well, the title says it all. Making the issue critical because file_move() API can destroy data, which is not nice.
In Drupal 6 file_move() looks like this:
<?php
function file_move(&$source, $dest = 0, $replace = FILE_EXISTS_RENAME) {
$path_original = is_object($source) ? $source->filepath : $source;
if (file_copy($source, $dest, $replace)) {
$path_current = is_object($source) ? $source->filepath : $source;
if ($path_original == $path_current || file_delete($path_original)) {
return 1;
}
drupal_set_message(t('The removal of the original file %file has failed.', array('%file' => $path_original)), 'error');
}
return 0;
}
?>Here file_copy() updates $source with the realpath, but it doesn't touch $dest. When $dest is something like './test.txt', it ends up converted to 'sites/example.com/files/./test.txt' that really points to the same exact original file.
Maybe this could be fixed by checking for the realpath of the destination after the copy operation has been executed, which is when it may return data that can be compared with the realpath of the source file.
<?php
if (file_copy($source, $dest, $replace)) {
- $path_current = is_object($source) ? $source->filepath : $source;
+ $path_current = realpath($dest);
if ($path_original == $path_current || file_delete($path_original)) {
?>Another possible fix that comes to mind would be to fix file_copy(), which seems to be a better place for this. So we could do:
<?php
@chmod($dest, 0664);
}
+ if (file_exists($dest)) {
+ $dest = realpath($dest);
+ }
+
if (isset($file) && is_object($file)) {
$file->filename = $basename;
?>
Comments
#1
Is this still an issue in modern Drupal?
#2