[tarantool-patches] [http.client 2/2] http: Fix parse long headers names

[Ilya Markov <imarkov@tarantool.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