Fix additional AOF filename issues. (#10110)

This extends the previous fix (#10049) to address any form of
non-printable or whitespace character (including newlines, quotes,
non-printables, etc.)

Also, removes the limitation on appenddirname, to align with the way
filenames are handled elsewhere in Redis.

src/aof.c

@@ -116,18 +116,18 @@ aofInfo *aofInfoDup(aofInfo *orig) {
 
 /* 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)) {
+    sds filename_repr = NULL;
-        /* If file_name contains spaces we wrap it in quotes. */
+
-        return sdscatprintf(buf, "%s \"%s\" %s %lld %s %c\n", 
+    if (sdsneedsrepr(ai->file_name))
-            AOF_MANIFEST_KEY_FILE_NAME, ai->file_name, 
+        filename_repr = sdscatrepr(sdsempty(), ai->file_name, sdslen(ai->file_name));
+
+    sds ret = sdscatprintf(buf, "%s %s %s %lld %s %c\n",
+        AOF_MANIFEST_KEY_FILE_NAME, filename_repr ? filename_repr : ai->file_name,
         AOF_MANIFEST_KEY_FILE_SEQ, ai->file_seq,
         AOF_MANIFEST_KEY_FILE_TYPE, ai->file_type);
-    } else {
+    sdsfree(filename_repr);
-        return sdscatprintf(buf, "%s %s %s %lld %s %c\n", 
+
-            AOF_MANIFEST_KEY_FILE_NAME, ai->file_name, 
+    return ret;
-            AOF_MANIFEST_KEY_FILE_SEQ, ai->file_seq, 
-            AOF_MANIFEST_KEY_FILE_TYPE, ai->file_type);
-    }
 }
 
 /* Method to free AOF list elements. */

src/config.c

@@ -2166,10 +2166,6 @@ 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/sds.c

@@ -1021,6 +1021,26 @@ sds sdscatrepr(sds s, const char *p, size_t len) {
     return sdscatlen(s,"\"",1);
 }
 
+/* Returns one if the string contains characters to be escaped
+ * by sdscatrepr(), zero otherwise.
+ *
+ * Typically, this should be used to help protect aggregated strings in a way
+ * that is compatible with sdssplitargs(). For this reason, also spaces will be
+ * treated as needing an escape.
+ */
+int sdsneedsrepr(const sds s) {
+    size_t len = sdslen(s);
+    const char *p = s;
+
+    while (len--) {
+        if (*p == '\\' || *p == '"' || *p == '\n' || *p == '\r' ||
+            *p == '\t' || *p == '\a' || *p == '\b' || !isprint(*p) || isspace(*p)) return 1;
+        p++;
+    }
+
+    return 0;
+}
+
 /* Helper function for sdssplitargs() that returns non zero if 'c'
  * is a valid hex digit. */
 int is_hex_digit(char c) {

src/sds.h

@@ -253,6 +253,7 @@ sds *sdssplitargs(const char *line, int *argc);
 sds sdsmapchars(sds s, const char *from, const char *to, size_t setlen);
 sds sdsjoin(char **argv, int argc, char *sep);
 sds sdsjoinsds(sds *argv, int argc, const char *sep, size_t seplen);
+int sdsneedsrepr(const sds s);
 
 /* Callback for sdstemplate. The function gets called by sdstemplate
  * every time a variable needs to be expanded. The variable name is

src/util.c

@@ -904,16 +904,6 @@ 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,7 +71,6 @@ 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

@@ -675,10 +675,10 @@ tags {"external:skip"} {
         }
     }
 
-    test {Multi Part AOF can handle appendfilename contains spaces} {
+    test {Multi Part AOF can handle appendfilename contains whitespaces} {
-        start_server [list overrides [list appendonly yes appendfilename "\" file seq .aof \""]] {
+        start_server [list overrides [list appendonly yes appendfilename "\" file seq \\n\\n.aof \""]] {
             set dir [get_redis_dir]
-            set aof_manifest_name [format "%s/%s/%s%s" $dir "appendonlydir" " file seq .aof " $::manifest_suffix]
+            set aof_manifest_name [format "%s/%s/%s%s" $dir "appendonlydir" " file seq \n\n.aof " $::manifest_suffix]
             set redis [redis [srv host] [srv port] 0 $::tls]
 
             assert_equal OK [$redis set k1 v1]
@@ -687,8 +687,8 @@ tags {"external:skip"} {
             waitForBgrewriteaof $redis
 
             assert_aof_manifest_content $aof_manifest_name {
-                {file " file seq .aof .2.base.rdb" seq 2 type b}
+                {file " file seq \n\n.aof .2.base.rdb" seq 2 type b}
-                {file " file seq .aof .2.incr.aof" seq 2 type i}
+                {file " file seq \n\n.aof .2.incr.aof" seq 2 type i}
             }
 
             set d1 [$redis debug digest]