5

For my application, I have to write a method that takes an InputStream as argument, writes the content to a temporary file, performs some operations and finally deletes the temporary.

This is what I have so far:

public void myMethod(InputStream in, String name) {
    //...
    Path path = Paths.get("./tmp/benchmarks/" + name + ".zip")

    try {
        Files.copy(in, path);
        //operations...
    } catch (IOException e) {
        //error handling for copy...
    } finally {
        try {
            Files.delete(path));
       } catch (IOException e) {
           //error handling for delete...
       }
    }
    //...
}

It does the job, but it also looks really ugly. I was wondering if there was some way to use try-with-resources to handle this more gracefully. Is it possible somehow?

UPDATE: I wrote an on-the-fly solution in ten minutes. It looks like this:

public class TemporaryFileHandler implements AutoCloseable {

    private File file;

    public TemporaryFileHandler(final InputStream in, final Path path) throws IOException {
        Files.copy(in, path);
        this.file = new File(path.toString());
    }

    public File getFile() { return file; }

    @Override
    public void close() throws IOException {
        Files.delete(file.toPath());
    }
}

I'm sure it's not the best, but it does the job for now. If anyone has suggestions on how to improve this in any way, suggestions are more than welcome.

link
  • 1,676
  • 1
  • 13
  • 21
  • 1
    You know about `Files.createTempFile`? –  Dec 02 '15 at 17:40
  • @RC. yes, but using that means that the file will be deleted on the JVM's shutdown. Ideally, my JVM shouldn't shut down (I am developing a REST API). – link Dec 02 '15 at 17:42
  • 1
    Not tried but something similar to `try (InputStream in = Files.newInputStream(tempFile, StandardOpenOption.DELETE_ON_CLOSE))` could work for you. Works on streams so it's probably not as convenient as file based. – zapl Dec 02 '15 at 18:02
  • Also, if not implementing `AutoCloseable` for a temporary file like @Jazeppi suggested, you could just create a static method like `runWithTemporaryFile` that could just accept a sort of an `IOException`-friendly interface instance and let the method do dirty work itself entirely. This can look nice in Java 8 if the interface is `@FunctionalInterface`. Another option would be implementing a template method, say, `TemporaryFileConsumer` so it's probably only abstract method could consume the temporary file before it's deleted in the base, `TemporaryFileConsumer`. – Lyubomyr Shaydariv Dec 02 '15 at 18:03

5 Answers5

7

I think with a little helper / wrapper like

public class AutoDeletingTempFile implements AutoCloseable {

    private final Path file;

    public AutoDeletingTempFile() throws IOException {
        file = Files.createTempFile(null, null);
    }

    public Path getFile() {
        return file;
    }

    @Override
    public void close() throws IOException {
        Files.deleteIfExists(file);
    }
}

which gets closed and deletes the file it wraps you get a nice and short syntax:

public void myMethod(InputStream in, String name) {
    try (AutoDeletingTempFile wrapper = new AutoDeletingTempFile()) {
        //Files.copy(in, wrapper.getFile());
        //operations...
    } catch (IOException e) {
        //error handling for copy...
        // + temp file creation
    }
}

or a neat little Closable via lambdas

public void myMethod(InputStream in, Path existingFile, String name) {
    try (Closeable closable = () -> Files.deleteIfExists(existingFile)) {
        // ...
    } catch (IOException e) {
        // 
    }
}
zapl
  • 63,179
  • 10
  • 123
  • 154
  • Thank you! It's similar to my solution, but I guess that storing a reference to a `Path` as you do may be better than storing a reference to a `File`. I'll rework a little my solution. – link Dec 02 '15 at 18:44
4

Try-with-resource just calls the close method on classes implementing the interfaces implementing java.lang.AutoCloseable. There's nothing from stopping you from creating a File implementation that implements AutoCloseable and deletes itself when close() is called.

