diff options
author | Yossi Gottlieb <yossigo@gmail.com> | 2022-01-18 12:52:27 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-18 12:52:27 +0200 |
commit | 25e6d4d4597d6ca06503d5fa76af0e4e1b57302e (patch) | |
tree | 66e78300bc0ec4983c0223e17a3065aa6793146c | |
parent | 51f9bed3dd8d35a59e9124e49450c9795822dbf5 (diff) | |
download | redis-25e6d4d4597d6ca06503d5fa76af0e4e1b57302e.tar.gz |
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.
-rw-r--r-- | src/aof.c | 24 | ||||
-rw-r--r-- | src/config.c | 4 | ||||
-rw-r--r-- | src/sds.c | 20 | ||||
-rw-r--r-- | src/sds.h | 1 | ||||
-rw-r--r-- | src/util.c | 10 | ||||
-rw-r--r-- | src/util.h | 1 | ||||
-rw-r--r-- | tests/integration/aof-multi-part.tcl | 10 |
7 files changed, 38 insertions, 32 deletions
@@ -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)) { - /* 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); - } + sds filename_repr = NULL; + + if (sdsneedsrepr(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); + sdsfree(filename_repr); + + return ret; } /* Method to free AOF list elements. */ diff --git a/src/config.c b/src/config.c index d11445775..7915385cf 100644 --- a/src/config.c +++ b/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; @@ -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) { @@ -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 diff --git a/src/util.c b/src/util.c index 525146589..75086db42 100644 --- a/src/util.c +++ b/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> diff --git a/src/util.h b/src/util.h index 6c1feb08b..7dce8ff69 100644 --- a/src/util.h +++ b/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); diff --git a/tests/integration/aof-multi-part.tcl b/tests/integration/aof-multi-part.tcl index ac5ed4f33..6957c3dd0 100644 --- a/tests/integration/aof-multi-part.tcl +++ b/tests/integration/aof-multi-part.tcl @@ -675,10 +675,10 @@ tags {"external:skip"} { } } - test {Multi Part AOF can handle appendfilename contains spaces} { - start_server [list overrides [list appendonly yes appendfilename "\" file seq .aof \""]] { + test {Multi Part AOF can handle appendfilename contains whitespaces} { + 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 .aof .2.incr.aof" seq 2 type i} + {file " file seq \n\n.aof .2.base.rdb" seq 2 type b} + {file " file seq \n\n.aof .2.incr.aof" seq 2 type i} } set d1 [$redis debug digest] |