http_parser: revert `memchr()` optimization

An optimization was introduced in c6097e1d and 0097de58. The crux of
optimization was to skip all characters in header value until either
of CR or LF. Unfortunately, this optimization comes at cost of
inconsistency in header value validation, which might lead to security
issue due to violated expectations in the user code.

Partially revert the optimization, and add additional check to make
general header value parsing consistent.

Fix: #468
PR-URL: https://github.com/nodejs/http-parser/pull/469
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Harvey Tuch <htuch@google.com>
master
Fedor Indutny 6 years ago
parent 0d0a24e19e
commit 2a0d1065d4

@ -1496,28 +1496,24 @@ reexecute:
switch (h_state) {
case h_general:
{
const char* p_cr;
const char* p_lf;
size_t limit = data + len - p;
limit = MIN(limit, max_header_size);
p_cr = (const char*) memchr(p, CR, limit);
p_lf = (const char*) memchr(p, LF, limit);
if (p_cr != NULL) {
if (p_lf != NULL && p_cr >= p_lf)
p = p_lf;
else
p = p_cr;
} else if (UNLIKELY(p_lf != NULL)) {
p = p_lf;
} else {
p = data + len;
{
const char* limit = p + MIN(data + len - p, max_header_size);
for (; p != limit; p++) {
ch = *p;
if (ch == CR || ch == LF) {
--p;
break;
}
if (!lenient && !IS_HEADER_CHAR(ch)) {
SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
goto error;
}
}
if (p == data + len)
--p;
break;
}
--p;
break;
}
case h_connection:
case h_transfer_encoding:

@ -4316,6 +4316,9 @@ main (void)
test_simple("GET / HTTP/11.1\r\n\r\n", HPE_INVALID_VERSION);
test_simple("GET / HTTP/1.01\r\n\r\n", HPE_INVALID_VERSION);
test_simple("GET / HTTP/1.0\r\nHello: w\1rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);
test_simple("GET / HTTP/1.0\r\nHello: woooo\2rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);
// Extended characters - see nodejs/test/parallel/test-http-headers-obstext.js
test_simple("GET / HTTP/1.1\r\n"
"Test: Düsseldorf\r\n",

Loading…
Cancel
Save