From 9f489a474d5761ca5715f1e13a3d39023a656424 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 16 May 2017 01:56:16 -0400 Subject: [PATCH] parser: fix HTTP version parsing Only one digit is allowed for the major version and only one is allowed for the minor version according to RFC 7230. PR-URL: https://github.com/nodejs/http-parser/pull/366 Reviewed-By: Fedor Indutny --- http_parser.c | 112 +++++++++++++------------------------------------- test.c | 21 +++++++++- 2 files changed, 47 insertions(+), 86 deletions(-) diff --git a/http_parser.c b/http_parser.c index 895bf0c..42b1ecd 100644 --- a/http_parser.c +++ b/http_parser.c @@ -286,10 +286,10 @@ enum state , s_res_HT , s_res_HTT , s_res_HTTP - , s_res_first_http_major , s_res_http_major - , s_res_first_http_minor + , s_res_http_dot , s_res_http_minor + , s_res_http_end , s_res_first_status_code , s_res_status_code , s_res_status_start @@ -316,10 +316,10 @@ enum state , s_req_http_HT , s_req_http_HTT , s_req_http_HTTP - , s_req_first_http_major , s_req_http_major - , s_req_first_http_minor + , s_req_http_dot , s_req_http_minor + , s_req_http_end , s_req_line_almost_done , s_header_field_start @@ -795,75 +795,48 @@ reexecute: case s_res_HTTP: STRICT_CHECK(ch != '/'); - UPDATE_STATE(s_res_first_http_major); + UPDATE_STATE(s_res_http_major); break; - case s_res_first_http_major: - if (UNLIKELY(ch < '0' || ch > '9')) { + case s_res_http_major: + if (UNLIKELY(!IS_NUM(ch))) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } parser->http_major = ch - '0'; - UPDATE_STATE(s_res_http_major); + UPDATE_STATE(s_res_http_dot); break; - /* major HTTP version or dot */ - case s_res_http_major: + case s_res_http_dot: { - if (ch == '.') { - UPDATE_STATE(s_res_first_http_minor); - break; - } - - if (!IS_NUM(ch)) { - SET_ERRNO(HPE_INVALID_VERSION); - goto error; - } - - parser->http_major *= 10; - parser->http_major += ch - '0'; - - if (UNLIKELY(parser->http_major > 999)) { + if (UNLIKELY(ch != '.')) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } + UPDATE_STATE(s_res_http_minor); break; } - /* first digit of minor HTTP version */ - case s_res_first_http_minor: + case s_res_http_minor: if (UNLIKELY(!IS_NUM(ch))) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } parser->http_minor = ch - '0'; - UPDATE_STATE(s_res_http_minor); + UPDATE_STATE(s_res_http_end); break; - /* minor HTTP version or end of request line */ - case s_res_http_minor: + case s_res_http_end: { - if (ch == ' ') { - UPDATE_STATE(s_res_first_status_code); - break; - } - - if (UNLIKELY(!IS_NUM(ch))) { - SET_ERRNO(HPE_INVALID_VERSION); - goto error; - } - - parser->http_minor *= 10; - parser->http_minor += ch - '0'; - - if (UNLIKELY(parser->http_minor > 999)) { + if (UNLIKELY(ch != ' ')) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } + UPDATE_STATE(s_res_first_status_code); break; } @@ -1153,57 +1126,41 @@ reexecute: case s_req_http_HTTP: STRICT_CHECK(ch != '/'); - UPDATE_STATE(s_req_first_http_major); - break; - - /* first digit of major HTTP version */ - case s_req_first_http_major: - if (UNLIKELY(ch < '1' || ch > '9')) { - SET_ERRNO(HPE_INVALID_VERSION); - goto error; - } - - parser->http_major = ch - '0'; UPDATE_STATE(s_req_http_major); break; - /* major HTTP version or dot */ case s_req_http_major: - { - if (ch == '.') { - UPDATE_STATE(s_req_first_http_minor); - break; - } - if (UNLIKELY(!IS_NUM(ch))) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } - parser->http_major *= 10; - parser->http_major += ch - '0'; + parser->http_major = ch - '0'; + UPDATE_STATE(s_req_http_dot); + break; - if (UNLIKELY(parser->http_major > 999)) { + case s_req_http_dot: + { + if (UNLIKELY(ch != '.')) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } + UPDATE_STATE(s_req_http_minor); break; } - /* first digit of minor HTTP version */ - case s_req_first_http_minor: + case s_req_http_minor: if (UNLIKELY(!IS_NUM(ch))) { SET_ERRNO(HPE_INVALID_VERSION); goto error; } parser->http_minor = ch - '0'; - UPDATE_STATE(s_req_http_minor); + UPDATE_STATE(s_req_http_end); break; - /* minor HTTP version or end of request line */ - case s_req_http_minor: + case s_req_http_end: { if (ch == CR) { UPDATE_STATE(s_req_line_almost_done); @@ -1215,21 +1172,8 @@ reexecute: break; } - /* XXX allow spaces after digit? */ - - if (UNLIKELY(!IS_NUM(ch))) { - SET_ERRNO(HPE_INVALID_VERSION); - goto error; - } - - parser->http_minor *= 10; - parser->http_minor += ch - '0'; - - if (UNLIKELY(parser->http_minor > 999)) { - SET_ERRNO(HPE_INVALID_VERSION); - goto error; - } - + SET_ERRNO(HPE_INVALID_VERSION); + goto error; break; } diff --git a/test.c b/test.c index f5744aa..33784d9 100644 --- a/test.c +++ b/test.c @@ -3313,9 +3313,11 @@ test_message_count_body (const struct message *message) } void -test_simple (const char *buf, enum http_errno err_expected) +test_simple_type (const char *buf, + enum http_errno err_expected, + enum http_parser_type type) { - parser_init(HTTP_REQUEST); + parser_init(type); enum http_errno err; @@ -3339,6 +3341,12 @@ test_simple (const char *buf, enum http_errno err_expected) } } +void +test_simple (const char *buf, enum http_errno err_expected) +{ + test_simple_type(buf, err_expected, HTTP_REQUEST); +} + void test_invalid_header_content (int req, const char* str) { @@ -3949,6 +3957,12 @@ main (void) //// RESPONSES + test_simple_type("HTP/1.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE); + test_simple_type("HTTP/01.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE); + test_simple_type("HTTP/11.1 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE); + test_simple_type("HTTP/1.01 200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE); + test_simple_type("HTTP/1.1\t200 OK\r\n\r\n", HPE_INVALID_VERSION, HTTP_RESPONSE); + for (i = 0; i < response_count; i++) { test_message(&responses[i]); } @@ -4026,6 +4040,9 @@ main (void) /// REQUESTS test_simple("GET / HTP/1.1\r\n\r\n", HPE_INVALID_VERSION); + test_simple("GET / HTTP/01.1\r\n\r\n", HPE_INVALID_VERSION); + 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); // Extended characters - see nodejs/test/parallel/test-http-headers-obstext.js test_simple("GET / HTTP/1.1\r\n"