Lua client fixes#1003
Merged
Merged
Conversation
encoder.client_name padded a name to encoder.CLIENT_NAME_LENGTH, which is not defined anywhere, so calling it raised an error attempting arithmetic on a nil value. It was left over from before the protocol used protocol buffers, when the connection message carried a fixed length name; the name is now set directly on the connection request message. Nothing called it.
The result of the pcall that coerces an argument was assigned to an undeclared ok, which made it a global, so every call needing a coercion wrote to the global namespace and left the value behind. Only ok is declared local. The value it is assigned alongside is the loop variable holding the argument, and has to stay that way: declaring both would make a new local scoped to the branch, and the coerced value is read after it, so the argument would silently be sent uncoerced. The test passes a plain table where a list is expected, which is a coercion that succeeds. Passing a list where a tuple is expected does not exercise this, as a tuple's Lua type is List, so the argument is already considered valid and no coercion is attempted.
set_values passed self._enum_name, which is never assigned; the field set when the type is constructed is _class_name. The name reaching _create_enum_type was therefore always nil. It is harmless today only because that function ignores both the service and class names it is given, and uses only the values.
djungelorm
force-pushed
the
lua-client-fixes
branch
from
July 21, 2026 20:10
519f282 to
cbc4e89
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three unrelated defects in the Lua client, each fixed in its own commit.
Global leak when coercing an argument. The result of the pcall that coerces an argument was assigned to an undeclared
ok, making it a global, so every call needing a coercion wrote to and left a value in the global namespace. Onlyokis now declared local: the value it is assigned alongside is the loop variable holding the argument and has to stay that way, since declaring both would scope a new local to the branch and the coerced value is read after it — the argument would then be silently sent uncoerced. A comment records this.Broken
encoder.client_name. It padded a name toencoder.CLIENT_NAME_LENGTH, which is never defined, so the function raised an error whatever it was passed. It is a leftover from before the protocol used protocol buffers and nothing calls it, so it is removed.Enumeration name never passed to its Lua type.
set_valuespassedself._enum_name, which is never assigned; the field set when the type is constructed is_class_name, so the name reaching_create_enum_typewas always nil. This is harmless today only because that function ignores the service and class names it is given and uses only the values, hence no changelog entry.Testing. New
test_coercion_does_not_leak_a_globalin the Lua client tests passes a plain table where a list is expected, which is a coercion that succeeds, and asserts no globalokis left behind. Passing a list where a tuple is expected does not exercise this, as a tuple's Lua type isList, so the argument is already considered valid and no coercion is attempted.