[tarantool-patches] [http.client 2/2] http: Fix parse long headers names
Ilya Markov
imarkov at tarantool.org
Fri Jun 15 17:24:11 MSK 2018
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 at 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
More information about the Tarantool-patches
mailing list