refactor(std/multipart): make readForm() return value more type safe (#4710)

std/mime/multipart.ts

@@ -251,6 +251,17 @@ function skipLWSPChar(u: Uint8Array): Uint8Array {
   return ret.slice(0, j);
 }
 
+export interface MultipartFormData {
+  file(key: string): FormFile | undefined;
+  value(key: string): string | undefined;
+  entries(): IterableIterator<[string, string | FormFile | undefined]>;
+  [Symbol.iterator](): IterableIterator<
+    [string, string | FormFile | undefined]
+  >;
+  /** Remove all tempfiles */
+  removeAll(): Promise<void>;
+}
+
 /** Reader for parsing multipart/form-data */
 export class MultipartReader {
   readonly newLine = encoder.encode("\r\n");
@@ -268,11 +279,11 @@ export class MultipartReader {
    * overflowed file data will be written to temporal files.
    * String field values are never written to files.
    * null value means parsing or writing to file was failed in some reason.
+   * @param maxMemory maximum memory size to store file in memory. bytes. @default 1048576 (1MB)
    *  */
-  async readForm(
+  async readForm(maxMemory = 10 << 20): Promise<MultipartFormData> {
-    maxMemory: number
+    const fileMap = new Map<string, FormFile>();
-  ): Promise<{ [key: string]: null | string | FormFile }> {
+    const valueMap = new Map<string, string>();
-    const result = Object.create(null);
     let maxValueBytes = maxMemory + (10 << 20);
     const buf = new Buffer(new Uint8Array(maxValueBytes));
     for (;;) {
@@ -292,11 +303,11 @@ export class MultipartReader {
           throw new RangeError("message too large");
         }
         const value = buf.toString();
-        result[p.formName] = value;
+        valueMap.set(p.formName, value);
         continue;
       }
       // file
-      let formFile: FormFile | null = null;
+      let formFile: FormFile | undefined;
       const n = await copy(buf, p);
       const contentType = p.headers.get("content-type");
       assert(contentType != null, "content-type must be set");
@@ -333,9 +344,11 @@ export class MultipartReader {
         maxMemory -= n;
         maxValueBytes -= n;
       }
-      result[p.formName] = formFile;
+      if (formFile) {
+        fileMap.set(p.formName, formFile);
+      }
     }
-    return result;
+    return multipatFormData(fileMap, valueMap);
   }
 
   private currentPart: PartReader | undefined;
@@ -399,6 +412,43 @@ export class MultipartReader {
   }
 }
 
+function multipatFormData(
+  fileMap: Map<string, FormFile>,
+  valueMap: Map<string, string>
+): MultipartFormData {
+  function file(key: string): FormFile | undefined {
+    return fileMap.get(key);
+  }
+  function value(key: string): string | undefined {
+    return valueMap.get(key);
+  }
+  function* entries(): IterableIterator<
+    [string, string | FormFile | undefined]
+  > {
+    yield* fileMap;
+    yield* valueMap;
+  }
+  async function removeAll(): Promise<void> {
+    const promises: Array<Promise<void>> = [];
+    for (const val of fileMap.values()) {
+      if (!val.tempfile) continue;
+      promises.push(Deno.remove(val.tempfile));
+    }
+    await Promise.all(promises);
+  }
+  return {
+    file,
+    value,
+    entries,
+    removeAll,
+    [Symbol.iterator](): IterableIterator<
+      [string, string | FormFile | undefined]
+    > {
+      return entries();
+    },
+  };
+}
+
 class PartWriter implements Writer {
   closed = false;
   private readonly partHeader: string;

std/mime/multipart_test.ts

@@ -1,16 +1,14 @@
 // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license.
 
-const { Buffer, copy, open, remove } = Deno;
+const { Buffer, copy, open, test } = Deno;
 import {
   assert,
   assertEquals,
   assertThrows,
   assertThrowsAsync,
 } from "../testing/asserts.ts";
-const { test } = Deno;
 import * as path from "../path/mod.ts";
 import {
-  FormFile,
   MultipartReader,
   MultipartWriter,
   isFormFile,
@@ -173,46 +171,93 @@ test(async function multipartMultipartWriter3(): Promise<void> {
   );
 });
 
-test(async function multipartMultipartReader(): Promise<void> {
+test({
-  // FIXME: path resolution
+  name: "[mime/multipart] readForm() basic",
-  const o = await open(path.resolve("./mime/testdata/sample.txt"));
+  async fn() {
-  const mr = new MultipartReader(
+    const o = await open(path.resolve("./mime/testdata/sample.txt"));
-    o,
+    const mr = new MultipartReader(
-    "--------------------------434049563556637648550474"
+      o,
-  );
+      "--------------------------434049563556637648550474"
-  const form = await mr.readForm(10 << 20);
+    );
-  assertEquals(form["foo"], "foo");
+    const form = await mr.readForm();
-  assertEquals(form["bar"], "bar");
+    assertEquals(form.value("foo"), "foo");
-  const file = form["file"] as FormFile;
+    assertEquals(form.value("bar"), "bar");
-  assertEquals(isFormFile(file), true);
+    const file = form.file("file");
-  assert(file.content !== void 0);
+    assert(isFormFile(file));
-  o.close();
+    assert(file.content !== void 0);
+    o.close();
+  },
 });
 
-test(async function multipartMultipartReader2(): Promise<void> {
+test({
-  const o = await open(path.resolve("./mime/testdata/sample.txt"));
+  name: "[mime/multipart] readForm() should store big file in temp file",
-  const mr = new MultipartReader(
+  async fn() {
-    o,
+    const o = await open(path.resolve("./mime/testdata/sample.txt"));
-    "--------------------------434049563556637648550474"
+    const mr = new MultipartReader(
-  );
+      o,
-  const form = await mr.readForm(20); //
+      "--------------------------434049563556637648550474"
-  try {
+    );
-    assertEquals(form["foo"], "foo");
+    // use low-memory to write "file" into temp file.
-    assertEquals(form["bar"], "bar");
+    const form = await mr.readForm(20);
-    const file = form["file"] as FormFile;
+    try {
-    assertEquals(file.type, "application/octet-stream");
+      assertEquals(form.value("foo"), "foo");
-    assert(file.tempfile != null);
+      assertEquals(form.value("bar"), "bar");
-    const f = await open(file.tempfile);
+      const file = form.file("file");
-    const w = new StringWriter();
+      assert(file != null);
-    await copy(w, f);
+      assertEquals(file.type, "application/octet-stream");
-    const json = JSON.parse(w.toString());
+      assert(file.tempfile != null);
-    assertEquals(json["compilerOptions"]["target"], "es2018");
+      const f = await open(file.tempfile);
-    f.close();
+      const w = new StringWriter();
-  } finally {
+      await copy(w, f);
-    const file = form["file"] as FormFile;
+      const json = JSON.parse(w.toString());
-    if (file.tempfile) {
+      assertEquals(json["compilerOptions"]["target"], "es2018");
-      await remove(file.tempfile);
+      f.close();
+    } finally {
+      await form.removeAll();
+      o.close();
     }
-    o.close();
+  },
-  }
+});
+
+test({
+  name: "[mime/multipart] removeAll() should remove all tempfiles",
+  async fn() {
+    const o = await open(path.resolve("./mime/testdata/sample.txt"));
+    const mr = new MultipartReader(
+      o,
+      "--------------------------434049563556637648550474"
+    );
+    const form = await mr.readForm(20);
+    const file = form.file("file");
+    assert(file != null);
+    const { tempfile, content } = file;
+    assert(tempfile != null);
+    assert(content == null);
+    const stat = await Deno.stat(tempfile);
+    assertEquals(stat.size, file.size);
+    await form.removeAll();
+    await assertThrowsAsync(async () => {
+      await Deno.stat(tempfile);
+    }, Deno.errors.NotFound);
+    o.close();
+  },
+});
+
+test({
+  name: "[mime/multipart] entries()",
+  async fn() {
+    const o = await open(path.resolve("./mime/testdata/sample.txt"));
+    const mr = new MultipartReader(
+      o,
+      "--------------------------434049563556637648550474"
+    );
+    const form = await mr.readForm();
+    const map = new Map(form.entries());
+    assertEquals(map.get("foo"), "foo");
+    assertEquals(map.get("bar"), "bar");
+    const file = map.get("file");
+    assert(isFormFile(file));
+    assertEquals(file.filename, "tsconfig.json");
+    o.close();
+  },
 });