Private upload automatically replaces files instead of renaming them
PWG - July 2, 2008 - 13:02
| Project: | Private Upload |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | patch (to be ported) |
Description
When the Private upload module makes files private and moves them from /files to /files/private it replaces any existing files with the same filenames thus potentially deleting someone else's files and giving them access to your files instead. This is not ideal and it is also contradictory to the default behavior of the Upload module - which renames new files with duplicate filenames - causing extra confusion for the developer as to what is going on. It would be good if there was a setting where admin could choose if private files with duplicate filenames should replace the original files or be renamed.
Petter
#1
I'm attaching a patch to fix this issue - it uses the default drupal behavior of appending an incremented digit to duplicate filenames. It would be easy to make a preference for this behavior, but my contention is that this patch produces the the correct behavior, as it avoids database inconsistency. (producing messages like this: There are '29' files in the private folder, and the DB thinks there are '116' private files.)
#2
I'm attaching a patch to fix this issue - it uses the default drupal behavior of appending an incremented digit to duplicate filenames. It would be easy to make a preference for this behavior, but my contention is that this patch produces the the correct behavior, as it avoids database inconsistency. (producing messages like this: There are '29' files in the private folder, and the DB thinks there are '116' private files.)
#3
See also #371492: Upload a file with the same name doesn't remove the old upload. Same bug exists in D6. It can cause loss of files, incorrect file served, and corrupt (truncated by wrong filesize) files served.
#4
attached is a patch for the 6 branch.
#5
Reviewed and tested Justin's patch.
As well as patching the module you may need to do some manual cleanup of your files if they were affected by this. A few of the things I did to generate lists of files needing cleanup:
The bug could result in having missing files or files that were overwritten with a different file. This will print out files that do not match their filesize in the database, either because they are missing or because they were overwritten with a different file:
<?php$result = db_query("SELECT * FROM {files}");
while ($file = db_fetch_object($result)) {
if( filesize(realpath($file->filepath)) != $file->filesize) {
print $file->filepath .' '. $file->filesize .' '. filesize(realpath($file->filepath)) .' ';
}
}
?>
Sql query to list filepaths that are in the files and upload table more than once.
<?phpSELECT f.filename, u.nid, u2.nid FROM files f INNER JOIN files f2 ON f.filepath = f2.filepath INNER JOIN upload u ON u.fid = f.fid INNER JOIN upload u2 ON u2.fid = f2.fid WHERE f.fid <> f2.fid AND u.fid <> u2.fid AND f.timestamp > f2.timestamp ORDER BY f.timestamp ASC;
?>
#6
Fixed in the D6 dev branch. Still an issue for D5. The patch in comment 1 needs review and testing or the patch in #4 needs porting.