From 9517b3e0045e2daf04bb10e14bf96bda9a6baba9 Mon Sep 17 00:00:00 2001 From: jyelon Date: Mon, 27 Dec 2021 16:44:12 -0500 Subject: [PATCH] Repair buggy error-handling in makeclass/getclass --- luprex/core/cpp/luastack.cpp | 173 +++++++++++++++++++---------- luprex/core/cpp/luastack.hpp | 28 ++++- luprex/core/cpp/source.cpp | 23 ++++ luprex/core/cpp/table.cpp | 5 +- luprex/core/cpp/world-accessor.cpp | 17 +-- 5 files changed, 170 insertions(+), 76 deletions(-) diff --git a/luprex/core/cpp/luastack.cpp b/luprex/core/cpp/luastack.cpp index 68bb6d87..6ed7fe22 100644 --- a/luprex/core/cpp/luastack.cpp +++ b/luprex/core/cpp/luastack.cpp @@ -188,66 +188,142 @@ lua_State *LuaStack::newthread(LuaSlot target) const { return result; } -void LuaStack::getclass(LuaSlot classtab, LuaSlot classname) const { +bool LuaStack::validclassname(const std::string &cname) { + if (cname.empty()) return false; + if (cname == "_G") return false; + return true; +} + +bool LuaStack::validclassname(LuaSlot slot) const { + if (!isstring(slot)) return false; + return validclassname(ckstring(slot)); +} + +std::string LuaStack::classname(LuaSlot tab) const { + std::string result; + if (istable(tab)) { + lua_pushstring(L_, "__class"); + lua_rawget(L_, tab); + if (lua_type(L_, -1) == LUA_TSTRING) { + lua_pushglobaltable(L_); // cname table + lua_pushvalue(L_, -2); // cname table cname + lua_rawget(L_, -2); // cname table ctab + if (lua_rawequal(L_, -1, tab)) { + size_t len; + const char *s = lua_tolstring(L_, -3, &len); + result = std::string(s, len); + if (!validclassname(result)) { + result = ""; + } + } + lua_pop(L_, 3); + } else { + lua_pop(L_, 1); + } + } + return result; +} + +std::string LuaStack::getclass(LuaSlot classtab, LuaSlot classname) const { lua_checkstack(L_, 20); LuaVar globtab, cname; LuaStack LS(L_, globtab, cname); - - if (!LS.isstring(classname)) { - luaL_error(L_, "Not a class name: %s", lua_tostring(L_, classname)); - } - - // Convert class name to a table. - if (LS.rawequal(classname, "_G")) { - luaL_error(L_, "_G is explicitly not allowed as a class name"); - } LS.getglobaltable(globtab); - LS.rawget(classtab, globtab, classname); - if (!LS.istable(classtab)) { - luaL_error(L_, "Not a class: %s", lua_tostring(L_, classname)); - } - LS.rawget(cname, classtab, "__class"); - if (!LS.rawequal(cname, classname)) { - luaL_error(L_, "Not a class: %s", lua_tostring(L_, classname)); - } - // OK, we're done. - LS.result(); + if (LS.isstring(classname)) { + if (!validclassname(LS.ckstring(classname))) { + std::string err = "Not allowed as a class name: " + LS.ckstring(classname); + LS.result(); + return err; + } + LS.rawget(classtab, globtab, classname); + if (!LS.istable(classtab)) { + std::string err = "Not a class: " + LS.ckstring(classname); + LS.result(); + return err; + } + LS.rawget(cname, classtab, "__class"); + if (!LS.rawequal(cname, classname)) { + std::string err = "Not a valid class: " + LS.ckstring(classname); + LS.result(); + return err; + } + LS.result(); + return ""; + } else if (LS.istable(classname)) { + LS.rawget(cname, classname, "__class"); + if (!LS.isstring(cname)) { + std::string err = "Table is not a class."; + LS.result(); + return err; + } + if (!validclassname(LS.ckstring(cname))) { + std::string err = "Not allowed as a class name: " + LS.ckstring(cname); + LS.result(); + return err; + } + LS.rawget(classtab, globtab, cname); + if (!LS.rawequal(classtab, classname)) { + std::string err = "Not a valid class: " + LS.ckstring(cname); + LS.result(); + return err; + } + LS.result(); + return ""; + } else { + std::string err = "getclass expects a string or a classtab"; + LS.result(); + return err; + } +} + +std::string LuaStack::getclass(LuaSlot tab, const char *name) const { + push_any_value(name); + LuaSpecial classname(lua_gettop(L_)); + std::string err = getclass(tab, classname); + lua_pop(L_, 1); + return err; +} + +std::string LuaStack::getclass(LuaSlot tab, const std::string &name) const { + push_any_value(name); + LuaSpecial classname(lua_gettop(L_)); + std::string err = getclass(tab, classname); + lua_pop(L_, 1); + return err; } void LuaStack::makeclass(LuaSlot classtab, LuaSlot classname) const { lua_checkstack(L_, 20); - LuaVar globtab; - LuaStack LS(L_, globtab); + LuaVar globtab, cname; + LuaStack LS(L_, globtab, cname); // Validate the class name. - if (!LS.isstring(classname)) { - luaL_error(L_, "Not a class name: %s", lua_tostring(L_, classname)); - } - if (LS.rawequal(classname, "_G")) { - luaL_error(L_, "_G is explicitly not allowed as a class name"); - } - + assert(LS.validclassname(classname)); + // Fetch the global environment from the registry. LS.getglobaltable(globtab); // Get the classtab from the global environment. - // Create it if it doesn't exist. LS.rawget(classtab, globtab, classname); - if (LS.isnil(classtab)) { + + // Make sure we're not reusing a table that isn't a class. + if (LS.istable(classtab)) { + LS.rawget(cname, classtab, "__class"); + } else { + LS.set(cname, LuaNil); + } + + // Make a new table if necessary. + if (!LS.rawequal(cname, classname)) { LS.newtable(classtab); LS.rawset(globtab, classname, classtab); + LS.rawset(classtab, "__class", classname); } - - // If the name isn't bound to a table, abort. - if (!LS.istable(classtab)) { - luaL_error(L_, "%s is not a class", ckstring(classname).c_str()); - } - + // Repair the special fields. LS.settabletype(classtab, LUA_TT_CLASS); LS.rawset(classtab, "__index", classtab); - LS.rawset(classtab, "__class", classname); // Put the stack back. LS.result(); @@ -267,27 +343,6 @@ void LuaStack::makeclass(LuaSlot tab, const std::string &name) const { lua_pop(L_, 1); } -std::string LuaStack::classname(LuaSlot tab) const { - std::string result; - if (istable(tab)) { - lua_pushstring(L_, "__class"); - lua_rawget(L_, tab); - if (lua_type(L_, -1) == LUA_TSTRING) { - lua_pushglobaltable(L_); // cname table - lua_pushvalue(L_, -2); // cname table cname - lua_rawget(L_, -2); // cname table ctab - if (lua_rawequal(L_, -1, tab)) { - size_t len; - const char *s = lua_tolstring(L_, -3, &len); - result = std::string(s, len); - } - lua_pop(L_, 3); - } else { - lua_pop(L_, 1); - } - } - return result; -} int64_t LuaStack::tanid(LuaSlot tab) const { int64_t result = 0; diff --git a/luprex/core/cpp/luastack.hpp b/luprex/core/cpp/luastack.hpp index c91b7557..04626fa0 100644 --- a/luprex/core/cpp/luastack.hpp +++ b/luprex/core/cpp/luastack.hpp @@ -386,17 +386,35 @@ public: int next(LuaSlot tab, LuaSlot key, LuaSlot value) const; - void getclass(LuaSlot tab, LuaSlot name) const; + // Return true if the classname is legal. + bool validclassname(LuaSlot value) const; + static bool validclassname(const std::string &cname); + + // Return the class name if x is a valid classtab. + // Otherwise, returns empty string. If nonempty, the + // result is guaranteed to be a validclassname. + // This can also function as an "isclass" operator. + std::string classname(LuaSlot x) const; + + // Look up a class. + // If there is a problem, returns an error message. + // There are lots of error conditions, including such things + // as no such class, corrupted class, classname invalid, etc. + std::string getclass(LuaSlot tab, LuaSlot name) const; + std::string getclass(LuaSlot tab, const char *name) const; + std::string getclass(LuaSlot tab, const std::string &name) const; + + // Create a class, or look up an existing class. + // WARNING: this routine assert-fails if the parameter is not + // a valid classname. Check the classname before calling this! void makeclass(LuaSlot tab, LuaSlot name) const; void makeclass(LuaSlot tab, const char *name) const; void makeclass(LuaSlot tab, const std::string &name) const; - std::string classname(LuaSlot tab) const; + // Get the ID of a tangible. It's a little weird to put this in + // this module. int64_t tanid(LuaSlot tab) const; - // There's no 'isclass' operator, but 'classname' will return empty - // string if tab is not a valid class. - void movesortablekey(LuaSlot val, LuaStack &other, LuaSlot otherslot); bool rawequal(LuaSlot v1, LuaSlot v2) const { diff --git a/luprex/core/cpp/source.cpp b/luprex/core/cpp/source.cpp index 1d92d3ae..7ea4ff2b 100644 --- a/luprex/core/cpp/source.cpp +++ b/luprex/core/cpp/source.cpp @@ -19,10 +19,27 @@ LuaDefine(makeclass, "classname", "create a class if it doesn't already exist") LuaArg classname; LuaRet classtab; LuaStack LS(L, classname, classtab); + if (!LS.isstring(classname)) { + luaL_error(L, "class name must be a string"); + } + if (!LS.validclassname(classname)) { + luaL_error(L, "invalid class name: %s", LS.ckstring(classname).c_str()); + }; LS.makeclass(classtab, classname); return LS.result(); } +LuaDefine(getclass, "classname", "get the classtab with the specified name") { + LuaArg classname; + LuaRet classtab; + LuaStack LS(L, classname, classtab); + std::string err = LS.getclass(classtab, classname); + if (err != "") { + luaL_error(L, "%s", err.c_str()); + } + return LS.result(); +} + LuaDefine(classname, "classtable", "get the class name from a class table") { LuaArg table; LuaRet result; @@ -41,6 +58,12 @@ LuaDefine(maketangible, "classname", "create a class if it doesn't already exist LuaRet classtab; LuaVar subtab; LuaStack LS(L, classname, classtab, subtab); + if (!LS.isstring(classname)) { + luaL_error(L, "class name must be a string"); + } + if (!LS.validclassname(classname)) { + luaL_error(L, "invalid class name: %s", LS.ckstring(classname).c_str()); + }; LS.makeclass(classtab, classname); LS.makesubtable(subtab, classtab, "action"); return LS.result(); diff --git a/luprex/core/cpp/table.cpp b/luprex/core/cpp/table.cpp index 5f8ea9fc..794088b9 100644 --- a/luprex/core/cpp/table.cpp +++ b/luprex/core/cpp/table.cpp @@ -229,6 +229,10 @@ LuaDefine(deque_create, "", "create a deque") { LuaVar classobj; LuaStack LS(L, rdeque, classobj); const int imax = 4; + std::string err = LS.getclass(classobj, "deque"); + if (err != "") { + luaL_error(L, "Class deque has been corrupted"); + } LS.createtable(rdeque, DEQUE_BASE + imax - 1, 0); LS.rawset(rdeque, DEQUE_LEFT, 0); LS.rawset(rdeque, DEQUE_FILL, 0); @@ -236,7 +240,6 @@ LuaDefine(deque_create, "", "create a deque") { for (int i = 0; i < imax; i++) { LS.rawset(rdeque, DEQUE_BASE + i, 0); } - LS.makeclass(classobj, "deque"); LS.setmetatable(rdeque, classobj); return LS.result(); } diff --git a/luprex/core/cpp/world-accessor.cpp b/luprex/core/cpp/world-accessor.cpp index b7042a4e..a0cbfaf6 100644 --- a/luprex/core/cpp/world-accessor.cpp +++ b/luprex/core/cpp/world-accessor.cpp @@ -80,10 +80,9 @@ LuaDefine(tangible_setclass, "tan,class", LuaStack LS(L, tanobj, classname, classtab, mt); World *w = World::fetch_global_pointer(L); w->tangible_get(LS, tanobj); - if (LS.classname(classname) != "") { - LS.set(classtab, classname); - } else { - LS.getclass(classtab, classname); + std::string err = LS.getclass(classtab, classname); + if (err != "") { + luaL_error(L, "%s", err.c_str()); } LS.getmetatable(mt, tanobj); LS.rawset(mt, "__index", classtab); @@ -140,13 +139,9 @@ LuaDefine(tangible_build, "configtable", LS.checktable(config); // Get the class of the new tangible. LS.rawget(classname, config, "class"); - - if (LS.isnil(classname)) { - luaL_error(L, "must specify a class name"); - } else if (LS.classname(classname) != "") { - LS.set(classtab, classname); - } else { - LS.getclass(classtab, classname); + std::string err = LS.getclass(classtab, classname); + if (err != "") { + luaL_error(L, "%s", err.c_str()); } // Parse the initial animation step.