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 8FA8C262C2 for ; Sat, 23 Feb 2019 17:43:39 -0500 (EST) 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 xaPuUfu1D1f5 for ; Sat, 23 Feb 2019 17:43:39 -0500 (EST) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 AA6ED216C6 for ; Sat, 23 Feb 2019 17:43:38 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] httpc: add checking of headers in httpc:request From: Roman Khabibov In-Reply-To: Date: Sun, 24 Feb 2019 01:43:36 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20181221181844.4417-1-roman.habibov@tarantool.org> <20181223031929.hhmxdlsox2aon5ih@tkn_work_nb> <3b2626ce-b887-70dd-38b0-ef38b37a983c@tarantool.org> <72bbdfde-54d7-0e94-c5e7-ca84c2a6078d@tarantool.org> <7f31300f-2e4a-f0b1-a40c-90d631fb9778@tarantool.org> <2e5f83a6-2885-96f8-6fd6-a5fd34e32644@tarantool.org> <1549927450.14514.0@smtp.mail.ru> <1550573375.20050.0@smtp.mail.ru> 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: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy Hi! Thanks for review.=20 On 2/22/19 7:01 PM, Vladislav Shpilevoy wrote:=20 > Hi! Thanks for the fixes! See 3 comments below.=20 > On 19/02/2019 13:49, Roman wrote:=20 >=20 >> Hi! Thanks for review.=20 >> On =D0=A1=D0=B1, =D1=84=D0=B5=D0=B2 16, 2019 at 12:17 AM, Vladislav = Shpilevoy=20 >> >> wrote:=20 >>=20 >>> Hi!=20 >>> 5. Out of 80 symbols. =7F=7F 2. Still out of 80.=20 >>>=20 >>> My text editor shows 70 symbols.=20 >>>=20 >>> Then there is something wrong with your editor, sorry.=20 >>> = https://github.com/tarantool/tarantool/blob/romanhabibov/gh-3679-httpc-req= uest/src/lua/httpc.c#L180 = >>> Lets count. Here you have 6 tabs - 6 * 8 =3D 48 symbols. Then you = have 55 symbols of "headers must be string or table with = \"__tostring\""); 48 + 55 =3D 103. Also, the error string is not aligned = by opening parenthesis. In your email I see, that you have 4 symbols = tab, what gives you 79 symbols, but tab width in this file should be 8. = So probably you mistakenly set tab width to 4 and the editor shows you < = 80 width. Please, fix.=20 >>>=20 >> Ok. Done.=20 >>=20 > 1. Now you have leading whitespace on line 182. It is clearly visible = in=20 > git diff as a bright red point. Please, do self-review before sending=20= > a patch.=20 >=20 I'm sorry to waste your time. I was in a hurry and did't notice.=20 >> commit c9802592b27cb6986e3e72eb948d8dbfb21baa7e=20 >> Author: Roman Khabibov=20 >> >> =20 >> Date: Wed Dec 26 17:49:34 2018 +0300=20 >> httpc: add checking of headers in httpc:request=20 >> Add preprocessing of the request headers. Each header must be = 'string' or 'table'=20 >> with '__tostring' metamethod.=20 >> Closes #3679=20 >> diff --git a/src/lua/httpc.c b/src/lua/httpc.c=20 >> index 5f4e2e912..fb012f58b 100644=20 >> --- a/src/lua/httpc.c=20 >> +++ b/src/lua/httpc.c=20 >> @@ -173,6 +173,18 @@ luaT_httpc_request(lua_State *L)=20 >> if (!lua_isnil(L, -1)) {=20 >> lua_pushnil(L);=20 >> while (lua_next(L, -2) !=3D 0) {=20 >> + int header_type =3D lua_type(L, -1);=20 >> + if (header_type !=3D LUA_TSTRING) {=20 >> + char *err_msg =3D "headers must be " \=20 >> + "string or table with \"__tostring\"";=20 >>=20 > 2. Please, do not assign a const char string to=20 > a 'char *' variable. Use 'const char *'. Also,=20 > please, properly align strings. This part should=20 > look like this:=20 > if (header_type !=3D LUA_TSTRING) {=20 > const char *err_msg =3D=20 > "headers must be string or table with "\=20 > "\"__tostring\"";=20 > if (header_type !=3D LUA_TTABLE) {=20 > Wrapped string is aligned by the beginning.=20 >=20 >> + if (header_type !=3D LUA_TTABLE) {=20 >> + return luaL_error(L, err_msg);=20 >> + } else if (!luaL_getmetafield(L, -1,=20 >> + "__tostring")) {=20 >> + return luaL_error(L, err_msg);=20 >> + }=20 >> + lua_pop(L, 1);=20 >> + }=20 >>=20 Done. commit 42d25f4d6eef69ae56db3841863afa3373fa9b67 Author: Roman Khabibov Date: Wed Dec 26 17:49:34 2018 +0300 httpc: add checking of headers in httpc:request =20 Add preprocessing of the request headers. Each header must be = 'string' or 'table' with '__tostring' metamethod. =20 Closes #3679 diff --git a/src/lua/httpc.c b/src/lua/httpc.c index 5f4e2e912..db8634949 100644 --- a/src/lua/httpc.c +++ b/src/lua/httpc.c @@ -173,6 +173,19 @@ luaT_httpc_request(lua_State *L) if (!lua_isnil(L, -1)) { lua_pushnil(L); while (lua_next(L, -2) !=3D 0) { + int header_type =3D lua_type(L, -1); + if (header_type !=3D LUA_TSTRING) { + const char *err_msg =3D + "headers must be string or table = "\ + "with \"__tostring\""; + if (header_type !=3D LUA_TTABLE) { + return luaL_error(L, err_msg); + } else if (!luaL_getmetafield(L, -1, + = "__tostring")) { + return luaL_error(L, err_msg); + } + lua_pop(L, 1); + } if (httpc_set_header(req, "%s: %s", lua_tostring(L, -2), lua_tostring(L, -1)) < 0) { diff --git a/test/app-tap/http_client.test.lua = b/test/app-tap/http_client.test.lua index 10a3728f8..8a98d1e92 100755 --- a/test/app-tap/http_client.test.lua +++ b/test/app-tap/http_client.test.lua @@ -205,6 +205,80 @@ local function test_errors(test) test:is(r.status, 595, "GET: response on bad url") end =20 +-- gh-3679 allow only headers can be converted to string +local function test_request_headers(test, url, opts) + local exp_err =3D 'headers must be string or table with = "__tostring"' + local cases =3D { + { + 'string header', + opts =3D {headers =3D {aaa =3D 'aaa'}}, + exp_err =3D nil, + }, + { + 'header with __tostring() metamethod', + opts =3D {headers =3D {aaa =3D setmetatable({}, { + __tostring =3D function(self) + return 'aaa' + end})}}, + exp_err =3D nil, + postrequest_check =3D function(opts) + assert(type(opts.headers.aaa) =3D=3D 'table', + '"aaa" header was modified in http_client') + end, + }, + { + 'boolean header', + opts =3D {headers =3D {aaa =3D true}}, + exp_err =3D exp_err, + }, + { + 'number header', + opts =3D {headers =3D {aaa =3D 10}}, + exp_err =3D exp_err, + }, + { + 'cdata header (box.NULL)', + opts =3D {headers =3D {aaa =3D box.NULL}}, + exp_err =3D exp_err, + }, + { + 'cdata header', + opts =3D {headers =3D {aaa =3D 10ULL}}, + exp_err =3D exp_err, + }, + { + 'table header w/o metatable', + opts =3D {headers =3D {aaa =3D {}}}, + exp_err =3D exp_err, + }, + { + 'table header w/o __tostring() metamethod', + opts =3D {headers =3D {aaa =3D setmetatable({}, {})}}, + exp_err =3D exp_err, + }, + } + test:plan(#cases) + + local http =3D client:new() + + for _, case in ipairs(cases) do + local opts =3D merge(table.copy(opts), case.opts) + local ok, err =3D pcall(http.get, http, url, opts) + if case.postrequest_check ~=3D nil then + case.postrequest_check(opts) + end + if case.exp_err =3D=3D nil then + -- expect success + test:ok(ok, case[1]) + else + -- expect fail + assert(type(err) =3D=3D 'string') + err =3D err:gsub('^builtin/[a-z._]+.lua:[0-9]+: ', '') + test:is_deeply({ok, err}, {false, case.exp_err}, case[1]) + end + end +end + local function test_headers(test, url, opts) test:plan(19) local http =3D client:new() @@ -397,12 +471,13 @@ local function test_concurrent(test, url, opts) end =20 function run_tests(test, sock_family, sock_addr) - test:plan(9) + test:plan(10) local server, url, opts =3D start_server(test, sock_family, = sock_addr) test:test("http.client", test_http_client, url, opts) test:test("cancel and errinj", test_cancel_and_errinj, url .. = 'long_query', opts) test:test("basic http post/get", test_post_and_get, url, opts) test:test("errors", test_errors) + test:test("request_headers", test_request_headers, url, opts) test:test("headers", test_headers, url, opts) test:test("special methods", test_special_methods, url, opts) if sock_family =3D=3D 'AF_UNIX' and jit.os ~=3D "Linux" then