From bd46a2abf48ca08809e52fb1ea0ca924803cba3e Mon Sep 17 00:00:00 2001 From: chenyang8094 Date: Mon, 10 Jan 2022 15:09:39 +0800 Subject: [PATCH] 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 | 35 ++++++++++++++++++++-------- src/config.c | 4 ++++ src/util.c | 10 ++++++++ src/util.h | 1 + tests/integration/aof-multi-part.tcl | 29 ++++++++++++++++++++--- 5 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/aof.c b/src/aof.c index a46e8a7e3b..d360d59f28 100644 --- a/src/aof.c +++ b/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; } diff --git a/src/config.c b/src/config.c index 055c849ce1..f206bcc8c0 100644 --- a/src/config.c +++ b/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; diff --git a/src/util.c b/src/util.c index 75086db429..5251465893 100644 --- a/src/util.c +++ b/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 diff --git a/src/util.h b/src/util.h index 7dce8ff69a..6c1feb08be 100644 --- a/src/util.h +++ b/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); diff --git a/tests/integration/aof-multi-part.tcl b/tests/integration/aof-multi-part.tcl index 0ff09ea04c..b5b2f3ff9c 100644 --- a/tests/integration/aof-multi-part.tcl +++ b/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.