Do not allow vector components to be nil

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

src/script/common/c_converter.cpp

@@ -18,6 +18,8 @@ extern "C" {
 #include <cmath>
 #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;
+	return v2s16::from(read_v2f(L, index));
-	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;
 }
 
 void push_v2s16(lua_State *L, v2s16 p)
 {
-	lua_createtable(L, 0, 2);
+	push_v2s32(L, v2s32::from(p));
-	lua_pushinteger(L, p.X);
-	lua_setfield(L, -2, "x");
-	lua_pushinteger(L, p.Y);
-	lua_setfield(L, -2, "y");
 }
 
 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;
+	return v2s32::from(read_v2f(L, index));
-	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;
 }
 
 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);
+	return v3f::from(read_v3d(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);
 }
 
 v3f check_v3f(lua_State *L, int index)
 {
-	read_v3_aux(L, index);
+	return v3f::from(check_v3d(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);
 }
 
 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;

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<aabb3f> read_aabb3f_vector  (lua_State *L, int index, f32 scale);
 size_t read_stringlist(lua_State *L, int index,
 		std::vector<std::string> *result);

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
+	v3s16 pos;
-	// mirrors implementation of read_v3s16 (with the exact same rounding)
+	// mirrors the implementation of read_v3s16 (with the exact same rounding)
-	double x = lua_tonumber(L, 1);
+	{
-	double y = lua_tonumber(L, 2);
+		if (lua_isnoneornil(L, 1))
-	double z = lua_tonumber(L, 3);
+			throw LuaError("X position is nil");
-	v3s16 pos = doubleToInt(v3d(x, y, z), 1.0);
+		if (lua_isnoneornil(L, 2))
-	// Do it
+			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