http: fix content-length checking (denoland#437)

caspervonb · May 23, 2019 · ce4e3cc · ce4e3cc
1 parent 94dcb27
commit ce4e3cc
Show file tree

Hide file tree

Showing 2 changed files with 70 additions and 34 deletions.
diff --git a/http/server.ts b/http/server.ts
@@ -196,6 +196,25 @@ export class ServerRequest {
   }
 }
 
+function fixLength(req: ServerRequest): void {
+  const contentLength = req.headers.get("Content-Length");
+  if (contentLength) {
+    const arrClen = contentLength.split(",");
+    if (arrClen.length > 1) {
+      const distinct = [...new Set(arrClen.map((e): string => e.trim()))];
+      if (distinct.length > 1) {
+        throw Error("cannot contain multiple Content-Length headers");
+      } else {
+        req.headers.set("Content-Length", distinct[0]);
+      }
+    }
+    const c = req.headers.get("Content-Length");
+    if (req.method === "HEAD" && c && c !== "0") {
+      throw Error("http: method cannot contain a Content-Length");
+    }
+  }
+}
+
 export async function readRequest(
   bufr: BufReader
 ): Promise<[ServerRequest, BufState]> {
@@ -211,6 +230,11 @@ export async function readRequest(
   }
   [req.method, req.url, req.proto] = firstLine.split(" ", 3);
   [req.headers, err] = await tp.readMIMEHeader();
+  fixLength(req);
+  // TODO(zekth) : add parsing of headers eg:
+  // rfc: https://tools.ietf.org/html/rfc7230#section-3.3.2
+  // A sender MUST NOT send a Content-Length header field in any message
+  // that contains a Transfer-Encoding header field.
   return [req, err];
 }
 

diff --git a/http/server_test.ts b/http/server_test.ts
@@ -322,55 +322,67 @@ test(async function testReadRequestError(): Promise<void> {
     },
     1: { in: "GET / HTTP/1.1\r\nheader:foo\r\n", err: "EOF", headers: [] },
     2: { in: "", err: "EOF", headers: [] },
-    // 3: {
-    //   in: "HEAD / HTTP/1.1\r\nContent-Length:4\r\n\r\n",
-    //   err: "http: method cannot contain a Content-Length"
-    // },
+    3: {
+      in: "HEAD / HTTP/1.1\r\nContent-Length:4\r\n\r\n",
+      err: "http: method cannot contain a Content-Length"
+    },
     4: {
       in: "HEAD / HTTP/1.1\r\n\r\n",
       headers: [],
       err: null
-    }
+    },
     // Multiple Content-Length values should either be
     // deduplicated if same or reject otherwise
     // See Issue 16490.
-    // 5: {
-    //   in:
-    //     "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 0\r\n\r\nGopher hey\r\n",
-    //   err: "cannot contain multiple Content-Length headers"
-    // },
-    // 6: {
-    //   in:
-    //     "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 6\r\n\r\nGopher\r\n",
-    //   err: "cannot contain multiple Content-Length headers"
-    // },
-    // 7: {
-    //   in:
-    //     "PUT / HTTP/1.1\r\nContent-Length: 6 \r\nContent-Length: 6\r\nContent-Length:6\r\n\r\nGopher\r\n",
-    //   err: null,
-    //   headers: [{ key: "Content-Length", value: "6" }]
-    // },
-    // 8: {
-    //   in: "PUT / HTTP/1.1\r\nContent-Length: 1\r\nContent-Length: 6 \r\n\r\n",
-    //   err: "cannot contain multiple Content-Length headers"
-    // },
+    5: {
+      in:
+        "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 0\r\n\r\nGopher hey\r\n",
+      err: "cannot contain multiple Content-Length headers"
+    },
+    6: {
+      in:
+        "POST / HTTP/1.1\r\nContent-Length: 10\r\nContent-Length: 6\r\n\r\nGopher\r\n",
+      err: "cannot contain multiple Content-Length headers"
+    },
+    7: {
+      in:
+        "PUT / HTTP/1.1\r\nContent-Length: 6 \r\nContent-Length: 6\r\nContent-Length:6\r\n\r\nGopher\r\n",
+      err: null,
+      headers: [{ key: "Content-Length", value: "6" }]
+    },
+    8: {
+      in: "PUT / HTTP/1.1\r\nContent-Length: 1\r\nContent-Length: 6 \r\n\r\n",
+      err: "cannot contain multiple Content-Length headers"
+    },
+    // Setting an empty header is swallowed by textproto
+    // see: readMIMEHeader()
     // 9: {
     //   in: "POST / HTTP/1.1\r\nContent-Length:\r\nContent-Length: 3\r\n\r\n",
     //   err: "cannot contain multiple Content-Length headers"
     // },
-    // 10: {
-    //   in: "HEAD / HTTP/1.1\r\nContent-Length:0\r\nContent-Length: 0\r\n\r\n",
-    //   headers: [{ key: "Content-Length", value: "0" }],
-    //   err: null
-    // }
+    10: {
+      in: "HEAD / HTTP/1.1\r\nContent-Length:0\r\nContent-Length: 0\r\n\r\n",
+      headers: [{ key: "Content-Length", value: "0" }],
+      err: null
+    }
   };
   for (const p in testCases) {
     const test = testCases[p];
     const reader = new BufReader(new StringReader(test.in));
-    const [req, err] = await readRequest(reader);
-    assertEquals(test.err, err);
-    for (const h of test.headers) {
-      assertEquals(req.headers.get(h.key), h.value);
+    let _err;
+    if (test.err && test.err != "EOF") {
+      try {
+        await readRequest(reader);
+      } catch (e) {
+        _err = e;
+      }
+      assertEquals(_err.message, test.err);
+    } else {
+      const [req, err] = await readRequest(reader);
+      assertEquals(test.err, err);
+      for (const h of test.headers) {
+        assertEquals(req.headers.get(h.key), h.value);
+      }
     }
   }
 });