Returned file is not released before onComplete invocation

- fix #3765

Author: jknack

jooby/src/main/java/io/jooby/FileDownload.java

@@ -16,6 +16,7 @@
 import java.nio.file.Paths;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 
 /**
  * Represents a file download.
@@ -60,6 +61,10 @@ public enum Mode {
 
   private final InputStream content;
 
+  private boolean deleteOnComplete;
+
+  private Path file;
+
   /**
    * Creates a new file attachment.
    *
@@ -120,6 +125,7 @@ public FileDownload(Mode mode, @NonNull byte[] content, @NonNull String fileName
    */
   public FileDownload(Mode mode, @NonNull Path file, @NonNull String fileName) throws IOException {
     this(mode, new FileInputStream(file.toFile()), fileName, Files.size(file));
+    this.file = file;
   }
 
   /**
@@ -131,6 +137,25 @@ public FileDownload(Mode mode, @NonNull Path file, @NonNull String fileName) thr
    */
   public FileDownload(Mode mode, @NonNull Path file) throws IOException {
     this(mode, file, file.getFileName().toString());
+    this.file = file;
+  }
+
+  /**
+   * True if the file will be deleted after sending to the client.
+   *
+   * @return True if the file will be deleted after sending to the client.
+   */
+  public boolean deleteOnComplete() {
+    return deleteOnComplete;
+  }
+
+  /**
+   * Get the underlying file or <code>null</code>.
+   *
+   * @return Get the underlying file or <code>null</code>.
+   */
+  public @Nullable Path getFile() {
+    return file;
   }
 
   /**
@@ -213,6 +238,21 @@ default FileDownload inline() {
     }
   }
 
+  /**
+   * Add support for delete file on complete when created from {@link #build(Path, String)} or
+   * {@link #build(Path)}.
+   */
+  public interface BuilderExt extends Builder {
+
+    /**
+     * Mark the file to be deleted once it send to the client. This options works on file created
+     * within a {@link Path} instance.
+     *
+     * @return This builder.
+     */
+    Builder deleteOnComplete();
+  }
+
   /**
    * Creates a builder with the specified content which can be used to create a {@link FileDownload}
    * with any {@link Mode}.
@@ -259,12 +299,25 @@ public static Builder build(@NonNull byte[] content, @NonNull String fileName) {
    * @param fileName Filename.
    * @return a {@link Builder} with the specified content
    */
