Changed strlen to lua_strlen for producers key and value #22
Conversation
kafka/producer.c
Outdated
| lua_pushstring(L, "key"); | ||
| lua_gettable(L, -2 ); | ||
| // rd_kafka will copy key so no need to worry about this cast | ||
| char *key = (char *)lua_tostring(L, -1); |
There was a problem hiding this comment.
But why didn't you simply use lua_tolstring that accepts an additional argument where string will be returned.
| size_t key_len = key != NULL ? lua_strlen(L, -1) : 0; | ||
| char *key = (char *)lua_tolstring(L, -1, &key_len); | ||
|
|
||
| lua_pop(L, 1); |
There was a problem hiding this comment.
Well, as we discussed in Tarantool-ru telegram chat, this approach could lead to use-after-free (https://pgl.yoyo.org/luai/i/lua_tolstring)
Maybe we should use luaL_ref/luaL_unref here.
Also it would be great to add a test for such case.
There was a problem hiding this comment.
The problem of use after free was present before (and I suppose this is another issue), this PR just replace strlen to lua_tolstring
There was a problem hiding this comment.
Well, you are right. I don't have objections.
There was a problem hiding this comment.
this approach could lead to use-after-free
@filonenko-mikhail @olegrok please confirm or deny that this issue has been fixed till the current day's release.
There was a problem hiding this comment.
I think to problem still exists
3 participants
No description provided.