Fix: server will crash if rdbload or rdbsave method is not provided in module (#8670)

With this fix, module data type registration will fail if the load or save callbacks are not defined, or the optional aux load and save callbacks are not either both defined or both missing.

src/module.c

@@ -4374,9 +4374,9 @@ robj *moduleTypeDupOrReply(client *c, robj *fromkey, robj *tokey, robj *value) {
  *             .defrag = myType_DefragCallback
  *         }
  *
- * * **rdb_load**: A callback function pointer that loads data from RDB files.
- * * **rdb_save**: A callback function pointer that saves data to RDB files.
- * * **aof_rewrite**: A callback function pointer that rewrites data as commands.
+ * * **rdb_load**: A callback function pointer that loads data from RDB files. (mandatory)
+ * * **rdb_save**: A callback function pointer that saves data to RDB files. (mandatory)
+ * * **aof_rewrite**: A callback function pointer that rewrites data as commands. (mandatory)
  * * **digest**: A callback function pointer that is used for `DEBUG DIGEST`.
  * * **free**: A callback function pointer that can free a type value.
  * * **aux_save**: A callback function pointer that saves out of keyspace data to RDB files.
@@ -4416,8 +4416,8 @@ robj *moduleTypeDupOrReply(client *c, robj *fromkey, robj *tokey, robj *value) {
  * Note: the module name "AAAAAAAAA" is reserved and produces an error, it
  * happens to be pretty lame as well.
  *
- * If there is already a module registering a type with the same name,
- * and if the module name or encver is invalid, NULL is returned.
+ * If there is already a module registering a type with the same name, the name
+ * or encver are invalid, or a mandatory callback is missing, NULL is returned.
  * Otherwise the new type is registered into Redis, and a reference of
  * type RedisModuleType is returned: the caller of the function should store
  * this reference into a global variable to make future use of it in the
@@ -4460,6 +4460,16 @@ moduleType *RM_CreateDataType(RedisModuleCtx *ctx, const char *name, int encver,
         } v3;
     } *tms = (struct typemethods*) typemethods_ptr;
 
+    /* Persistence functions can't be NULL */
+    if (tms->rdb_load == NULL || tms->rdb_save == NULL || tms->aof_rewrite == NULL) {
+        return NULL;
+    }
+
+    /* Function aux_load and aux_save must be either both defined or undefined. */
+    if ((tms->version >= 2) && ((tms->v2.aux_load != NULL) != (tms->v2.aux_save != NULL))) {
+        return NULL;
+    }
+
     moduleType *mt = zcalloc(sizeof(*mt));
     mt->id = id;
     mt->module = ctx->module;

tests/modules/datatype.c

@@ -32,6 +32,10 @@ static void datatype_save(RedisModuleIO *io, void *value) {
     RedisModule_SaveString(io, dt->strval);
 }
 
+static void datatype_rewrite(RedisModuleIO *aof, RedisModuleString *key, void *value) {
+    /*  do-nothing callback. */
+}
+
 static void datatype_free(void *value) {
     if (value) {
         DataType *dt = (DataType *) value;
@@ -190,6 +194,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
         .version = REDISMODULE_TYPE_METHOD_VERSION,
         .rdb_load = datatype_load,
         .rdb_save = datatype_save,
+        .aof_rewrite = datatype_rewrite,
         .free = datatype_free,
         .copy = datatype_copy
     };

tests/modules/defragtest.c

@@ -25,6 +25,20 @@ unsigned long int global_defragged = 0;
 int global_strings_len = 0;
 RedisModuleString **global_strings = NULL;
 
+static void *fragtype_load(RedisModuleIO *io, int encver) {
+    /*  do-nothing callback. */
+    return NULL;
+}
+
+static void fragtype_save(RedisModuleIO *io, void *value) {
+    /*  do-nothing callback. */
+}
+
+static void fragtype_rewrite(RedisModuleIO *aof, RedisModuleString *key, void *value) {
+    /*  do-nothing callback. */
+}
+
+
 static void createGlobalStrings(RedisModuleCtx *ctx, int count)
 {
     global_strings_len = count;
@@ -211,6 +225,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
 
     RedisModuleTypeMethods tm = {
             .version = REDISMODULE_TYPE_METHOD_VERSION,
+            .rdb_load = fragtype_load,
+            .rdb_save = fragtype_save,
+            .aof_rewrite = fragtype_rewrite,
             .free = FragFree,
             .free_effort = FragFreeEffort,
             .defrag = FragDefrag

tests/modules/testrdb.c

@@ -79,6 +79,11 @@ void testrdb_type_save(RedisModuleIO *rdb, void *value) {
     RedisModule_SaveLongDouble(rdb, 0.333333333333333333L);
 }
 
+
+void testrdb_type_aof_rewrite(RedisModuleIO *aof, RedisModuleString *key, void *value) {
+    /*  do-nothing callback. */
+}
+
 void testrdb_aux_save(RedisModuleIO *rdb, int when) {
     if (conf_aux_count==1) assert(when == REDISMODULE_AUX_AFTER_RDB);
     if (conf_aux_count==0) assert(0);
@@ -240,7 +245,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
             .version = 1,
             .rdb_load = testrdb_type_load,
             .rdb_save = testrdb_type_save,
-            .aof_rewrite = NULL,
+            .aof_rewrite = testrdb_type_aof_rewrite,
             .digest = NULL,
             .free = testrdb_type_free,
         };
@@ -253,7 +258,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
             .version = REDISMODULE_TYPE_METHOD_VERSION,
             .rdb_load = testrdb_type_load,
             .rdb_save = testrdb_type_save,
-            .aof_rewrite = NULL,
+            .aof_rewrite = testrdb_type_aof_rewrite,
             .digest = NULL,
             .free = testrdb_type_free,
             .aux_load = testrdb_aux_load,