-  public static Builder build(@NonNull Path file, @NonNull String fileName) {
-    return mode -> {
-      try {
-        return new FileDownload(mode, file, fileName);
-      } catch (IOException e) {
-        throw SneakyThrows.propagate(e);
+  public static BuilderExt build(@NonNull Path file, @NonNull String fileName) {
+    return new BuilderExt() {
+      private boolean deleteOnComplete;
+
+      @Override
+      public FileDownload build(Mode mode) {
+        try {
+          var output = new FileDownload(mode, file, fileName);
+          output.deleteOnComplete = deleteOnComplete;
+          return output;
+        } catch (IOException e) {
+          throw SneakyThrows.propagate(e);
+        }
+      }
+
+      @Override
+      public Builder deleteOnComplete() {
+        this.deleteOnComplete = true;
+        return this;
       }
     };
   }
@@ -276,13 +329,7 @@ public static Builder build(@NonNull Path file, @NonNull String fileName) {
    * @param file File content.
    * @return a {@link Builder} with the specified content
    */
-  public static Builder build(@NonNull Path file) {
-    return mode -> {
-      try {
-        return new FileDownload(mode, file);
-      } catch (IOException e) {
-        throw SneakyThrows.propagate(e);
-      }
-    };
+  public static BuilderExt build(@NonNull Path file) {
+    return build(file, file.getFileName().toString());
   }
 }

modules/jooby-jetty/src/main/java/io/jooby/internal/jetty/JettyContext.java

@@ -21,6 +21,7 @@
 import java.nio.channels.FileChannel;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.charset.Charset;
+import java.nio.file.Files;
 import java.security.cert.Certificate;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -38,6 +39,7 @@
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.Content;
 import org.eclipse.jetty.io.content.InputStreamContentSource;
 import org.eclipse.jetty.server.Request;
@@ -51,39 +53,38 @@
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
-import io.jooby.Body;
+import io.jooby.*;
 import io.jooby.ByteRange;
-import io.jooby.CompletionListeners;
-import io.jooby.Context;
-import io.jooby.Cookie;
-import io.jooby.DefaultContext;
-import io.jooby.FileUpload;
-import io.jooby.Formdata;
-import io.jooby.MediaType;
-import io.jooby.QueryString;
-import io.jooby.Route;
-import io.jooby.Router;
-import io.jooby.Sender;
-import io.jooby.Server;
-import io.jooby.ServerSentEmitter;
-import io.jooby.Session;
-import io.jooby.SessionStore;
-import io.jooby.SneakyThrows;
-import io.jooby.StatusCode;
-import io.jooby.WebSocket;
 import io.jooby.internal.jetty.http2.JettyHeaders;
 import io.jooby.output.Output;
 import io.jooby.value.Value;
 
 public class JettyContext implements DefaultContext, Callback {
+  private interface DeleteFileTask {
+    void delete() throws IOException;
+
+    static DeleteFileTask of(FileUpload file) {
+      return file::close;
+    }
+
+    static DeleteFileTask of(FileDownload file) {
+      return () -> {
+        var path = file.getFile();
+        if (path != null) {
+          Files.delete(path);
+        }
+      };
+    }
+  }
+
   private final int bufferSize;
   private final long maxRequestSize;
   Request request;
   Response response;
 
   private QueryString query;
   private Formdata formdata;
-  private List<FileUpload> files;
+  private List<DeleteFileTask> files;
   private Value headers;
   private Map<String, String> pathMap = Collections.EMPTY_MAP;
   private Map<String, Object> attributes = new HashMap<>();
@@ -558,6 +559,14 @@ public Context send(@NonNull ReadableByteChannel channel) {
     return sendStreamInternal(Channels.newInputStream(channel));
   }
 
+  @Override
+  public @NonNull Context send(@NonNull FileDownload file) {
+    if (file.deleteOnComplete()) {
+      register(DeleteFileTask.of(file));
+    }
+    return DefaultContext.super.send(file);
+  }
+
   @NonNull @Override
   public Context send(@NonNull InputStream in) {
     try {
@@ -582,7 +591,8 @@ private Context sendStreamInternal(@NonNull InputStream in) {
         stream = in;
       }
       responseStarted = true;
-      Content.copy(new InputStreamContentSource(stream), response, this);
+      Content.copy(
+          new InputStreamContentSource(stream, ByteBufferPool.SIZED_NON_POOLING), response, this);
       return this;
     } catch (IOException x) {
       throw SneakyThrows.propagate(x);
@@ -633,11 +643,11 @@ public String toString() {
 
   private void clearFiles() {
     if (files != null) {
-      for (FileUpload file : files) {
+      for (var file : files) {
         try {
-          file.close();
+          file.delete();
         } catch (Exception e) {
-          router.getLog().debug("file upload destroy resulted in exception", e);
+          router.getLog().debug("file destroy resulted in exception", e);
         }
       }
       files.clear();
@@ -708,11 +718,15 @@ private void ifSetChunked() {
   }
 
   private FileUpload register(FileUpload upload) {
+    register(DeleteFileTask.of(upload));
+    return upload;
+  }
+
+  private void register(DeleteFileTask deleteFileTask) {
     if (files == null) {
       files = new ArrayList<>();
     }
-    files.add(upload);
-    return upload;
+    files.add(deleteFileTask);
   }
 
   private static void formParam(Request request, Formdata form) {