From 5ddc89a033c47430f9b813e35ae418d6a0ee35fd Mon Sep 17 00:00:00 2001 From: Bart van Strien Date: Wed, 30 Aug 2017 13:07:57 +0200 Subject: [PATCH] Added cycle detection to Variant We accidentally enabled nested tables previously, and now it errors properly when the tables contain cycles. Before you'd get (or at least I got) a nice lua stack overflow error. --HG-- branch : minor --- changes.txt | 1 + src/common/Variant.cpp | 20 +++++++++++++++++--- src/common/Variant.h | 3 ++- src/modules/event/Event.cpp | 4 +++- src/modules/event/wrap_Event.cpp | 8 +++++--- src/modules/thread/wrap_Channel.cpp | 22 +++++++++++++--------- src/modules/thread/wrap_LuaThread.cpp | 4 +++- 7 files changed, 44 insertions(+), 18 deletions(-) diff --git a/changes.txt b/changes.txt index 1d74c06ce..3a076f10e 100644 --- a/changes.txt +++ b/changes.txt @@ -53,6 +53,7 @@ Released: N/A * Added a variant of Mesh:setVertexMap which accepts a Data object. * Added love.window.updateMode. * Added the ability to prevent love from creating a stencil buffer for the window. + * Added cycle detection to Variant's table serialization, cycles now cause an error, rather than a stack overflow. * Changed all color values to be in the range 0-1, rather than 0-255. * Changed high-dpi functionality to require much less code (often none at all) for things to appear at the correct sizes and positions. diff --git a/src/common/Variant.cpp b/src/common/Variant.cpp index 8631de593..3e71b5e9e 100644 --- a/src/common/Variant.cpp +++ b/src/common/Variant.cpp @@ -18,6 +18,8 @@ * 3. This notice may not be removed or altered from any source distribution. **/ +#include + #include "Variant.h" #include "common/StringMap.h" @@ -162,7 +164,7 @@ Variant &Variant::operator = (const Variant &v) return *this; } -Variant Variant::fromLua(lua_State *L, int n, bool allowTables) +Variant Variant::fromLua(lua_State *L, int n, std::set *tableSet) { size_t len; const char *str; @@ -186,11 +188,23 @@ Variant Variant::fromLua(lua_State *L, int n, bool allowTables) case LUA_TNIL: return Variant(); case LUA_TTABLE: - if (allowTables) { bool success = true; + std::unique_ptr> tableSetPtr; std::vector> *table = new std::vector>(); + // If we had no tables argument, allocate one now, and store it in our unique_ptr + if (tableSet == nullptr) + tableSetPtr.reset(tableSet = new std::set); + + // Now make sure this table wasn't already serialised + { + const void *table = lua_topointer(L, n); + auto result = tableSet->insert(table); + if (!result.second) // insertion failed + throw love::Exception("Cycle detected in table"); + } + size_t len = luax_objlen(L, -1); if (len > 0) table->reserve(len); @@ -199,7 +213,7 @@ Variant Variant::fromLua(lua_State *L, int n, bool allowTables) while (lua_next(L, n)) { - table->emplace_back(fromLua(L, -2), fromLua(L, -1)); + table->emplace_back(fromLua(L, -2, tableSet), fromLua(L, -1, tableSet)); lua_pop(L, 1); const auto &p = table->back(); diff --git a/src/common/Variant.h b/src/common/Variant.h index 3e2a7d981..9c1494590 100644 --- a/src/common/Variant.h +++ b/src/common/Variant.h @@ -27,6 +27,7 @@ #include #include +#include namespace love { @@ -63,7 +64,7 @@ public: Type getType() const { return type; } - static Variant fromLua(lua_State *L, int n, bool allowTables = true); + static Variant fromLua(lua_State *L, int n, std::set *tableSet = nullptr); void toLua(lua_State *L) const; private: diff --git a/src/modules/event/Event.cpp b/src/modules/event/Event.cpp index 24a0636f4..2e399306a 100644 --- a/src/modules/event/Event.cpp +++ b/src/modules/event/Event.cpp @@ -63,7 +63,9 @@ Message *Message::fromLua(lua_State *L, int n) if (lua_isnoneornil(L, n+i)) break; - vargs.push_back(Variant::fromLua(L, n+i)); + luax_catchexcept(L, [&]() { + vargs.push_back(Variant::fromLua(L, n+i)); + }); if (vargs.back().getType() == Variant::UNKNOWN) { diff --git a/src/modules/event/wrap_Event.cpp b/src/modules/event/wrap_Event.cpp index d475281fd..2578ce27f 100644 --- a/src/modules/event/wrap_Event.cpp +++ b/src/modules/event/wrap_Event.cpp @@ -93,10 +93,12 @@ int w_clear(lua_State *L) int w_quit(lua_State *L) { - std::vector args = {Variant::fromLua(L, 1)}; + luax_catchexcept(L, [&]() { + std::vector args = {Variant::fromLua(L, 1)}; - StrongRef m(new Message("quit", args), Acquire::NORETAIN); - instance()->push(m); + StrongRef m(new Message("quit", args), Acquire::NORETAIN); + instance()->push(m); + }); luax_pushboolean(L, true); return 1; diff --git a/src/modules/thread/wrap_Channel.cpp b/src/modules/thread/wrap_Channel.cpp index 3701b8682..41fb8733f 100644 --- a/src/modules/thread/wrap_Channel.cpp +++ b/src/modules/thread/wrap_Channel.cpp @@ -33,21 +33,25 @@ Channel *luax_checkchannel(lua_State *L, int idx) int w_Channel_push(lua_State *L) { Channel *c = luax_checkchannel(L, 1); - Variant var = Variant::fromLua(L, 2); - if (var.getType() == Variant::UNKNOWN) - return luaL_argerror(L, 2, "boolean, number, string, love type, or flat table expected"); - uint64 id = c->push(var); - lua_pushnumber(L, (lua_Number) id); + luax_catchexcept(L, [&]() { + Variant var = Variant::fromLua(L, 2); + if (var.getType() == Variant::UNKNOWN) + luaL_argerror(L, 2, "boolean, number, string, love type, or table expected"); + uint64 id = c->push(var); + lua_pushnumber(L, (lua_Number) id); + }); return 1; } int w_Channel_supply(lua_State *L) { Channel *c = luax_checkchannel(L, 1); - Variant var = Variant::fromLua(L, 2); - if (var.getType() == Variant::UNKNOWN) - return luaL_argerror(L, 2, "boolean, number, string, love type, or flat table expected"); - c->supply(var); + luax_catchexcept(L, [&]() { + Variant var = Variant::fromLua(L, 2); + if (var.getType() == Variant::UNKNOWN) + luaL_argerror(L, 2, "boolean, number, string, love type, or table expected"); + c->supply(var); + }); return 0; } diff --git a/src/modules/thread/wrap_LuaThread.cpp b/src/modules/thread/wrap_LuaThread.cpp index d7dd7c815..db71f9ca1 100644 --- a/src/modules/thread/wrap_LuaThread.cpp +++ b/src/modules/thread/wrap_LuaThread.cpp @@ -38,7 +38,9 @@ int w_Thread_start(lua_State *L) for (int i = 0; i < nargs; ++i) { - args.push_back(Variant::fromLua(L, i+2)); + luax_catchexcept(L, [&]() { + args.push_back(Variant::fromLua(L, i+2)); + }); if (args.back().getType() == Variant::UNKNOWN) {