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 435CA2775E for ; Tue, 19 Feb 2019 05:49:42 -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 1-oDGMvVLz-o for ; Tue, 19 Feb 2019 05:49:42 -0500 (EST) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 B4A7626907 for ; Tue, 19 Feb 2019 05:49:41 -0500 (EST) Date: Tue, 19 Feb 2019 13:49:35 +0300 From: Roman Subject: [tarantool-patches] Re: [PATCH] httpc: add checking of headers in httpc:request Message-Id: <1550573375.20050.0@smtp.mail.ru> In-Reply-To: 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> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="=-ISaappKGA/3SHquv8cvp" 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 --=-ISaappKGA/3SHquv8cvp Content-Type: text/plain; charset=iso-8859-5; format=flowed Content-Transfer-Encoding: quoted-printable Hi! Thanks for review. On =C1=D1, =E4=D5=D2 16, 2019 at 12:17 AM, Vladislav Shpilevoy=20 wrote: > Hi! >=20 >>> 5. Out of 80 symbols. =7F=7F >>> 2. Still out of 80. >> 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-re= quest/src/lua/httpc.c#L180 >=20 > Lets count. Here you have 6 tabs - 6 * 8 =3D 48 symbols. Then you have=20 > 55 symbols of >=20 > "headers must be string or table with \"__tostring\""); >=20 > 48 + 55 =3D 103. >=20 > Also, the error string is not aligned by opening parenthesis. >=20 > In your email I see, that you have 4 symbols tab, what gives you 79=20 > symbols, but > tab width in this file should be 8. So probably you mistakenly set=20 > tab width to > 4 and the editor shows you < 80 width. Please, fix. >=20 Ok. Done. commit c9802592b27cb6986e3e72eb948d8dbfb21baa7e Author: Roman Khabibov Date: Wed Dec 26 17:49:34 2018 +0300 httpc: add checking of headers in httpc:request Add preprocessing of the request headers. Each header must be=20 'string' or 'table' with '__tostring' metamethod. Closes #3679 diff --git a/src/lua/httpc.c b/src/lua/httpc.c index 5f4e2e912..fb012f58b 100644 --- a/src/lua/httpc.c +++ b/src/lua/httpc.c @@ -173,6 +173,18 @@ 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) { + 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=20 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 +-- 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 function run_tests(test, sock_family, sock_addr) - test:plan(9) + test:plan(10) local server, url, opts =3D start_server(test, sock_family,=20 sock_addr) test:test("http.client", test_http_client, url, opts) test:test("cancel and errinj", test_cancel_and_errinj, url ..=20 '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 = --=-ISaappKGA/3SHquv8cvp Content-Type: text/html; charset=iso-8859-5 Content-Transfer-Encoding: quoted-printable

Hi! Tha= nks for review.

On =C1=D1, =E4=D5=D2 16, 2019 at 12:17 AM, Vlad= islav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
Hi!
5. Out of 80 symbols. =7F=7F 2. Still out of 80.
My text editor shows 70 symbols.
Then there is something wrong with your editor, sorry. https://github.com/tarantool/tarantool= /blob/romanhabibov/gh-3679-httpc-request/src/lua/httpc.c#L180 Lets count. Here you have 6 tabs - 6 * 8 =3D 48 symbols. Then you have 55 s= ymbols 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 widt= h to 4 and the editor shows you < 80 width. Please, fix.
Ok. Done.

commit= c9802592b27cb6986e3e72eb948d8dbfb21baa7e
Author: Roman Khabibov <roman.habibov@tarantool.o= rg>
Date: Wed = Dec 26 17:49:34 2018 +0300

ht= tpc: add checking of headers in httpc:request
Add preprocessing of the request headers. Each header mus= t be 'string' or 'table'
with '__tostring' metamethod.
Closes #3679

diff --gi= t a/src/lua/httpc.c b/src/lua/httpc.c
index 5f4e2e912..fb012f58b 100644
--- a/src/lua/httpc.c
+++ b/src/lua/httpc.c
@@ -173,6 +173,18 @@ luaT_httpc_req= uest(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) {
+ ch= ar *err_msg =3D "headers must be " \
+ "string or table with \"__tostring\"";
<= div>+ if (header_type !=3D LUA_TT= ABLE) {
+ retur= n luaL_error(L, err_msg);
+ } else if (!luaL_getmetafield(L, -1,
+ "__tostring")) {
<= div>+ return luaL_error(L, err_m= sg);
+ }<= /div>
+ lua_pop(L, 1);=
+ }
if (httpc_set_header(req, "%s: %s"= ,
lua_to= string(L, -2),
= lua_tostring(L, -1)) < 0) {
diff --git a/test/app-tap/http_client.test.lua b/test/ap= p-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
+-- gh-3679 allow only headers can be conver= ted to string
+loca= l function test_request_headers(test, url, opts)
+ local exp_err =3D 'headers must be strin= g 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({}, {
+ __tost= ring =3D function(self)
+ return 'aaa'
+ end})}},
+ exp_err =3D nil,
= + postrequest_check =3D f= unction(opts)
+ = assert(type(opts.headers.aaa) =3D=3D 'table',
= + '"aaa" header w= as modified in http_client')
+ end,
+ },
+ = 'boolean header',
+ opts =3D {headers =3D {aaa =3D true}},
= + exp_err =3D exp_err,
+ },
+ {
= + 'number header',=
+ opts =3D {he= aders =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<uint64_t> header',
+ opts =3D {headers =3D {aaa =3D 10ULL}},=
+ exp_err =3D = exp_err,
+ }= ,
+ {=
+ 'table heade= r w/o metatable',
+= opts =3D {headers =3D {aaa =3D {}}},
+ exp_err =3D exp_err,
+ },
<= span style=3D"white-space: pre-wrap;">+ {
+ 'table header w/o __tostring() m= etamethod',
+ = opts =3D {headers =3D {aaa =3D setmetatable({}, {})}},
+ exp_err =3D exp_err,=
+ },=
+ }
<= span style=3D"white-space: pre-wrap;">+ 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._]+.lu= a:[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 fun= ction test_concurrent(test, url, opts)
end
function r= un_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", tes= t_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_spe= cial_methods, url, opts)
if sock_family =3D=3D 'AF_UNIX' and jit.os ~=3D "Linux" then

= --=-ISaappKGA/3SHquv8cvp--