Support whitespace characters in appendfilename, and ban them in appenddirname (#10049)

1. Ban whitespace characters in `appenddirname`   
2. Handle the case where `appendfilename` contains spaces (for backwards compatibility)

src/aof.c

@@ -113,6 +113,22 @@ aofInfo *aofInfoDup(aofInfo *orig) {
     return ai;
 }
 
+/* Format aofInfo as a string and it will be a line in the manifest. */
+sds aofInfoFormat(sds buf, aofInfo *ai) {
+    if (includeSpace(ai->file_name)) {
+        /* If file_name contains spaces we wrap it in quotes. */
+        return sdscatprintf(buf, "%s \"%s\" %s %lld %s %c\n", 
+            AOF_MANIFEST_KEY_FILE_NAME, ai->file_name, 
+            AOF_MANIFEST_KEY_FILE_SEQ, ai->file_seq, 
+            AOF_MANIFEST_KEY_FILE_TYPE, ai->file_type);
+    } else {
+        return sdscatprintf(buf, "%s %s %s %lld %s %c\n", 
+            AOF_MANIFEST_KEY_FILE_NAME, ai->file_name, 
+            AOF_MANIFEST_KEY_FILE_SEQ, ai->file_seq, 
+            AOF_MANIFEST_KEY_FILE_TYPE, ai->file_type);
+    }
+}
+
 /* Method to free AOF list elements. */
 void aofListFree(void *item) {
     aofInfo *ai = (aofInfo *)item;
@@ -169,12 +185,6 @@ sds getTempAofManifestFileName() {
  * The base file, if exists, will always be first, followed by history files,
  * and incremental files.
  */
-#define AOF_INFO_FORMAT_AND_CAT(buf, info)                      \
-    sdscatprintf((buf), "%s %s %s %lld %s %c\n",                \
-                 AOF_MANIFEST_KEY_FILE_NAME, (info)->file_name, \
-                 AOF_MANIFEST_KEY_FILE_SEQ, (info)->file_seq,   \
-                 AOF_MANIFEST_KEY_FILE_TYPE, (info)->file_type)
-
 sds getAofManifestAsString(aofManifest *am) {
     serverAssert(am != NULL);
 
@@ -185,21 +195,21 @@ sds getAofManifestAsString(aofManifest *am) {
     /* 1. Add BASE File information, it is always at the beginning
      * of the manifest file. */
     if (am->base_aof_info) {
-        buf = AOF_INFO_FORMAT_AND_CAT(buf, am->base_aof_info);
+        buf = aofInfoFormat(buf, am->base_aof_info);
     }
 
     /* 2. Add HISTORY type AOF information. */
     listRewind(am->history_aof_list, &li);
     while ((ln = listNext(&li)) != NULL) {
         aofInfo *ai = (aofInfo*)ln->value;
-        buf = AOF_INFO_FORMAT_AND_CAT(buf, ai);
+        buf = aofInfoFormat(buf, ai);
     }
 
     /* 3. Add INCR type AOF information. */
     listRewind(am->incr_aof_list, &li);
     while ((ln = listNext(&li)) != NULL) {
         aofInfo *ai = (aofInfo*)ln->value;
-        buf = AOF_INFO_FORMAT_AND_CAT(buf, ai);
+        buf = aofInfoFormat(buf, ai);
     }
 
     return buf;
@@ -280,6 +290,11 @@ void aofLoadManifestFromDisk(void) {
         }
 
         line = sdstrim(sdsnew(buf), " \t\r\n");
+        if (!sdslen(line)) {
+            err = "The AOF manifest file is invalid format";
+            goto loaderr;
+        }
+
         argv = sdssplitargs(line, &argc);
         /* 'argc < 6' was done for forward compatibility. */
         if (argv == NULL || argc < 6 || (argc % 2)) {
@@ -305,7 +320,7 @@ void aofLoadManifestFromDisk(void) {
 
         /* We have to make sure we load all the information. */
         if (!ai->file_name || !ai->file_seq || !ai->file_type) {
-            err = "Mismatched manifest key";
+            err = "The AOF manifest file is invalid format";
             goto loaderr;
         }
 

src/config.c

@@ -2166,6 +2166,10 @@ static int isValidAOFdirname(char *val, const char **err) {
         *err = "appenddirname can't be empty";
         return 0;
     }
+    if (includeSpace(val)) {
+        *err = "appenddirname can't contain whitespace characters";
+        return 0;
+    }
     if (!pathIsBaseName(val)) {
         *err = "appenddirname can't be a path, just a dirname";
         return 0;

src/util.c

@@ -904,6 +904,16 @@ sds makePath(char *path, char *filename) {
     return sdscatfmt(sdsempty(), "%s/%s", path, filename);
 }
 
+int includeSpace(char *s) {
+    if (s == NULL) return 0;
+    for (size_t i = 0; i < strlen(s); i++) {
+        if (isspace(s[i])) {
+            return 1;
+        }
+    }
+    return 0;
+}
+
 #ifdef REDIS_TEST
 #include <assert.h>
 

src/util.h

@@ -71,6 +71,7 @@ int dirExists(char *dname);
 int dirRemove(char *dname);
 int fileExist(char *filename);
 sds makePath(char *path, char *filename);
+int includeSpace(char *s);
 
 #ifdef REDIS_TEST
 int utilTest(int argc, char **argv, int flags);

tests/integration/aof-multi-part.tcl

@@ -186,7 +186,7 @@ tags {"external:skip"} {
                 fail "AOF loading didn't fail"
             }
 
-            assert_equal 1 [count_message_lines $server_path/stdout "Mismatched manifest key"]
+            assert_equal 2 [count_message_lines $server_path/stdout "The AOF manifest file is invalid format"]
         }
 
         clean_aof_persistence $aof_dirpath
@@ -213,7 +213,7 @@ tags {"external:skip"} {
                 fail "AOF loading didn't fail"
             }
 
-            assert_equal 2 [count_message_lines $server_path/stdout "The AOF manifest file is invalid format"]
+            assert_equal 3 [count_message_lines $server_path/stdout "The AOF manifest file is invalid format"]
         }
 
         clean_aof_persistence $aof_dirpath
@@ -267,7 +267,7 @@ tags {"external:skip"} {
                 fail "AOF loading didn't fail"
             }
 
-            assert_equal 3 [count_message_lines $server_path/stdout "The AOF manifest file is invalid format"]
+            assert_equal 4 [count_message_lines $server_path/stdout "The AOF manifest file is invalid format"]
         }
 
         clean_aof_persistence $aof_dirpath
@@ -675,6 +675,29 @@ tags {"external:skip"} {
         }
     }
 
+    test {Multi Part AOF can handle appendfilename contains spaces} {
+        start_server [list overrides [list appendonly yes appendfilename "\" file seq .aof \""]] {
+            set dir [get_redis_dir]
+            set aof_manifest_name [format "%s/%s/%s%s" $dir "appendonlydir" " file seq .aof " $::manifest_suffix]
+            set redis [redis [srv host] [srv port] 0 $::tls]
+
+            assert_equal OK [$redis set k1 v1]
+
+            $redis bgrewriteaof
+            waitForBgrewriteaof $redis
+
+            assert_aof_manifest_content $aof_manifest_name {
+                {file " file seq .aof .1.base.rdb" seq 1 type b}
+                {file " file seq .aof .2.incr.aof" seq 2 type i}
+            }
+
+            set d1 [$redis debug digest]
+            $redis debug loadaof
+            set d2 [$redis debug digest]
+            assert {$d1 eq $d2}
+        }
+    }
+
     # Test Part 2
     #
     # To test whether the AOFRW behaves as expected during the redis run.