You can also call deleteOnExit() on the file to have the JVM delete it when it exits. This is only appropriate if you're okay with waiting until the JVM is done to delete your temporary file. This probably wouldn't be a good idea for a long running JVM like a Java webapp.

Jazzepi
  • 5,259
  • 11
  • 55
  • 81
  • Thank you for your suggestion. I already knew about `AutoCloseable`, but I was hoping something similar had already been implemented. Since it doesn't seem to be the case, I'll do it myself. Just in case, I will wait a little more before accepting your answer. – link Dec 02 '15 at 18:01
  • @link I feel your pain. Good luck if you find something that does this specifically :) Try-with-resource is relatively new. If you do create a class that deletes itself I'd upload it to Git or something and link it from here. I'm sure other people could use it. – Jazzepi Dec 02 '15 at 18:24
  • thank you! See my update for a very quick solution :) let me know if you have any suggestions! – link Dec 02 '15 at 18:32
  • 1
    @link according to the AutoCloseable docs `implementers of this interface are strongly encouraged to make their close methods idempotent.` Ergo I would use deleteIfExists() instead of delete() – Jazzepi Dec 02 '15 at 18:40
3

You could do something like this in java 8:

Path path = Files.createTempFile("temp-", ".tmp");
try (Closeable onClose = () -> Files.delete(path)) {
    ...
}

but this is really the same as:

Path path = Files.createTempFile("temp-", ".tmp");
try {
    ...
} finally {
    Files.delete(path);
}
Xavier Dury
  • 1,530
  • 1
  • 16
  • 23
1

Files.createTempFile allows you to create a temporary file in the default temporary directory of the JVM. This does not mean that the file is automatically deleted or marked for deletion using File.deleteOnExit(). The developer is responsible for managing the lifecycle of temporary files.

The name of the file is a vector for security vulnerabilities. Only use the name for display in the UI for validation and feedback. Do not use untrusted user input for file names.

It is not possible to manage the lifecycle of file with try-with-resources using java.io.File or java.nio.file.Path. The InputStream is manageable with the try-with-resources.

public void myMethod(InputStream in) {
    Path path = null;

    try (InputStream stream = in) {
        path = Files.createTempFile(null, ".zip");
        Files.copy(stream, path);
    } catch (IOException e) {
        //error handling for copy...
    } finally {
        if (path != null) {
            try {
                Files.delete(path));
           } catch (IOException e) {
               //error handling for delete...
           }
        }
    }
}
Aaron
  • 574
  • 3
  • 8
  • Thank you for the code, I know that I can handle `InputStream`s in try with resources, but it's not what I am interested in doing here :) Nevertheless, the comment about the security vulnerability sounds interesting. I know it's not strictly related, but could you elaborate a little more? – link Dec 02 '15 at 18:28
  • The name of the file may contain references to directories such as ~/ and ../. This will change the directory the temporary file is written to. – Aaron Dec 02 '15 at 18:44
  • I didn't think about this at all. Thank you for the suggestion :) – link Dec 02 '15 at 18:45
0

I had a similar problem where temporary ZIP files weren't being deleted. My assumption is that the output streams weren't being closed before the code attempted to delete the temp file.

My solution, which uses a nested try, isn't as elegant but it should guarantee that the streams are closed beforehand.

Before

File out = // get file;

try(
    FileOutputStream fos = new FileOutputStream(out);
    ZipOutputStream zos =  new ZipOutputStream(fos);
){
    // Create ZIP file and deliver to client using HTTPServletResponse
}
finally{
    if (out != null){
        out.delete();
    }
}

After

File out = // get file;

try{
    try(
        FileOutputStream fos = new FileOutputStream(out);
        ZipOutputStream zos =  new ZipOutputStream(fos);
    ){
        // Create ZIP file and deliver to client using HTTPServletResponse
    }
}
finally{
    if (out != null){
        out.delete();
    }
}
Ian
  • 7,480
  • 2
  • 47
  • 51