From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 0896826737 for ; Fri, 15 Jun 2018 10:24:24 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JjMJtGFxZq_p for ; Fri, 15 Jun 2018 10:24:23 -0400 (EDT) Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6193226742 for ; Fri, 15 Jun 2018 10:24:23 -0400 (EDT) From: Ilya Markov Subject: [tarantool-patches] [http.client 2/2] http: Fix parse long headers names Date: Fri, 15 Jun 2018 17:24:11 +0300 Message-Id: <6d6982b682331716053b4db3efe6b245debde092.1529072604.git.imarkov@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: georgy@tarantool.org Cc: tarantool-patches@freelists.org Bug: During parsing http headers, long headers names are truncated to zero length, but values are not ignored. Fix this with adding parameter max_header_name_length to http request. If header name is bigger than this value, header name is truncated to this length. Default value of max_header_name_length is 32. Do some refactoring with renaming long names in http_parser. Closes #3451 --- src/http_parser.c | 51 ++++++++++++++++++++------------------- src/http_parser.h | 13 +++++----- src/lua/httpc.c | 42 ++++++++++++++++++++++---------- test/app-tap/http_client.test.lua | 8 +++++- test/app-tap/httpd.py | 1 + 5 files changed, 70 insertions(+), 45 deletions(-) diff --git a/src/http_parser.c b/src/http_parser.c index 64f20e5..94d7bc0 100644 --- a/src/http_parser.c +++ b/src/http_parser.c @@ -210,13 +210,13 @@ done: } int -http_parse_header_line(struct http_parser *parser, char **bufp, - const char *end_buf) +http_parse_header_line(struct http_parser *prsr, char **bufp, + const char *end_buf, int max_hname_len) { char c, ch; char *p = *bufp; char *header_name_start = p; - parser->header_name_idx = 0; + prsr->hdr_name_idx = 0; enum { sw_start = 0, @@ -252,19 +252,19 @@ http_parse_header_line(struct http_parser *parser, char **bufp, case sw_start: switch (ch) { case CR: - parser->header_value_end = p; + prsr->hdr_value_end = p; state = sw_header_almost_done; break; case LF: - parser->header_value_end = p; + prsr->hdr_value_end = p; goto header_done; default: state = sw_name; c = lowcase[ch]; if (c != 0) { - parser->header_name[0] = c; - parser->header_name_idx = 1; + prsr->hdr_name[0] = c; + prsr->hdr_name_idx = 1; break; } if (ch == '\0') { @@ -296,9 +296,10 @@ http_parse_header_line(struct http_parser *parser, char **bufp, case sw_name: c = lowcase[ch]; if (c != 0) { - parser->header_name[parser->header_name_idx] = c; - parser->header_name_idx++; - parser->header_name_idx &= (HEADER_LEN - 1); + if (prsr->hdr_name_idx < max_hname_len) { + prsr->hdr_name[prsr->hdr_name_idx] = c; + prsr->hdr_name_idx++; + } break; } if (ch == ':') { @@ -306,25 +307,25 @@ http_parse_header_line(struct http_parser *parser, char **bufp, break; } if (ch == CR) { - parser->header_value_start = p; - parser->header_value_end = p; + prsr->hdr_value_start = p; + prsr->hdr_value_end = p; state = sw_almost_done; break; } if (ch == LF) { - parser->header_value_start = p; - parser->header_value_end = p; + prsr->hdr_value_start = p; + prsr->hdr_value_end = p; goto done; } /* handle "HTTP/1.1 ..." lines */ if (ch == '/' && p - header_name_start == 4 && strncmp(header_name_start, "HTTP", 4) == 0) { - int rc = http_parse_status_line(parser, + int rc = http_parse_status_line(prsr, &header_name_start, end_buf); if (rc == HTTP_PARSE_INVALID) { - parser->http_minor = -1; - parser->http_major = -1; + prsr->http_minor = -1; + prsr->http_major = -1; state = sw_start; } else { /* Skip it till end of line. */ @@ -341,18 +342,18 @@ http_parse_header_line(struct http_parser *parser, char **bufp, case ' ': break; case CR: - parser->header_value_start = p; - parser->header_value_end = p; + prsr->hdr_value_start = p; + prsr->hdr_value_end = p; state = sw_almost_done; break; case LF: - parser->header_value_start = p; - parser->header_value_end = p; + prsr->hdr_value_start = p; + prsr->hdr_value_end = p; goto done; case '\0': return HTTP_PARSE_INVALID; default: - parser->header_value_start = p; + prsr->hdr_value_start = p; state = sw_value; break; } @@ -362,15 +363,15 @@ http_parse_header_line(struct http_parser *parser, char **bufp, case sw_value: switch (ch) { case ' ': - parser->header_value_end = p; + prsr->hdr_value_end = p; state = sw_space_after_value; break; case CR: - parser->header_value_end = p; + prsr->hdr_value_end = p; state = sw_almost_done; break; case LF: - parser->header_value_end = p; + prsr->hdr_value_end = p; goto done; case '\0': return HTTP_PARSE_INVALID; diff --git a/src/http_parser.h b/src/http_parser.h index c4d59a9..7a37ad3 100644 --- a/src/http_parser.h +++ b/src/http_parser.h @@ -32,7 +32,7 @@ #ifndef TARANTOOL_HTTP_PARSER_H #define TARANTOOL_HTTP_PARSER_H -#define HEADER_LEN 32 +#define HEADER_NAME_LEN 32 enum { HTTP_PARSE_OK, @@ -42,14 +42,14 @@ enum { }; struct http_parser { - char *header_value_start; - char *header_value_end; + char *hdr_value_start; + char *hdr_value_end; int http_major; int http_minor; - char header_name[HEADER_LEN]; - int header_name_idx; + char *hdr_name; + int hdr_name_idx; }; /* @@ -62,6 +62,7 @@ struct http_parser { * HTTP_PARSE_INVALID - error during parsing */ int -http_parse_header_line(struct http_parser *parser, char **bufp, const char *end_buf); +http_parse_header_line(struct http_parser *prsr, char **bufp, + const char *end_buf, int max_hname_len); #endif //TARANTOOL_HTTP_PARSER_H diff --git a/src/lua/httpc.c b/src/lua/httpc.c index 0fe27d4..5f4e2e9 100644 --- a/src/lua/httpc.c +++ b/src/lua/httpc.c @@ -60,15 +60,23 @@ lua_add_key_u64(lua_State *L, const char *key, uint64_t value) lua_settable(L, -3); } -static void -parse_headers(lua_State *L, char *buffer, size_t len) +static int +parse_headers(lua_State *L, char *buffer, size_t len, + int max_header_name_len) { struct http_parser parser; + parser.hdr_name = (char *) calloc(max_header_name_len, sizeof(char)); + if (parser.hdr_name == NULL) { + diag_set(OutOfMemory, max_header_name_len * sizeof(char), + "malloc", "hdr_name"); + return -1; + } char *end_buf = buffer + len; lua_pushstring(L, "headers"); lua_newtable(L); while (true) { - int rc = http_parse_header_line(&parser, &buffer, end_buf); + int rc = http_parse_header_line(&parser, &buffer, end_buf, + max_header_name_len); if (rc == HTTP_PARSE_INVALID || rc == HTTP_PARSE_CONTINUE) { continue; } @@ -76,26 +84,26 @@ parse_headers(lua_State *L, char *buffer, size_t len) break; } if (rc == HTTP_PARSE_OK) { - lua_pushlstring(L, parser.header_name, - parser.header_name_idx); + lua_pushlstring(L, parser.hdr_name, + parser.hdr_name_idx); /* check value of header, if exists */ - lua_pushlstring(L, parser.header_name, - parser.header_name_idx); + lua_pushlstring(L, parser.hdr_name, + parser.hdr_name_idx); lua_gettable(L, -3); - int value_len = parser.header_value_end - - parser.header_value_start; + int value_len = parser.hdr_value_end - + parser.hdr_value_start; /* table of values to handle duplicates*/ if (lua_isnil(L, -1)) { lua_pop(L, 1); lua_newtable(L); lua_pushinteger(L, 1); - lua_pushlstring(L, parser.header_value_start, + lua_pushlstring(L, parser.hdr_value_start, value_len); lua_settable(L, -3); } else if (lua_istable(L, -1)) { lua_pushinteger(L, lua_objlen(L, -1) + 1); - lua_pushlstring(L, parser.header_value_start, + lua_pushlstring(L, parser.hdr_value_start, value_len); lua_settable(L, -3); } @@ -120,6 +128,7 @@ parse_headers(lua_State *L, char *buffer, size_t len) /* proto */ lua_settable(L, -3); + return 0; } /* }}} */ @@ -142,7 +151,7 @@ luaT_httpc_request(lua_State *L) return luaT_error(L); double timeout = TIMEOUT_INFINITY; - + int max_header_name_length = HEADER_NAME_LEN; if (lua_isstring(L, 4)) { size_t len = 0; const char *body = lua_tolstring(L, 4, &len); @@ -250,6 +259,11 @@ luaT_httpc_request(lua_State *L) timeout = lua_tonumber(L, -1); lua_pop(L, 1); + lua_getfield(L, 5, "max_header_name_length"); + if (!lua_isnil(L, -1)) + max_header_name_length = lua_tonumber(L, -1); + lua_pop(L, 1); + lua_getfield(L, 5, "verbose"); if (!lua_isnil(L, -1) && lua_isboolean(L, -1)) httpc_set_verbose(req, true); @@ -278,7 +292,9 @@ luaT_httpc_request(lua_State *L) httpc_request_delete(req); return luaT_error(L); } - parse_headers(L, headers, headers_len); + if (parse_headers(L, headers, headers_len, + max_header_name_length) < 0) + diag_log(); } size_t body_len = region_used(&req->resp_body); diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua index d9380c0..a64d75f 100755 --- a/test/app-tap/http_client.test.lua +++ b/test/app-tap/http_client.test.lua @@ -197,7 +197,7 @@ local function test_errors(test) end local function test_headers(test, url, opts) - test:plan(16) + test:plan(19) local http = client:new() local r = http:get(url .. 'headers', opts) test:is(type(r.headers["set-cookie"]), 'string', "set-cookie check") @@ -216,6 +216,12 @@ local function test_headers(test, url, opts) test:ok(r.cookies["bad@name"] == nil , "cookie name check") test:ok(r.cookies["badname"] == nil , "cookie name check") test:ok(r.cookies["badcookie"] == nil , "cookie name check") + test:isnil(r.headers["very_very_very_long_headers_name1"], "no long header name") + test:is(r.headers["very_very_very_long_headers_name"], "true", "truncated name") + opts["max_header_name_length"] = 64 + local r = http:get(url .. 'headers', opts) + test:is(r.headers["very_very_very_long_headers_name1"], "true", "truncated max_header_name_length") + opts["max_header_name_length"] = nil end local function test_special_methods(test, url, opts) diff --git a/test/app-tap/httpd.py b/test/app-tap/httpd.py index 9a09637..30172ac 100755 --- a/test/app-tap/httpd.py +++ b/test/app-tap/httpd.py @@ -33,6 +33,7 @@ def headers(): ('Set-Cookie', 'age = 17; NOSuchOption; EmptyOption=Value;Secure'), ('my_header', 'value1'), ('my_header', 'value2'), + ('very_very_very_long_headers_name1', 'true'), ] return code, body, headers -- 2.7.4