* [Tarantool-patches] [PATCH] serializer: check for recursive serialization @ 2020-11-17 16:40 Roman Khabibov 2020-11-23 20:28 ` Igor Munkin 0 siblings, 1 reply; 7+ messages in thread From: Roman Khabibov @ 2020-11-17 16:40 UTC (permalink / raw) To: tarantool-patches; +Cc: alexander.turenko Print error if object after serialization is the same. Closes #3228 --- Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/serialize-check Issue: https://github.com/tarantool/tarantool/issues/3228 @ChangeLog: * Fix bug with bus error when __serialize function generates infinite recursion (gh-3228). src/lua/utils.c | 5 +++++ ...-3228-serializer-look-for-recursion.result | 19 +++++++++++++++++++ ...228-serializer-look-for-recursion.test.lua | 8 ++++++++ 3 files changed, 32 insertions(+) create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua diff --git a/src/lua/utils.c b/src/lua/utils.c index 23fbdd4ad..d12f3675a 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -508,6 +508,11 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, diag_set(LuajitError, lua_tostring(L, -1)); return -1; } + if (lua_rawequal(L, -2, -1) == 1) { + diag_set(LuajitError, "Bad __serialize function. It " + "can't return the same value."); + return -1; + } if (luaL_tofield(L, cfg, NULL, -1, field) != 0) return -1; lua_replace(L, idx); diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result new file mode 100644 index 000000000..cd86ab06a --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.result @@ -0,0 +1,19 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua new file mode 100644 index 000000000..d3c76ef0c --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua @@ -0,0 +1,8 @@ +test_run = require('test_run').new() + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) -- 2.24.3 (Apple Git-128) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-11-17 16:40 [Tarantool-patches] [PATCH] serializer: check for recursive serialization Roman Khabibov @ 2020-11-23 20:28 ` Igor Munkin 2020-11-24 1:51 ` roman 2020-12-02 0:53 ` Roman Khabibov 0 siblings, 2 replies; 7+ messages in thread From: Igor Munkin @ 2020-11-23 20:28 UTC (permalink / raw) To: Roman Khabibov; +Cc: tarantool-patches, alexander.turenko Roma, Thanks for the patch! This version looks much better than the previous one, but I still have a couple of nits. Otherwise LGTM. On 17.11.20, Roman Khabibov wrote: > Print error if object after serialization is the same. I believe we need a doc request to update __serialize description, since its behaviour is restricted with the introduced constraint now. > > Closes #3228 > --- > > Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/serialize-check > Issue: https://github.com/tarantool/tarantool/issues/3228 > > @ChangeLog: > * Fix bug with bus error when __serialize function generates infinite recursion (gh-3228). > > src/lua/utils.c | 5 +++++ > ...-3228-serializer-look-for-recursion.result | 19 +++++++++++++++++++ > ...228-serializer-look-for-recursion.test.lua | 8 ++++++++ > 3 files changed, 32 insertions(+) > create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result > create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua > <snipped> > diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result > new file mode 100644 > index 000000000..cd86ab06a > --- /dev/null > +++ b/test/app/gh-3228-serializer-look-for-recursion.result > @@ -0,0 +1,19 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > + > +-- > +-- gh-3228: Check the error message in the case of a __serialize > +-- function generating infinite recursion. > +-- > +setmetatable({}, {__serialize = function(a) return a end}) > + | --- > + | - error: 'console: an exception occurred when formatting the output: Bad __serialize > + | function. It can''t return the same value.' > + | ... > +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) > + | --- > + | - error: 'console: an exception occurred when formatting the output: Bad __serialize > + | function. It can''t return the same value.' Hm, AFAICS the custom serializer accepts a single argument (i.e. "self") and a single return value is expected (considering the code you were around to). Hence, the latter check is the same as the first one and checks literally nothing. By the way, I guess it's worth to check that __eq metamethod is ignored when the object itself is compared with its "serialized" value. Just to be sure it won't be broken unintentionally in future. > + | ... <snipped> > -- > 2.24.3 (Apple Git-128) > -- Best regards, IM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-11-23 20:28 ` Igor Munkin @ 2020-11-24 1:51 ` roman 2020-12-02 0:53 ` Roman Khabibov 1 sibling, 0 replies; 7+ messages in thread From: roman @ 2020-11-24 1:51 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches, alexander.turenko Hi! Thanks for the review. On 23.11.2020 23:28, Igor Munkin wrote: > Roma, > > Thanks for the patch! This version looks much better than the previous one, > but I still have a couple of nits. Otherwise LGTM. > > On 17.11.20, Roman Khabibov wrote: >> Print error if object after serialization is the same. > I believe we need a doc request to update __serialize description, since > its behaviour is restricted with the introduced constraint now. Done. >> Closes #3228 >> --- >> >> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/serialize-check >> Issue: https://github.com/tarantool/tarantool/issues/3228 >> >> @ChangeLog: >> * Fix bug with bus error when __serialize function generates infinite recursion (gh-3228). >> >> src/lua/utils.c | 5 +++++ >> ...-3228-serializer-look-for-recursion.result | 19 +++++++++++++++++++ >> ...228-serializer-look-for-recursion.test.lua | 8 ++++++++ >> 3 files changed, 32 insertions(+) >> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result >> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua >> > <snipped> > >> diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result >> new file mode 100644 >> index 000000000..cd86ab06a >> --- /dev/null >> +++ b/test/app/gh-3228-serializer-look-for-recursion.result >> @@ -0,0 +1,19 @@ >> +-- test-run result file version 2 >> +test_run = require('test_run').new() >> + | --- >> + | ... >> + >> +-- >> +-- gh-3228: Check the error message in the case of a __serialize >> +-- function generating infinite recursion. >> +-- >> +setmetatable({}, {__serialize = function(a) return a end}) >> + | --- >> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >> + | function. It can''t return the same value.' >> + | ... >> +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) >> + | --- >> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >> + | function. It can''t return the same value.' > Hm, AFAICS the custom serializer accepts a single argument (i.e. "self") > and a single return value is expected (considering the code you were > around to). Hence, the latter check is the same as the first one and > checks literally nothing. By the way, I guess it's worth to check that > __eq metamethod is ignored when the object itself is compared with its > "serialized" value. Just to be sure it won't be broken unintentionally > in future. Added. >> + | ... > <snipped> > >> -- >> 2.24.3 (Apple Git-128) >> commit a40f5623b31e547cc0c273f2224484a591002e31 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Thu Oct 8 18:22:24 2020 +0300 serializer: check for recursive serialization Print error if object after serialization is the same. Closes #3228 @TarantoolBot documnet Title: __serialize parameter If __serialize parameter is function, then this function can't return the value passed to it. Such functions generates recursions, so this is forbidden. Example: ``` tarantool> setmetatable({},{__serialize = function(_) return _ end}) --- - error: 'console: an exception occurred when formatting the output: Bad __serialize function. It can''t return the same value.' ... ``` diff --git a/src/lua/utils.c b/src/lua/utils.c index 23fbdd4..d12f367 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -508,6 +508,11 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, diag_set(LuajitError, lua_tostring(L, -1)); return -1; } + if (lua_rawequal(L, -2, -1) == 1) { + diag_set(LuajitError, "Bad __serialize function. It " + "can't return the same value."); + return -1; + } if (luaL_tofield(L, cfg, NULL, -1, field) != 0) return -1; lua_replace(L, idx); diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result new file mode 100644 index 0000000..aaa3a4f --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.result @@ -0,0 +1,26 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... + +-- +--Check that __eq metamethod is ignored. +-- +local table = setmetatable({}, {__eq = function(a, b) return a ~= b end}) + | --- + | ... +setmetatable(table, {__serialize = function(a) return a end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua new file mode 100644 index 0000000..2f757af --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua @@ -0,0 +1,13 @@ +test_run = require('test_run').new() + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) + +-- +--Check that __eq metamethod is ignored. +-- +local table = setmetatable({}, {__eq = function(a, b) return a ~= b end}) +setmetatable(table, {__serialize = function(a) return a end}) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-11-23 20:28 ` Igor Munkin 2020-11-24 1:51 ` roman @ 2020-12-02 0:53 ` Roman Khabibov 2020-12-08 16:59 ` Sergey Ostanevich 1 sibling, 1 reply; 7+ messages in thread From: Roman Khabibov @ 2020-12-02 0:53 UTC (permalink / raw) To: Sergey Ostanevich; +Cc: tarantool-patches Thanks for the LGTM. SergOs, could you, please, look through the patch? > On Nov 23, 2020, at 23:28, Igor Munkin <imun@tarantool.org> wrote: > > Roma, > > Thanks for the patch! This version looks much better than the previous one, > but I still have a couple of nits. Otherwise LGTM. > > On 17.11.20, Roman Khabibov wrote: >> Print error if object after serialization is the same. > > I believe we need a doc request to update __serialize description, since > its behaviour is restricted with the introduced constraint now. > >> >> Closes #3228 >> --- >> >> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/serialize-check >> Issue: https://github.com/tarantool/tarantool/issues/3228 >> >> @ChangeLog: >> * Fix bug with bus error when __serialize function generates infinite recursion (gh-3228). >> >> src/lua/utils.c | 5 +++++ >> ...-3228-serializer-look-for-recursion.result | 19 +++++++++++++++++++ >> ...228-serializer-look-for-recursion.test.lua | 8 ++++++++ >> 3 files changed, 32 insertions(+) >> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result >> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua >> > > <snipped> > >> diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result >> new file mode 100644 >> index 000000000..cd86ab06a >> --- /dev/null >> +++ b/test/app/gh-3228-serializer-look-for-recursion.result >> @@ -0,0 +1,19 @@ >> +-- test-run result file version 2 >> +test_run = require('test_run').new() >> + | --- >> + | ... >> + >> +-- >> +-- gh-3228: Check the error message in the case of a __serialize >> +-- function generating infinite recursion. >> +-- >> +setmetatable({}, {__serialize = function(a) return a end}) >> + | --- >> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >> + | function. It can''t return the same value.' >> + | ... >> +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) >> + | --- >> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >> + | function. It can''t return the same value.' > > Hm, AFAICS the custom serializer accepts a single argument (i.e. "self") > and a single return value is expected (considering the code you were > around to). Hence, the latter check is the same as the first one and > checks literally nothing. By the way, I guess it's worth to check that > __eq metamethod is ignored when the object itself is compared with its > "serialized" value. Just to be sure it won't be broken unintentionally > in future. > >> + | ... > > <snipped> > >> -- >> 2.24.3 (Apple Git-128) >> > > -- > Best regards, > IM commit 0eebee84ac425fc028f07920352ad2f9ec8be1e1 (HEAD -> romanhabibov/serialize-check, origin/romanhabibov/serialize-check) Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Thu Oct 8 18:22:24 2020 +0300 serializer: check for recursive serialization Print error if object after serialization is the same. Closes #3228 @TarantoolBot documnet Title: __serialize parameter If __serialize parameter is function, then this function can't return the value passed to it. Such functions generates recursions, so this is forbidden. Example: ``` tarantool> setmetatable({},{__serialize = function(_) return _ end}) --- - error: 'console: an exception occurred when formatting the output: Bad __serialize function. It can''t return the same value.' ... ``` diff --git a/src/lua/utils.c b/src/lua/utils.c index 23fbdd4ad..d12f3675a 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -508,6 +508,11 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, diag_set(LuajitError, lua_tostring(L, -1)); return -1; } + if (lua_rawequal(L, -2, -1) == 1) { + diag_set(LuajitError, "Bad __serialize function. It " + "can't return the same value."); + return -1; + } if (luaL_tofield(L, cfg, NULL, -1, field) != 0) return -1; lua_replace(L, idx); diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result new file mode 100644 index 000000000..e55c2796b --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.result @@ -0,0 +1,26 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... + +-- +--Check that __eq metamethod is ignored. +-- +local table = setmetatable({}, {__eq = function(a, b) error('__eq is called') end}) + | --- + | ... +setmetatable(table, {__serialize = function(a) return a end}) + | --- + | - error: 'console: an exception occurred when formatting the output: Bad __serialize + | function. It can''t return the same value.' + | ... diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua new file mode 100644 index 000000000..01268f026 --- /dev/null +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua @@ -0,0 +1,13 @@ +test_run = require('test_run').new() + +-- +-- gh-3228: Check the error message in the case of a __serialize +-- function generating infinite recursion. +-- +setmetatable({}, {__serialize = function(a) return a end}) + +-- +--Check that __eq metamethod is ignored. +-- +local table = setmetatable({}, {__eq = function(a, b) error('__eq is called') end}) +setmetatable(table, {__serialize = function(a) return a end}) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-12-02 0:53 ` Roman Khabibov @ 2020-12-08 16:59 ` Sergey Ostanevich 2020-12-08 17:25 ` Igor Munkin 0 siblings, 1 reply; 7+ messages in thread From: Sergey Ostanevich @ 2020-12-08 16:59 UTC (permalink / raw) To: Roman Khabibov; +Cc: tarantool-patches Hi! Thanks for the patch! My biggest concern was how would you check the recursion appears. You just check if the result is equivalent to the argument. To me it is not enough, obviously. I tried this on your branch and… tarantool> setmetatable({},{__serialize = function(_) return {_} end}) Segmentation fault (core dumped) Regards, Sergos > On 2 Dec 2020, at 03:53, Roman Khabibov <roman.habibov@tarantool.org> wrote: > > Thanks for the LGTM. > > SergOs, could you, please, look through the patch? > >> On Nov 23, 2020, at 23:28, Igor Munkin <imun@tarantool.org> wrote: >> >> Roma, >> >> Thanks for the patch! This version looks much better than the previous one, >> but I still have a couple of nits. Otherwise LGTM. >> >> On 17.11.20, Roman Khabibov wrote: >>> Print error if object after serialization is the same. >> >> I believe we need a doc request to update __serialize description, since >> its behaviour is restricted with the introduced constraint now. >> >>> >>> Closes #3228 >>> --- >>> >>> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/serialize-check >>> Issue: https://github.com/tarantool/tarantool/issues/3228 >>> >>> @ChangeLog: >>> * Fix bug with bus error when __serialize function generates infinite recursion (gh-3228). >>> >>> src/lua/utils.c | 5 +++++ >>> ...-3228-serializer-look-for-recursion.result | 19 +++++++++++++++++++ >>> ...228-serializer-look-for-recursion.test.lua | 8 ++++++++ >>> 3 files changed, 32 insertions(+) >>> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result >>> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua >>> >> >> <snipped> >> >>> diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result >>> new file mode 100644 >>> index 000000000..cd86ab06a >>> --- /dev/null >>> +++ b/test/app/gh-3228-serializer-look-for-recursion.result >>> @@ -0,0 +1,19 @@ >>> +-- test-run result file version 2 >>> +test_run = require('test_run').new() >>> + | --- >>> + | ... >>> + >>> +-- >>> +-- gh-3228: Check the error message in the case of a __serialize >>> +-- function generating infinite recursion. >>> +-- >>> +setmetatable({}, {__serialize = function(a) return a end}) >>> + | --- >>> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >>> + | function. It can''t return the same value.' >>> + | ... >>> +setmetatable({}, {__serialize = function(a, b, c) return a, b, c end}) >>> + | --- >>> + | - error: 'console: an exception occurred when formatting the output: Bad __serialize >>> + | function. It can''t return the same value.' >> >> Hm, AFAICS the custom serializer accepts a single argument (i.e. "self") >> and a single return value is expected (considering the code you were >> around to). Hence, the latter check is the same as the first one and >> checks literally nothing. By the way, I guess it's worth to check that >> __eq metamethod is ignored when the object itself is compared with its >> "serialized" value. Just to be sure it won't be broken unintentionally >> in future. >> >>> + | ... >> >> <snipped> >> >>> -- >>> 2.24.3 (Apple Git-128) >>> >> >> -- >> Best regards, >> IM > > commit 0eebee84ac425fc028f07920352ad2f9ec8be1e1 (HEAD -> romanhabibov/serialize-check, origin/romanhabibov/serialize-check) > Author: Roman Khabibov <roman.habibov@tarantool.org> > Date: Thu Oct 8 18:22:24 2020 +0300 > > serializer: check for recursive serialization > > Print error if object after serialization is the same. > > Closes #3228 > > @TarantoolBot documnet > Title: __serialize parameter > If __serialize parameter is function, then this function > can't return the value passed to it. Such functions > generates recursions, so this is forbidden. > > Example: > ``` > tarantool> setmetatable({},{__serialize = function(_) return _ end}) > --- > - error: 'console: an exception occurred when formatting the output: Bad __serialize > function. It can''t return the same value.' > ... > ``` > > diff --git a/src/lua/utils.c b/src/lua/utils.c > index 23fbdd4ad..d12f3675a 100644 > --- a/src/lua/utils.c > +++ b/src/lua/utils.c > @@ -508,6 +508,11 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg, > diag_set(LuajitError, lua_tostring(L, -1)); > return -1; > } > + if (lua_rawequal(L, -2, -1) == 1) { > + diag_set(LuajitError, "Bad __serialize function. It " > + "can't return the same value."); > + return -1; > + } > if (luaL_tofield(L, cfg, NULL, -1, field) != 0) > return -1; > lua_replace(L, idx); > diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result > new file mode 100644 > index 000000000..e55c2796b > --- /dev/null > +++ b/test/app/gh-3228-serializer-look-for-recursion.result > @@ -0,0 +1,26 @@ > +-- test-run result file version 2 > +test_run = require('test_run').new() > + | --- > + | ... > + > +-- > +-- gh-3228: Check the error message in the case of a __serialize > +-- function generating infinite recursion. > +-- > +setmetatable({}, {__serialize = function(a) return a end}) > + | --- > + | - error: 'console: an exception occurred when formatting the output: Bad __serialize > + | function. It can''t return the same value.' > + | ... > + > +-- > +--Check that __eq metamethod is ignored. > +-- > +local table = setmetatable({}, {__eq = function(a, b) error('__eq is called') end}) > + | --- > + | ... > +setmetatable(table, {__serialize = function(a) return a end}) > + | --- > + | - error: 'console: an exception occurred when formatting the output: Bad __serialize > + | function. It can''t return the same value.' > + | ... > diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua > new file mode 100644 > index 000000000..01268f026 > --- /dev/null > +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua > @@ -0,0 +1,13 @@ > +test_run = require('test_run').new() > + > +-- > +-- gh-3228: Check the error message in the case of a __serialize > +-- function generating infinite recursion. > +-- > +setmetatable({}, {__serialize = function(a) return a end}) > + > +-- > +--Check that __eq metamethod is ignored. > +-- > +local table = setmetatable({}, {__eq = function(a, b) error('__eq is called') end}) > +setmetatable(table, {__serialize = function(a) return a end}) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-12-08 16:59 ` Sergey Ostanevich @ 2020-12-08 17:25 ` Igor Munkin 2020-12-11 3:22 ` Roman Khabibov 0 siblings, 1 reply; 7+ messages in thread From: Igor Munkin @ 2020-12-08 17:25 UTC (permalink / raw) To: Sergey Ostanevich; +Cc: tarantool-patches Sergos, On 08.12.20, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch! > > My biggest concern was how would you check the recursion appears. You Well, I see the issue is reported for "identity" serializer (considering documentation request and the error message), and such recursion can be fixed the way Roma did. You're talking about a general case of infinite recursions, but I believe there is no other approach to handle this situation the way different from one Python does (I mentioned it in my previous review[1]). By the way, other runtimes such as Perl starts spamming with the corresponding warning after the recursion hits the "soft" limit, but the result is the same: stack overflow. Thoughts? > just check if the result is equivalent to the argument. To me it is > not enough, obviously. I tried this on your branch and… > > > tarantool> setmetatable({},{__serialize = function(_) return {_} end}) > Segmentation fault (core dumped) > > > Regards, > Sergos > <snipped> > [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018524.html -- Best regards, IM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH] serializer: check for recursive serialization 2020-12-08 17:25 ` Igor Munkin @ 2020-12-11 3:22 ` Roman Khabibov 0 siblings, 0 replies; 7+ messages in thread From: Roman Khabibov @ 2020-12-11 3:22 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches Hi! > On Dec 8, 2020, at 22:25, Igor Munkin <imun@tarantool.org> wrote: > > Sergos, > > On 08.12.20, Sergey Ostanevich wrote: >> Hi! >> >> Thanks for the patch! >> >> My biggest concern was how would you check the recursion appears. You > > Well, I see the issue is reported for "identity" serializer (considering > documentation request and the error message), and such recursion can be > fixed the way Roma did. You're talking about a general case of infinite > recursions, but I believe there is no other approach to handle this > situation the way different from one Python does (I mentioned it in my > previous review[1]). By the way, other runtimes such as Perl starts > spamming with the corresponding warning after the recursion hits the > "soft" limit, but the result is the same: stack overflow. Thoughts? Let’s just add ‘hard’ limit? Only it is not clear how to calculate its value. Just empirically? >> just check if the result is equivalent to the argument. To me it is >> not enough, obviously. I tried this on your branch and… >> >> >> tarantool> setmetatable({},{__serialize = function(_) return {_} end}) >> Segmentation fault (core dumped) >> >> >> Regards, >> Sergos >> > > <snipped> > >> > > [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018524.html > > -- > Best regards, > IM ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-11 3:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-17 16:40 [Tarantool-patches] [PATCH] serializer: check for recursive serialization Roman Khabibov 2020-11-23 20:28 ` Igor Munkin 2020-11-24 1:51 ` roman 2020-12-02 0:53 ` Roman Khabibov 2020-12-08 16:59 ` Sergey Ostanevich 2020-12-08 17:25 ` Igor Munkin 2020-12-11 3:22 ` Roman Khabibov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox