Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[svacer] lua_pcall result is ignored in src/lua/serializer.c #9396

Closed
sergepetrenko opened this issue Nov 23, 2023 · 0 comments
Closed

[svacer] lua_pcall result is ignored in src/lua/serializer.c #9396

sergepetrenko opened this issue Nov 23, 2023 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@sergepetrenko
Copy link
Collaborator

Svacer link

Relevant code:

if (lua_pcall(L, 2, 1, 0) == 0 && !lua_isnil(L, -1)) {
if (!lua_isfunction(L, -1)) {
diag_set(LuajitError,
"invalid " LUAL_SERIALIZE " value");
return -1;
}
/* copy object itself */
lua_pushvalue(L, idx);
lua_pcall(L, 1, 1, 0);
/* replace obj with the unpacked value */
lua_replace(L, idx);
if (luaL_tofield(L, cfg, idx, field) < 0)
return -1;
} /* else ignore lua_gettable exceptions */

The result of lua_pcall on line 277 is ignored. But lua_pcall() pushes the error message on the stack in case there was an error. So if we leave the lua_pcall result unchecked we then serialize the error. Basically, if __serialize metamethod returns an error ,this error will be silently serialized.

This might've been changed accidentally quite a while ago: 5519276. In this commit a lua_call wrapped with try ... catch was replaced with a lua_pcall. So previously the error wasn't serialized, even though it was ignored as well.

It seems we shouldn't ignore the __serialize errors and report them to the user.

@sergepetrenko sergepetrenko added the bug Something isn't working label Nov 23, 2023
Buristan added a commit to Buristan/tarantool that referenced this issue Nov 27, 2023
Without checking the return value of lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes tarantool#9396
igormunkin pushed a commit to igormunkin/tarantool that referenced this issue Dec 5, 2023
Without checking the return value of lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes tarantool#9396

(cherry picked from commit 98474f7)
Buristan added a commit to Buristan/tarantool that referenced this issue Dec 7, 2023
Without checking the return value of lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes tarantool#9396

(cherry picked from commit 98474f7)
Buristan added a commit to Buristan/tarantool that referenced this issue Dec 7, 2023
Without checking the return value of `lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes tarantool#9396

(cherry picked from commit 98474f7)
Buristan added a commit to Buristan/tarantool that referenced this issue Dec 7, 2023
Without checking the return value of `lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes tarantool#9396

(cherry picked from commit 98474f7)

Since the behavior of the <app/fiber.lua> test is changed in 2.11 the
result file is updated here.
igormunkin pushed a commit that referenced this issue Dec 7, 2023
Without checking the return value of `lua_pcall()` in
`lua_field_inspect_ucdata()`, the error message itself is returned as a
serialized result. The result status of `lua_pcall()` is not ignored
now.

NO_DOC=bugfix

Closes #9396

(cherry picked from commit 98474f7)

Since the behavior of the <app/fiber.lua> test is changed in 2.11 the
result file is updated here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants