lua: be stricter around nonsense returned from modules

Well, the part about giving semantics to nil actually
decreases strictness.

diff --git a/daemon/ffimodule.c b/daemon/ffimodule.c
index c1cb50e9880890524f3011c7eed3ad0241ce272a..7e55b09f36830033ebdc6ffe9d56fbf342affc5c 100644 (file)
--- a/daemon/ffimodule.c
+++ b/daemon/ffimodule.c
@@ -65,26 +65,6 @@ static int l_ffi_defer(lua_State *L)
        return uv_idle_start(check, l_ffi_resume_cb);
 }
 
-/** @internal Helper for calling the entrypoint, for kr_module functions. */
-static int l_ffi_call_mod(lua_State *L, int argc)
-{
-       int status = lua_pcall(L, argc, 1, 0);
-       if (status != 0) {
-               kr_log_error("error: %s\n", lua_tostring(L, -1));
-               lua_pop(L, 1);
-               return kr_error(EIO);
-       }
-       if (lua_isnumber(L, -1)) { /* Return code */
-               status = lua_tointeger(L, -1);
-       } else if (lua_isthread(L, -1)) { /* Continuations */
-               /* TODO: unused, possibly in a bad shape.  Meant KR_STATE_YIELD? */
-               assert(!ENOTSUP);
-               status = l_ffi_defer(lua_tothread(L, -1));
-       }
-       lua_pop(L, 1);
-       return status;
-}
-
 /** Common part of calling modname.(de)init in lua.
  * The function to call should be on top of the stack and it gets popped. */
 static int l_ffi_modcb(lua_State *L, struct kr_module *module)
@@ -141,9 +121,40 @@ static int l_ffi_call_layer(kr_layer_t *ctx, int slot_ix)
         * lua (full) userdata, as that's relatively expensive (GC-allocated).
         * Performance: copying isn't ideal, but it's not visible in profiles. */
        memcpy(&kr_layer_t_static, ctx, sizeof(*ctx));
-       const int ret = l_ffi_call_mod(L, 1);
-       /* The return codes are mixed at this point.  We need to return KR_STATE_* */
-       return ret < 0 ? KR_STATE_FAIL : ret;
+
+       int ret = lua_pcall(L, 1, 1, 0);
+       /* Handle result of the pcall.
+        * Default state: ctx->req->state seems safer than ctx->state,
+        * in case the pcall touched req->state. */
+       int state = ctx->req->state;
+       if (ret) { /* Exception or another lua problem. */
+               state = KR_STATE_FAIL;
+               kr_log_error("error: %s\n", lua_tostring(L, -1));
+
+       } else if (lua_isnumber(L, -1)) { /* Explicitly returned state. */
+               state = lua_tointeger(L, -1);
+               if (!kr_state_consistent(state)) {
+                       kr_log_error("error: nonsense state returned from lua module layer: %d\n",
+                                       state);
+                       state = KR_STATE_FAIL;
+               }
+
+       } else if (lua_isnil(L, -1)) { /* Don't change state. */
+
+       } else if (lua_isthread(L, -1)) { /* Continuations */
+               /* TODO: unused, possibly in a bad shape.  Meant KR_STATE_YIELD? */
+               assert(!ENOTSUP);
+               if (l_ffi_defer(lua_tothread(L, -1)) != 0)
+                       state = KR_STATE_FAIL;
+
+       } else { /* Nonsense returned. */
+               state = KR_STATE_FAIL;
+               kr_log_error("error: nonsense returned from lua module layer: %s\n",
+                               lua_tostring(L, -1));
+               /* Unfortunately we can't easily get name of the module/function here. */
+       }
+       lua_pop(L, 1);
+       return state;
 }
 
 static int l_ffi_layer_begin(kr_layer_t *ctx)

diff --git a/lib/layer.h b/lib/layer.h
index 5fbd9df2f476c6922adfabce8220e01631b5c7bc..823db83ec4c0b962aebfadcbef24ed2a71040924 100644 (file)
--- a/lib/layer.h
+++ b/lib/layer.h
@@ -59,6 +59,12 @@ enum kr_layer_state {
        KR_STATE_YIELD   = 1 << 4, /*!< Paused, waiting for a sub-query. */
 };
 
+/** Check that a kr_layer_state makes sense.  We're not very strict ATM. */
+static inline bool kr_state_consistent(enum kr_layer_state s)
+{
+       return s >= 0 && s < (1 << 5);
+}
+
 /* Forward declarations. */
 struct kr_layer_api;
 
@@ -72,7 +78,10 @@ typedef struct kr_layer {
        bool is_stream;       /*!< In glue for checkout layer it's used to pass the parameter. */
 } kr_layer_t;
 
-/** Packet processing module API.  All functions return the new kr_layer_state. */
+/** Packet processing module API.  All functions return the new kr_layer_state.
+ *
+ * Lua modules are allowed to return nil/nothing, meaning the state shall not change.
+ */
 struct kr_layer_api {
        /** Start of processing the DNS request. */
        int (*begin)(kr_layer_t *ctx);