Fix MultipartReader for big files (#4865)

denoland · Apr 29, 2020 · 78e0ae6 · 78e0ae6
1 parent d308e8d
commit 78e0ae6
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 21 deletions.
diff --git a/std/mime/multipart.ts b/std/mime/multipart.ts
@@ -308,7 +308,7 @@ export class MultipartReader {
       }
       // file
       let formFile: FormFile | undefined;
-      const n = await copy(p, buf);
+      const n = await copyN(p, buf, maxValueBytes);
       const contentType = p.headers.get("content-type");
       assert(contentType != null, "content-type must be set");
       if (n > maxMemory) {
@@ -319,11 +319,8 @@ export class MultipartReader {
           postfix: ext,
         });
         try {
-          const size = await copyN(
-            new MultiReader(buf, p),
-            file,
-            maxValueBytes
-          );
+          const size = await copy(new MultiReader(buf, p), file);
+
           file.close();
           formFile = {
             filename: p.fileName,
@@ -333,6 +330,7 @@ export class MultipartReader {
           };
         } catch (e) {
           await remove(filepath);
+          throw e;
         }
       } else {
         formFile = {

diff --git a/std/mime/multipart_test.ts b/std/mime/multipart_test.ts
@@ -1,6 +1,6 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
-const { Buffer, copy, open, test } = Deno;
+const { Buffer, open, test } = Deno;
 import {
   assert,
   assertEquals,
@@ -21,6 +21,7 @@ const e = new TextEncoder();
 const boundary = "--abcde";
 const dashBoundary = e.encode("--" + boundary);
 const nlDashBoundary = e.encode("\r\n--" + boundary);
+const testdataDir = path.resolve("mime", "testdata");
 
 test("multipartScanUntilBoundary1", function (): void {
   const data = `--${boundary}`;
@@ -192,29 +193,43 @@ test({
 });
 
 test({
-  name: "[mime/multipart] readForm() should store big file in temp file",
+  name:
+    "[mime/multipart] readForm() should store big file completely in temp file",
   async fn() {
-    const o = await open(path.resolve("./mime/testdata/sample.txt"));
-    const mr = new MultipartReader(
-      o,
-      "--------------------------434049563556637648550474"
-    );
+    const multipartFile = path.join(testdataDir, "form-data.dat");
+    const sampleFile = await Deno.makeTempFile();
+    const writer = await open(multipartFile, { write: true, create: true });
+
+    const size = 1 << 24; // 16mb
+
+    await Deno.truncate(sampleFile, size);
+    const bigFile = await open(sampleFile, { read: true });
+
+    const mw = new MultipartWriter(writer);
+    await mw.writeField("deno", "land");
+    await mw.writeField("bar", "bar");
+    await mw.writeFile("file", "sample.bin", bigFile);
+
+    await mw.close();
+    writer.close();
+    bigFile.close();
+
+    const o = await Deno.open(multipartFile);
+    const mr = new MultipartReader(o, mw.boundary);
     // use low-memory to write "file" into temp file.
     const form = await mr.readForm(20);
     try {
-      assertEquals(form.value("foo"), "foo");
+      assertEquals(form.value("deno"), "land");
       assertEquals(form.value("bar"), "bar");
       const file = form.file("file");
       assert(file != null);
-      assertEquals(file.type, "application/octet-stream");
       assert(file.tempfile != null);
-      const f = await open(file.tempfile);
-      const w = new StringWriter();
-      await copy(f, w);
-      const json = JSON.parse(w.toString());
-      assertEquals(json["compilerOptions"]["target"], "es2018");
-      f.close();
+      assertEquals(file.size, size);
+      assertEquals(file.type, "application/octet-stream");
+      // TODO checksum of tmp & sampleFile
     } finally {
+      await Deno.remove(multipartFile);
+      await Deno.remove(sampleFile);
       await form.removeAll();
       o.close();
     }