From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Serge Petrenko Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_45342131-A1B3-4AC1-B813-A67C59CAC7DD" Mime-Version: 1.0 (Mac OS X Mail 11.5 \(3445.9.1\)) Subject: Re: [tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call Date: Mon, 17 Sep 2018 17:48:53 +0300 In-Reply-To: <20180914151752.f7f2nwa5dtpu334u@esperanza> References: <20180910133643.22656-1-sergepetrenko@tarantool.org> <20180914151752.f7f2nwa5dtpu334u@esperanza> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: --Apple-Mail=_45342131-A1B3-4AC1-B813-A67C59CAC7DD Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 > 14 =D1=81=D0=B5=D0=BD=D1=82. 2018 =D0=B3., =D0=B2 18:17, Vladimir = Davydov =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB= (=D0=B0): >=20 > On Mon, Sep 10, 2018 at 04:36:43PM +0300, Serge Petrenko wrote: >> Calling box.schema.user:passwd() instead of box.schema.user.passwd() >>=20 >=20 > This isn't a proper fix, because the following command still crashes: >=20 > box.schema.user.passwd(123) >=20 > And this one crashes too: >=20 > box.session.su('admin', function(x) return #x end, 123) >=20 > Making error messages user-friendly is good, but I think that #3659 is > about invalid usage of luaT_error in lbox_session_su... Hi! I found the cause of this and fixed the issue. I also removed the = type check since it seems to be outside of the patch scope now. New diff is below. --- src/box/lua/session.c | 5 ++++- test/box/misc.result | 9 +++++++++ test/box/misc.test.lua | 6 ++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/box/lua/session.c b/src/box/lua/session.c index b2e1400b6..7541da0a7 100644 --- a/src/box/lua/session.c +++ b/src/box/lua/session.c @@ -201,8 +201,11 @@ lbox_session_su(struct lua_State *L) /* Restore the original credentials. */ fiber_set_user(fiber(), old_credentials); =20 - if (error) + if (error) { + luaT_toerror(L); luaT_error(L); + } + return lua_gettop(L) - 1; } =20 diff --git a/test/box/misc.result b/test/box/misc.result index 62376754e..4ee4797d0 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -1148,6 +1148,15 @@ s =3D box.schema.space.create('test', = {user=3D"no_such_user"}) --- - error: User 'no_such_user' is not found ... +-- +-- gh-3659 assertion failure after an error in code called from +-- box.session.su() +-- +box.session.su("admin", function(x) return #x end, 3) +--- +- error: '[string "return box.session.su("admin", function(x) = re..."]:1: attempt to + get length of local ''x'' (a number value)' +... -- Too long WAL write warning (gh-2743). s =3D box.schema.space.create('test') --- diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua index d6815645e..ee81c7be1 100644 --- a/test/box/misc.test.lua +++ b/test/box/misc.test.lua @@ -320,6 +320,12 @@ s:drop() -- s =3D box.schema.space.create('test', {user=3D"no_such_user"}) =20 +-- +-- gh-3659 assertion failure after an error in code called from +-- box.session.su() +-- +box.session.su("admin", function(x) return #x end, 3) + -- Too long WAL write warning (gh-2743). s =3D box.schema.space.create('test') _ =3D s:create_index('pk') --=20 2.15.2 (Apple Git-101.1)= --Apple-Mail=_45342131-A1B3-4AC1-B813-A67C59CAC7DD Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8

14 =D1=81=D0=B5=D0=BD=D1=82= . 2018 =D0=B3., =D0=B2 18:17, Vladimir Davydov <vdavydov.dev@gmail.com> =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0= =B0=D0=BB(=D0=B0):

On Mon, Sep 10, 2018 at = 04:36:43PM +0300, Serge Petrenko wrote:
Calling= box.schema.user:passwd() instead of box.schema.user.passwd()

This isn't a = proper fix, because the following command still crashes:
box.schema.user.passwd(123)

And this one crashes too:

box.session.su('admin', function(x) return #x end, = 123)

Making error messages user-friendly is = good, but I think that #3659 is
about invalid usage of = luaT_error in lbox_session_su...

Hi! I found the cause of this and fixed the issue. I also = removed the type check since it
seems to be outside of the = patch scope now. New diff is below.

---
src/box/lua/session.c  | 5 ++++-
test/box/misc.result   | 9 +++++++++
test/box/misc.test.lua | 6 ++++++
3 = files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index b2e1400b6..7541da0a7 100644
--- = a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -201,8 +201,11 @@ lbox_session_su(struct lua_State *L)
= /* Restore the original credentials. */
= fiber_set_user(fiber(), old_credentials);

- if (error)
+ if = (error) {
+ luaT_toerror(L);
= luaT_error(L);
+ }
+
return lua_gettop(L) - 1;
}

diff --git = a/test/box/misc.result b/test/box/misc.result
index = 62376754e..4ee4797d0 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1148,6 +1148,15 = @@ s =3D box.schema.space.create('test', {user=3D"no_such_user"})
---
- error: User 'no_such_user' is = not found
...
+--
+-- = gh-3659 assertion failure after an error in code called from
+-- box.session.su()
+--
+box.session.su("admin", function(x) return #x end, 3)
+---
+- error: '[string "return = box.session.su("admin", function(x) re..."]:1: attempt to
+   get length of local ''x'' (a number = value)'
+...
-- Too long WAL write = warning (gh-2743).
s =3D = box.schema.space.create('test')
---
diff= --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index d6815645e..ee81c7be1 100644
--- = a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -320,6 +320,12 @@ s:drop()
--
s =3D box.schema.space.create('test', = {user=3D"no_such_user"})

+--
+-- gh-3659 assertion failure after an error in code called = from
+-- box.session.su()
+--
+box.session.su("admin", function(x) return #x end, 3)
+
-- Too long WAL write warning = (gh-2743).
s =3D box.schema.space.create('test')
_ =3D s:create_index('pk')
--
2.15.2 (Apple Git-101.1)= --Apple-Mail=_45342131-A1B3-4AC1-B813-A67C59CAC7DD--