8000 http_parser: revert `memchr()` optimization · nodejs/http-parser@2a0d106 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Commit 2a0d106

Browse files
indutnyindutny
committed
http_parser: revert memchr() optimization
An optimization was introduced in c6097e1 and 0097de5. 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: #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>
1 parent 0d0a24e commit 2a0d106

File tree

2 files changed

+20
-21
lines changed

2 files changed

+20
-21
lines changed

‎http_parser.c

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1496,28 +1496,24 @@ size_t http_parser_execute (http_parser *parser,
14961496

14971497
switch (h_state) {
14981498
case h_general:
1499-
{
1500-
const char* p_cr;
1501-
const char* p_lf;
1502-
size_t limit = data + len - p;
1503-
1504-
limit = MIN(limit, max_header_size);
1505-
1506-
p_cr = (const char*) memchr(p, CR, limit);
1507-
p_lf = (const char*) memchr(p, LF, limit);
1508-
if (p_cr != NULL) {
1509-
if (p_lf != NULL && p_cr >= p_lf)
1510-
p = p_lf;
1511-
else
1512-
p = p_cr;
1513-
} else if (UNLIKELY(p_lf != NULL)) {
1514-
p = p_lf;
1515-
} else {
1516-
p = data + len;
1499+
{
1500+
const char* limit = p + MIN(data + len - p, max_header_size);
1501+
1502+
for (; p != limit; p++) {
1503+
ch = *p;
1504+
if (ch == CR || ch == LF) {
1505+
--p;
1506+
break;
1507+
}
1508+
if (!lenient && !IS_HEADER_CHAR(ch)) {
1509+
SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
1510+
goto error;
1511+
}
1512+
}
1513+
if (p == data + len)
1514+
--p;
1515+
break;
15171516
}
1518-
--p;
1519-
break;
1520-
}
15211517

15221518
case h_connection:
15231519
case h_transfer_encoding:

‎test.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4316,6 +4316,9 @@ main (void)
43164316
test_simple("GET / HTTP/11.1\r\n\r\n", HPE_INVALID_VERSION);
43174317
test_simple("GET / HTTP/1.01\r\n\r\n", HPE_INVALID_VERSION);
43184318

4319+
test_simple("GET / HTTP/1.0\r\nHello: w\1rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);
4320+
test_simple("GET / HTTP/1.0\r\nHello: woooo\2rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);
4321+
43194322
// Extended characters - see nodejs/test/parallel/test-http-headers-obstext.js
43204323
test_simple("GET / HTTP/1.1\r\n"
43214324
"Test: Düsseldorf\r\n",

0 commit comments

Comments
 (0)
0