File uploads are left in tmp and other weirdness

is your files/tmp the same as php's upload_tmp?

Do you have any modules besides core modules? Are you testing against a current head. I'm not seeing this behavior.

maybe your apache lacks delete permission on the tmp dir?

anyword on the patch, or whether this is still an issue?

I have some hanging files in drupal's private tmp dir on a local installation.

Could anyone running a 4.7 site live, check their drupal tmp dir for hanging files? This could be big issue for sites upgrading to 4.7 after the release. I want to be certain this is nto an issue before the final release. If it is I have a garbage collection patch I can reroll against head.

upgrading to critical. since it could affect quotas and and be a real headache in shared hosting environments...

(Check drupal's tmp dir, not php's)

.darrel.

I've checked on drupal.org and there is indeed a number of dangling files in /tmp. It is a significant number of files after the data of the update. Most of them are however empty.

-rw------- 1 apache apache 0 Apr 5 23:30 tmp_gN4zbR

downgrading, shoudl be no problem to write a cron based cleanup thingie.

I'm bumping back to critical just to get people to check their temp directories for hanging files. If they're there I'll get the cron based GC patch rerolled ASAP so it can be tested.

I found this same bug last week and discussed it with killes and chx on IRC, due to the potential security nature of it. However it was generally decided to be a sysadmin issue rather than a Drupal issue, though I don't completely agree with this prognosis.

Basically, its possible for a single user with permission to upload files to upload an infinite number of files to the temporary directory, and those files will never be deleted by Drupal. This happens because on uploading a file (whether with upload.module or not - this applies to uploads to issues on drupal.org too) it first uploads to the temp directory on previewing a node or validating a node. But a session variable remembers that the file is there and later deletes it when the same user session visits a new node form or successfully submits the current node.

In theory, a script kiddie could write a script which fills up a server's tmp dir with unlimited files in the time it takes their bandwidth to upload that many files. This has the potential to fill up disks and hence cause database errors, lost files etc etc... not a fun idea at all!

To reproduce (on your own testing server):

- enable upload.module (though any file uploads will do the same)
- node/add/page and attach a file using AJAX (or upload normally and preview/validate the node without submitting it)
- without submitting the node, logout
- login, do the same again
- watch your temp directory fill up with files which will never get deleted now

Some ideas killes, chx and I came up with:
- a cron script to delete old files (though if a script kiddie fills up your disk before the cron run, this is no good)
- deleting files on closing sessions/deleting session info from the sessions db table
- stopping a user doing x y and z more than a certain number of times per x (e.g. flood control)
- ... i think we had more, but can't remember them now

None of the ideas seemed that great though.

To me, this has the potential to be a serious issue. I can just imagine a script kiddie writing a script which logs them in and out, in and out, uploading a file each time, until eventually the tmp directory is so full that the web server has no space left and mysql craps out and all kinds of things go wrong.

The right way to fix such problems is to have the tmp directory on its own partition so that even if that runs full the only bad thing that happens is that you cant upload more files.
The only other usefull way to counter such a DoS attack is to build in a throttle mechanism so that a given user account can only leave so many dangling files.

I think drupal should al least make an attempt to clean up hanging files over a day old. Not everyone has access to create a unique tmp partition, and a lot of the open basedir code were just to cater to the demographic stuck with shared hosting. We shold at least be considerate in cleaning up our tmp space. Even on my servers drupal's tmp is seperate from php's tmp, and only php's tmp has its own partition.

I am firm on this: write a five line shell script which deletes the oldest files from your tmp directory until you have enough space. Run this from cron. Nothing to see here, move on.

i agree with dopry that drupal should attempt to keep things tidy. we already do cleanup of watchdog entries - why are files different from Db records?

If for some reason the cron script is preferred over hook_cron(), then we should ship with such a script in /scripts.

i'm with moshe and others. drupal should do it's own garbage collection. however, i don't think this is critical for 6.7.0. i could see us fixing this in the code (or adding the suggested cron script into scripts) in either 6.7.1 or even putting it off until 6.8, but i don't think drupal should leave junk in /tmp on the assumption that sysadmins will clear it out for us. re-opening the issue and downgrading it so it's not forgotten completely.

Hmm, I don't think its critical for 6.7.0 either... but I think 3D electron-ray-defribulas will be by then.

I do however think that it is Drupal's job to clean up these files. As I said earlier, otherwise they don't get deleted at all. How often do you look in your tmp directory/ies?

blimin' b'llocks... 4.7.0 is out already!

Following #13, #17 and #18 Drupal should try to keep things tidy. If it's Drupal's data, Drupal should have control hover it.

thanks for the bump on this issue. however, it's still a bug, not a feature. ;) drupal is leaking files. that's incorrect behavior.

bump, any news??

The file system changed meanwhile, but the issue still exists. The cron should clear the temp folder the same way it does to the watchdog table.

The upload module handles temporary files cleanly. The only way to have drupal leave junk files was when uploading a new logo on the theme settings page. When looking at the code for the theme settings form in system.admin.inc, I found this:

    // The image was saved using file_save_upload() and was added to the
    // files table as a temporary file. We'll make a copy and let the garbage
    // collector delete the original upload.

I'm not sure what "garbage collector" is referred to here, but it's definitely not working when using a tmp folder other than php's upload_tmp.

The validation and saving of uploaded logo and favicon files also happens in the form itself rather than in the submit handler (probably so that if the upload fails the other settings are still saved), but this inconsistent behavior should be fixed.

@magico: This should be resolved in D6.
@flobruit: The garbage collector being referenced is in system_cron().

This issue should be resolved as far as D6 core is concerned. Temp files will be kept for at least 24 hours or mins(I can't remember which offhand), until the garbage collection in system_cron remove them. Note: garbage collection requires cron running on your site.

If you are still experiencing issues with dangling temp files in Drupal 6, please open a new issue with step by step directions to reproduce the issue on a clean Drupal 6 install.