passing in a full destination path to file_move / copy could cause the file system to be abused by trying to place files outside of the file system directory.

if the paths stored in the {file} table are stored as being relative to the file_directory_path(), (http://drupal.org/node/366464) this will cause problems. so imo, the $destination arg should be relative to the files directory, and not relative to the drupal installation.

should file_unmanaged_move/copy be left as relative dirs to the drupal installation. as these don't touch the db, i think the above restriction doesn't apply?

opinions?

Comments

George2’s picture

George2 commented
Issue tags: +File API

i think they should because by limiting the output of these functions to one place means that all files related to the drupal installation will be contained in one place.

this would cause a problem however if these functions are used to move files to the temp directory, so maybe the function should check if the destination is the temp dir, and if not, move the file relative to the file_dir_path?

j.somers’s picture

j.somers commented

I disagree.

In one of the custom modules of the website I am currently working on I use these functions to place certain files outside of the web root directory to a "protected" directory which cannot be access from the website itself. It would be silly to create my own implementations of these functions to do that.

drewish’s picture

drewish commented

I think what we really need to do is to rework #166759: Public/Private File Handling and have two "files" directories: files-public, files-private in addition to a temporary directory. Once that's the case then depending on the $file->status file_copy() would only allow the file into one of those three directories based on the $file->status.

If j.somers needs the file to be in another destination then he could use file_unmanaged_copy() which would not enforce the limitation.

aaron’s picture

aaron commented

I'm not sure this is even an issue now, as we're using streams for file storage. Can we close this issue?

Status: Active » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.