From a5263dc7ed4108b7ea57c08f82555bc7b2b4053c Mon Sep 17 00:00:00 2001 From: sfan5 Date: Sun, 18 May 2025 22:44:20 +0200 Subject: [PATCH] Do not allow vector components to be nil --- doc/breakages.md | 1 + src/script/common/c_converter.cpp | 69 ++++++++++++++----------------- src/script/common/c_converter.h | 13 +++++- src/script/lua_api/l_env.cpp | 23 +++++++---- 4 files changed, 58 insertions(+), 48 deletions(-) diff --git a/doc/breakages.md b/doc/breakages.md index 412cf2e41..6c9acbd95 100644 --- a/doc/breakages.md +++ b/doc/breakages.md @@ -25,3 +25,4 @@ This list is largely advisory and items may be reevaluated once the time comes. * remove built-in knockback and related functions entirely * remove `safe` parameter from `core.serialize`, always enforce `safe = true`. possibly error when `loadstring` calls are encountered in `core.deserialize`. +* introduce strict type checking for all instances of `v3s16` / `v3f` read from Lua diff --git a/src/script/common/c_converter.cpp b/src/script/common/c_converter.cpp index 138063d54..5b7331e47 100644 --- a/src/script/common/c_converter.cpp +++ b/src/script/common/c_converter.cpp @@ -18,6 +18,8 @@ extern "C" { #include #include "common/c_types.h" +static v3d read_v3d(lua_State *L, int index); +static v3d check_v3d(lua_State *L, int index); #define CHECK_TYPE(index, name, type) do { \ int t = lua_type(L, (index)); \ @@ -28,6 +30,13 @@ extern "C" { } \ } while(0) +#define CHECK_NOT_NIL(index, name) do { \ + if (lua_isnoneornil(L, (index))) { \ + throw LuaError(std::string("Invalid ") + (name) + \ + " (value is nil)."); \ + } \ + } while(0) + #define CHECK_FLOAT(value, name) do {\ if (std::isnan(value) || std::isinf(value)) { \ throw LuaError("Invalid float value for '" name \ @@ -35,7 +44,13 @@ extern "C" { } \ } while (0) +// strictly check type of coordinate +// (this won't permit string-to-int conversion, so maybe not the best idea?) #define CHECK_POS_COORD(index, name) CHECK_TYPE(index, "vector coordinate " name, LUA_TNUMBER) +// loosely check type of coordinate +#define CHECK_POS_COORD2(index, name) CHECK_NOT_NIL(index, "vector coordinate " name) + +// Note: not needed when using read_v3_aux #define CHECK_POS_TAB(index) CHECK_TYPE(index, "vector", LUA_TTABLE) @@ -44,6 +59,7 @@ extern "C" { */ static void read_v3_aux(lua_State *L, int index) { + // TODO: someone find out if it's faster to have the type check in Lua too CHECK_POS_TAB(index); lua_pushvalue(L, index); lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_READ_VECTOR); @@ -87,24 +103,12 @@ void push_v2f(lua_State *L, v2f p) v2s16 read_v2s16(lua_State *L, int index) { - v2s16 p; - CHECK_POS_TAB(index); - lua_getfield(L, index, "x"); - p.X = lua_tonumber(L, -1); - lua_pop(L, 1); - lua_getfield(L, index, "y"); - p.Y = lua_tonumber(L, -1); - lua_pop(L, 1); - return p; + return v2s16::from(read_v2f(L, index)); } void push_v2s16(lua_State *L, v2s16 p) { - lua_createtable(L, 0, 2); - lua_pushinteger(L, p.X); - lua_setfield(L, -2, "x"); - lua_pushinteger(L, p.Y); - lua_setfield(L, -2, "y"); + push_v2s32(L, v2s32::from(p)); } void push_v2s32(lua_State *L, v2s32 p) @@ -127,15 +131,7 @@ void push_v2u32(lua_State *L, v2u32 p) v2s32 read_v2s32(lua_State *L, int index) { - v2s32 p; - CHECK_POS_TAB(index); - lua_getfield(L, index, "x"); - p.X = lua_tonumber(L, -1); - lua_pop(L, 1); - lua_getfield(L, index, "y"); - p.Y = lua_tonumber(L, -1); - lua_pop(L, 1); - return p; + return v2s32::from(read_v2f(L, index)); } v2f read_v2f(lua_State *L, int index) @@ -143,9 +139,11 @@ v2f read_v2f(lua_State *L, int index) v2f p; CHECK_POS_TAB(index); lua_getfield(L, index, "x"); + CHECK_POS_COORD2(-1, "x"); p.X = lua_tonumber(L, -1); lua_pop(L, 1); lua_getfield(L, index, "y"); + CHECK_POS_COORD2(-1, "y"); p.Y = lua_tonumber(L, -1); lua_pop(L, 1); return p; @@ -170,30 +168,20 @@ v2f check_v2f(lua_State *L, int index) v3f read_v3f(lua_State *L, int index) { - read_v3_aux(L, index); - float x = lua_tonumber(L, -3); - float y = lua_tonumber(L, -2); - float z = lua_tonumber(L, -1); - lua_pop(L, 3); - return v3f(x, y, z); + return v3f::from(read_v3d(L, index)); } v3f check_v3f(lua_State *L, int index) { - read_v3_aux(L, index); - CHECK_POS_COORD(-3, "x"); - CHECK_POS_COORD(-2, "y"); - CHECK_POS_COORD(-1, "z"); - float x = lua_tonumber(L, -3); - float y = lua_tonumber(L, -2); - float z = lua_tonumber(L, -1); - lua_pop(L, 3); - return v3f(x, y, z); + return v3f::from(check_v3d(L, index)); } v3d read_v3d(lua_State *L, int index) { read_v3_aux(L, index); + CHECK_POS_COORD2(-3, "x"); + CHECK_POS_COORD2(-2, "y"); + CHECK_POS_COORD2(-1, "z"); double x = lua_tonumber(L, -3); double y = lua_tonumber(L, -2); double z = lua_tonumber(L, -1); @@ -286,18 +274,23 @@ video::SColor read_ARGB8(lua_State *L, int index) return std::fmax(0.0, std::fmin(255.0, c)); }; + // FIXME: maybe we should have strict type checks here. compare to is_color_table() + video::SColor color(0); CHECK_TYPE(index, "ARGB color", LUA_TTABLE); lua_getfield(L, index, "a"); color.setAlpha(lua_isnumber(L, -1) ? clamp_col(lua_tonumber(L, -1)) : 0xFF); lua_pop(L, 1); lua_getfield(L, index, "r"); + CHECK_NOT_NIL(-1, "color component R"); color.setRed(clamp_col(lua_tonumber(L, -1))); lua_pop(L, 1); lua_getfield(L, index, "g"); + CHECK_NOT_NIL(-1, "color component G"); color.setGreen(clamp_col(lua_tonumber(L, -1))); lua_pop(L, 1); lua_getfield(L, index, "b"); + CHECK_NOT_NIL(-1, "color component B"); color.setBlue(clamp_col(lua_tonumber(L, -1))); lua_pop(L, 1); return color; diff --git a/src/script/common/c_converter.h b/src/script/common/c_converter.h index b55f1c9c9..2744fa0b5 100644 --- a/src/script/common/c_converter.h +++ b/src/script/common/c_converter.h @@ -74,13 +74,23 @@ v2f check_v2f(lua_State *L, int index); v3f check_v3f(lua_State *L, int index); v3s16 check_v3s16(lua_State *L, int index); +// TODO: some day we should figure out the type-checking situation so it's done +// everywhere. (right now {x=true, y=false} as v2f is {0,0} with no warning) + +/// @warning relaxed type-checking, prefer `check_v3f`. v3f read_v3f(lua_State *L, int index); +/// @warning relaxed type-checking, prefer `check_v2f`. v2f read_v2f(lua_State *L, int index); +/// @warning relaxed type-checking v2s16 read_v2s16(lua_State *L, int index); +/// @warning relaxed type-checking v2s32 read_v2s32(lua_State *L, int index); +/// @warning relaxed type-checking, prefer `check_v3s16`. +v3s16 read_v3s16(lua_State *L, int index); + video::SColor read_ARGB8(lua_State *L, int index); bool read_color(lua_State *L, int index, video::SColor *color); -bool is_color_table (lua_State *L, int index); +bool is_color_table(lua_State *L, int index); /** * Read a floating-point axis-aligned box from Lua. @@ -95,7 +105,6 @@ bool is_color_table (lua_State *L, int index); */ aabb3f read_aabb3f(lua_State *L, int index, f32 scale); -v3s16 read_v3s16(lua_State *L, int index); std::vector read_aabb3f_vector (lua_State *L, int index, f32 scale); size_t read_stringlist(lua_State *L, int index, std::vector *result); diff --git a/src/script/lua_api/l_env.cpp b/src/script/lua_api/l_env.cpp index aafde7540..423479040 100644 --- a/src/script/lua_api/l_env.cpp +++ b/src/script/lua_api/l_env.cpp @@ -266,15 +266,22 @@ int ModApiEnv::l_bulk_swap_node(lua_State *L) // get_node_raw(x, y, z) -> content, param1, param2, pos_ok int ModApiEnv::l_get_node_raw(lua_State *L) { - GET_ENV_PTR; + GET_PLAIN_ENV_PTR; - // pos - // mirrors implementation of read_v3s16 (with the exact same rounding) - double x = lua_tonumber(L, 1); - double y = lua_tonumber(L, 2); - double z = lua_tonumber(L, 3); - v3s16 pos = doubleToInt(v3d(x, y, z), 1.0); - // Do it + v3s16 pos; + // mirrors the implementation of read_v3s16 (with the exact same rounding) + { + if (lua_isnoneornil(L, 1)) + throw LuaError("X position is nil"); + if (lua_isnoneornil(L, 2)) + throw LuaError("Y position is nil"); + if (lua_isnoneornil(L, 3)) + throw LuaError("Z position is nil"); + double x = lua_tonumber(L, 1); + double y = lua_tonumber(L, 2); + double z = lua_tonumber(L, 3); + pos = doubleToInt(v3d(x, y, z), 1.0); + } bool pos_ok; MapNode n = env->getMap().getNode(pos, &pos_ok); // Return node and pos_ok