Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[http] fixing content-length checking #437

Merged
merged 2 commits into from
May 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions http/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]> {
Expand All @@ -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];
}

Expand Down
80 changes: 46 additions & 34 deletions http/server_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
});
Expand Down