[development] files owned by uid, patch review request.

Darrel O'Pry dopry at thing.net
Fri May 4 22:47:10 UTC 2007


I've got one patch I've been working on for a while. I re-rolled it a
couple days ago. I'd appreciate getting it reviewed from a few people.
So far in my tests it works flawlessly, I'd really like to see this in 
D6. 

The idea to to associate files in the files table to a user id instead
of a node id. Upload, Filefield, Imagefield, etc will still be able to
associate files to nodes. This particular patch updates Upload to use
the file_revisions table to maintain the node>-<file associations, maybe
it should become the upload_node table.

QUICK OVERVIEW:

Problems this patch solves:
 - any thing related to displaying files on node previews.
 - hanging files in drupal's temp dir.
 - centralizes some security features that are in upload to file.inc
 
Problems this patch creates:
 - no ui to access orphaned files in files table.

Features this patch adds:
 - global quotas and extension limits on uploads per role.
 - save files with out a node via file_save_data.

Possibilities this patch creates:
 - associating existing files to nodes. 
 - advanced file browsers
 - extending db handling to file_move, file_copy, file_delete.
 - extended file functions would be an ideal spot to implement a
hook_file
     
VERBOSE OVERVIEW:    

This patch moves file upload limits and validation out of upload.module
and to Drupal Core. This allows administrators to set allowed extensions
per role, and quotas that will be enforced for any module using core's
file_save_upload function.

It also moves upload_munge_filename into file.inc so other modules
handling files do no have to reimplement it or depend on upload.module.

Database handling has worked it's way into file_save_upload. When you
call file_save_upload('upload'), It validates the file by testing quotas
and allowed extensions, munges the file name to prevent the mime based
exploits, saves the file to its destination, and adds a record to the
files table.

This patch also completely removes file_check_upload, since it is fully
merged with file_save_upload now..

This is a stepping stone to including more database interaction in
file.inc. In a better world you would file_copy($file_object,
$destination), instead of if (file_copy($file_object->filepath,
destination)) { db_query(INSERT ...) }. This doesn't preclude keeping
_file_copy($filepath, $destpath) which will probably become a helper
function of file_move. It does smell of an end to the mixed parameter
file functions that accept both objects and paths and do quite the
internal dance to support it.

File functions that interact with the database would be a great place to
implement life cycle hooks for files, since you have the file object as
a container for metadata, which isn't always the case with the file
functions in their current state. Fid is a better key than
md5_file($path) anyways.

This patch introduces one major issue, which is dealing with
unassociated files. There is currently no UI for navigating the files
table. I'm not sure if I have the time to build a core worthy browser
before the freeze.

.darrel.

ps. ted & nate, if you guys still have time I'd like to actually get the
file browser at least started the 3rd week of may.



More information about the development mailing list