rocket-multipart-form-data icon indicating copy to clipboard operation
rocket-multipart-form-data copied to clipboard

Use a tempfile for file fields

Open Vypo opened this issue 5 years ago • 4 comments

If a file is uploaded by a user, and stored in the file system, it hangs around forever. If the program exits, crashes, or even if the developer ignores the file, it will clutter the temp directory.

I think using tempfile() (or if there are too many issues with unnamed files, NamedTempFile) would deal with the issue effectively.

Vypo avatar Apr 02 '20 19:04 Vypo

Thanks for your advice.

If the developer ignores files that are stored in the file system after calling the parse method of a MultipartFormData instance, those files should be deleted automatically when the MultipartFormData instance is dropped. (unless the developer changes the MultipartFormData instance)

impl Drop for MultipartFormData {
    #[inline]
    fn drop(&mut self) {
        let files = &self.files;

        for field in files.values() {
            match field {
                FileField::Single(f) => {
                    try_delete(&f.path);
                }
                FileField::Multiple(vf) => {
                    for f in vf {
                        try_delete(&f.path);
                    }
                }
            }
        }
    }
}

Therefore, I think using NamedTempFile would make no difference.

The tempfile() function is using the O_TMPFILE | O_EXCL flags on Linux or the FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE flags on Windows to create a temporary file, which should prevent leaking when the program exits or crashes. But I don't know how to transform this kind of a temporary file to a regular file......

magiclen avatar Apr 03 '20 15:04 magiclen

Thanks for getting back to me! I didn't realize there was already some functionality for this. I noticed a couple files hanging around, and naively searched for Drop in multipart_form_data_field.rs and nowhere else.

Would it then make sense to add a lifetime to SingleFileField, to show that the path is only valid as long as the MultipartFormData is alive?


The manpage for open(2) has a nice example of using linkat to assign a name to a file descriptor, though I think that's probably way out of scope of a multipart upload crate for Rocket.

char path[PATH_MAX];
fd = open("/path/to/dir", O_TMPFILE | O_RDWR,
                      S_IRUSR | S_IWUSR);

/* File I/O on 'fd'... */

linkat(fd, NULL, AT_FDCWD, "/path/for/file", AT_EMPTY_PATH);

/* If the caller doesn't have the CAP_DAC_READ_SEARCH
 capability (needed to use AT_EMPTY_PATH with linkat(2)),
 and there is a proc(5) filesystem mounted, then the
 linkat(2) call above can be replaced with:

snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file",
                      AT_SYMLINK_FOLLOW);
*/

Vypo avatar Apr 03 '20 17:04 Vypo

Is there any benefit to add a lifetime which is not necessary?

If linkat can do the job on Linux, how about Windows and other operating systems?

magiclen avatar Apr 04 '20 11:04 magiclen

Is there any benefit to add a lifetime which is not necessary?

I would say so. It clearly indicates to the developer that the SingleFileField is only valid for a limited time. Though I may not be the most astute developer, I clearly missed that while reading the docs, so there'd be at least one less person bitten by it.

If linkat can do the job on Linux, how about Windows and other operating systems?

I think you can use CreateHardLink on windows. I haven't tested it though.

Other unix implementations create the file normally, then unlink it immediately. I don't think there's a way to relink the file.

Vypo avatar Apr 08 '20 18:04 Vypo