Do not delete temp file before writing to it (#6951)

* Do not delete temp file before writing to it
This patch changes NettyStreamingFileUpload to not delete the temp file it creates before writing to it. It is not necessary to delete the file, and deleting it weakens the security, because another user may recreate the temp file with lax permissions between the deletion and transferTo. This could potentially allow that process to read the file contents.

This temp file code path is somewhat pointless, though. The temp file name is not predictable, and the path is exposed to the API user nowhere, so there is actually no way for the user to get at the file if they call `transferTo(String)` without configuring the `micronaut.server.multipart.location`. We should just throw an error instead, but that's a breaking change.

Because there's no way to get at the file, the test case also doesn't actually verify that the file has the right permissions, even though it really should. I checked manually and confirmed that on linux, `createTempFile` sets the permissions to 600 by default.

I added a test to verify that the temp file transferTo works at all (doesn't error), but the content can't be verified.

* deprecate transferTo

http-server-netty/src/main/java/io/micronaut/http/server/netty/multipart/NettyStreamingFileUpload.java

@@ -134,16 +134,11 @@ public Publisher<Boolean> delete() {
      * @return The temporal file
      */
     protected File createTemp(String location) {
-        File tempFile;
         try {
-            tempFile = Files.createTempFile(DiskFileUpload.prefix, DiskFileUpload.postfix + '_' + location).toFile();
+            return Files.createTempFile(DiskFileUpload.prefix, DiskFileUpload.postfix + '_' + location).toFile();
         } catch (IOException e) {
-            throw new MultipartException("Unable to create temp directory: " + e.getMessage(), e);
+            throw new MultipartException("Unable to create temp file: " + e.getMessage(), e);
         }
-        if (tempFile.delete()) {
-            return tempFile;
-        }
-        return null;
     }
 
     @Override

http/src/main/java/io/micronaut/http/multipart/StreamingFileUpload.java

@@ -46,7 +46,9 @@ public interface StreamingFileUpload extends FileUpload, Publisher<PartData> {
      * @param location the name of the file to which the stream will be written. The file is created relative to
      *                 the location as specified in the <tt>MultipartConfiguration</tt>
      * @return A {@link Publisher} that outputs whether the transfer was successful
+     * @deprecated Use {@link #transferTo(File)} or {@link #transferTo(OutputStream)} instead.
      */
+    @Deprecated
     Publisher<Boolean> transferTo(String location);
 
     /**

test-suite/src/test/groovy/io/micronaut/upload/NoLocationTransferSpec.groovy

@@ -0,0 +1,42 @@
+package io.micronaut.upload
+
+import io.micronaut.AbstractMicronautSpec
+import io.micronaut.http.HttpRequest
+import io.micronaut.http.HttpResponse
+import io.micronaut.http.HttpStatus
+import io.micronaut.http.MediaType
+import io.micronaut.http.client.multipart.MultipartBody
+import io.netty.handler.codec.http.multipart.DiskFileUpload
+import reactor.core.publisher.Flux
+
+class NoLocationTransferSpec extends AbstractMicronautSpec {
+
+    @Override
+    Map<String, Object> getConfiguration() {
+        // leave micronaut.server.multipart.location unset
+        [:]
+    }
+
+    void "test simple in-memory file upload with JSON"() {
+        given:
+        DiskFileUpload.baseDirectory = null
+
+        MultipartBody requestBody = MultipartBody.builder()
+                .addPart("data", "data.json", MediaType.APPLICATION_JSON_TYPE, '{"title":"Foo"}'.bytes)
+                .addPart("title", "bar")
+                .build()
+
+        when:
+        Flux<HttpResponse<String>> flowable = Flux.from(client.exchange(
+                HttpRequest.POST("/upload/receive-file-upload", requestBody)
+                        .contentType(MediaType.MULTIPART_FORM_DATA)
+                        .accept(MediaType.TEXT_PLAIN_TYPE),
+                String
+        ))
+        def response = flowable.blockFirst()
+
+        then:
+        response.code() == HttpStatus.OK.code
+    }
+
+}