[Tarantool-patches] [PATCH] serializer: check for recursive serialization
Roman Khabibov
roman.habibov at tarantool.org
Wed Dec 2 03:53:52 MSK 2020
Thanks for the LGTM.
SergOs, could you, please, look through the patch?
> On Nov 23, 2020, at 23:28, Igor Munkin <imun at 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 at 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})
More information about the Tarantool-patches
mailing list