From 3c7d6a185329f2dc20c82a29bdbe125d5ad4140b Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Thu, 4 Mar 2021 15:03:49 +0200 Subject: [PATCH] Improve redis-cli non-binary safe string handling. (#8566) * The `redis-cli --scan` output should honor output mode (set explicitly or implicitly), and quote key names when not in raw mode. * Technically this is a breaking change, but it should be very minor since raw mode is by default on for non-tty output. * It should only affect TTY output (human users) or non-tty output if `--no-raw` is specified. * Added `--quoted-input` option to treat all arguments as potentially quoted strings. * Added `--quoted-pattern` option to accept a potentially quoted pattern. Unquoting is applied to potentially quoted input only if single or double quotes are used. Fixes #8561, #8563 --- src/redis-cli.c | 121 ++++++++++++++++++++------------ tests/integration/redis-cli.tcl | 36 ++++++++++ 2 files changed, 113 insertions(+), 44 deletions(-) diff --git a/src/redis-cli.c b/src/redis-cli.c index 63d31f79a..63cd4e463 100644 --- a/src/redis-cli.c +++ b/src/redis-cli.c @@ -227,7 +227,7 @@ static struct config { int scan_mode; int intrinsic_latency_mode; int intrinsic_latency_duration; - char *pattern; + sds pattern; char *rdb_filename; int bigkeys; int memkeys; @@ -237,6 +237,7 @@ static struct config { char *auth; int askpass; char *user; + int quoted_input; /* Force input args to be treated as quoted strings */ int output; /* output mode, see OUTPUT_* defines */ int push_output; /* Should we display spontaneous PUSH replies */ sds mb_delim; @@ -763,6 +764,23 @@ static void freeHintsCallback(void *ptr) { * Networking / parsing *--------------------------------------------------------------------------- */ +/* Unquote a null-terminated string and return it as a binary-safe sds. */ +static sds unquoteCString(char *str) { + int count; + sds *unquoted = sdssplitargs(str, &count); + sds res = NULL; + + if (unquoted && count == 1) { + res = unquoted[0]; + unquoted[0] = NULL; + } + + if (unquoted) + sdsfreesplitres(unquoted, count); + + return res; +} + /* Send AUTH command to the server */ static int cliAuth(redisContext *ctx, char *user, char *auth) { redisReply *reply; @@ -1533,6 +1551,8 @@ static int parseOptions(int argc, char **argv) { config.output = OUTPUT_RAW; } else if (!strcmp(argv[i],"--no-raw")) { config.output = OUTPUT_STANDARD; + } else if (!strcmp(argv[i],"--quoted-input")) { + config.quoted_input = 1; } else if (!strcmp(argv[i],"--csv")) { config.output = OUTPUT_CSV; } else if (!strcmp(argv[i],"--latency")) { @@ -1557,7 +1577,15 @@ static int parseOptions(int argc, char **argv) { } else if (!strcmp(argv[i],"--scan")) { config.scan_mode = 1; } else if (!strcmp(argv[i],"--pattern") && !lastarg) { - config.pattern = argv[++i]; + sdsfree(config.pattern); + config.pattern = sdsnew(argv[++i]); + } else if (!strcmp(argv[i],"--quoted-pattern") && !lastarg) { + sdsfree(config.pattern); + config.pattern = unquoteCString(argv[++i]); + if (!config.pattern) { + fprintf(stderr,"Invalid quoted string specified for --quoted-pattern.\n"); + exit(1); + } } else if (!strcmp(argv[i],"--intrinsic-latency") && !lastarg) { config.intrinsic_latency_mode = 1; config.intrinsic_latency_duration = atoi(argv[++i]); @@ -1841,6 +1869,7 @@ static void usage(void) { " --raw Use raw formatting for replies (default when STDOUT is\n" " not a tty).\n" " --no-raw Force formatted output even when STDOUT is not a tty.\n" +" --quoted-input Force input to be handled as quoted strings.\n" " --csv Output in CSV format.\n" " --show-pushes Whether to print RESP3 PUSH messages. Enabled by default when\n" " STDOUT is a tty but can be overriden with --show-pushes no.\n" @@ -1876,6 +1905,8 @@ static void usage(void) { " --scan List all keys using the SCAN command.\n" " --pattern Keys pattern when using the --scan, --bigkeys or --hotkeys\n" " options (default: *).\n" +" --quoted-pattern Same as --pattern, but the specified string can be\n" +" quoted, in order to pass an otherwise non binary-safe string.\n" " --intrinsic-latency Run a test to measure intrinsic system latency.\n" " The test will run for the specified amount of seconds.\n" " --eval Send an EVAL command using the Lua script at .\n" @@ -1901,6 +1932,7 @@ static void usage(void) { " redis-cli get mypasswd\n" " redis-cli -r 100 lpush mylist x\n" " redis-cli -r 100 -i 1 info | grep used_memory_human:\n" +" redis-cli --quoted-input set '\"null-\\x00-separated\"' value\n" " redis-cli --eval myscript.lua key1 key2 , arg1 arg2 arg3\n" " redis-cli --scan --pattern '*:12345*'\n" "\n" @@ -1930,22 +1962,28 @@ static int confirmWithYes(char *msg, int ignore_force) { return (nread != 0 && !strcmp("yes", buf)); } -/* Turn the plain C strings into Sds strings */ -static char **convertToSds(int count, char** args) { - int j; - char **sds = zmalloc(sizeof(char*)*count); +/* Create an sds array from argv, either as-is or by dequoting every + * element. When quoted is non-zero, may return a NULL to indicate an + * invalid quoted string. + */ +static sds *getSdsArrayFromArgv(int argc, char **argv, int quoted) { + sds *res = sds_malloc(sizeof(sds) * argc); - for(j = 0; j < count; j++) - sds[j] = sdsnew(args[j]); + for (int j = 0; j < argc; j++) { + if (quoted) { + sds unquoted = unquoteCString(argv[j]); + if (!unquoted) { + while (--j >= 0) sdsfree(res[j]); + sds_free(res); + return NULL; + } + res[j] = unquoted; + } else { + res[j] = sdsnew(argv[j]); + } + } - return sds; -} - -static void freeConvertedSds(int count, char **sds) { - int j; - for (j = 0; j < count; j++) - sdsfree(sds[j]); - zfree(sds); + return res; } static int issueCommandRepeat(int argc, char **argv, long repeat) { @@ -2178,17 +2216,19 @@ static void repl(void) { static int noninteractive(int argc, char **argv) { int retval = 0; - - argv = convertToSds(argc, argv); - if (config.stdinarg) { - argv = zrealloc(argv, (argc+1)*sizeof(char*)); - argv[argc] = readArgFromStdin(); - retval = issueCommand(argc+1, argv); - sdsfree(argv[argc]); - } else { - retval = issueCommand(argc, argv); + sds *sds_args = getSdsArrayFromArgv(argc, argv, config.quoted_input); + if (!sds_args) { + printf("Invalid quoted string\n"); + return 1; } - freeConvertedSds(argc, argv); + if (config.stdinarg) { + sds_args = sds_realloc(sds_args, (argc + 1) * sizeof(sds)); + sds_args[argc] = readArgFromStdin(); + argc++; + } + + retval = issueCommand(argc, sds_args); + sdsfreesplitres(sds_args, argc); return retval; } @@ -7332,8 +7372,8 @@ static redisReply *sendScan(unsigned long long *it) { redisReply *reply; if (config.pattern) - reply = redisCommand(context,"SCAN %llu MATCH %s", - *it,config.pattern); + reply = redisCommand(context, "SCAN %llu MATCH %b", + *it, config.pattern, sdslen(config.pattern)); else reply = redisCommand(context,"SCAN %llu",*it); @@ -7945,23 +7985,16 @@ static void scanMode(void) { unsigned long long cur = 0; do { - if (config.pattern) - reply = redisCommand(context,"SCAN %llu MATCH %s", - cur,config.pattern); - else - reply = redisCommand(context,"SCAN %llu",cur); - if (reply == NULL) { - printf("I/O error\n"); - exit(1); - } else if (reply->type == REDIS_REPLY_ERROR) { - printf("ERROR: %s\n", reply->str); - exit(1); - } else { - unsigned int j; - - cur = strtoull(reply->element[0]->str,NULL,10); - for (j = 0; j < reply->element[1]->elements; j++) + reply = sendScan(&cur); + for (unsigned int j = 0; j < reply->element[1]->elements; j++) { + if (config.output == OUTPUT_STANDARD) { + sds out = sdscatrepr(sdsempty(), reply->element[1]->element[j]->str, + reply->element[1]->element[j]->len); + printf("%s\n", out); + sdsfree(out); + } else { printf("%s\n", reply->element[1]->element[j]->str); + } } freeReplyObject(reply); } while(cur != 0); diff --git a/tests/integration/redis-cli.tcl b/tests/integration/redis-cli.tcl index d877542ed..7e8b41fca 100644 --- a/tests/integration/redis-cli.tcl +++ b/tests/integration/redis-cli.tcl @@ -207,6 +207,28 @@ start_server {tags {"cli"}} { assert_equal "foo\nbar" [run_cli lrange list 0 -1] } + test_nontty_cli "Quoted input arguments" { + r set "\x00\x00" "value" + assert_equal "value" [run_cli --quoted-input get {"\x00\x00"}] + } + + test_nontty_cli "No accidental unquoting of input arguments" { + run_cli --quoted-input set {"\x41\x41"} quoted-val + run_cli set {"\x41\x41"} unquoted-val + + assert_equal "quoted-val" [r get AA] + assert_equal "unquoted-val" [r get {"\x41\x41"}] + } + + test_nontty_cli "Invalid quoted input arguments" { + catch {run_cli --quoted-input set {"Unterminated}} err + assert_match {*exited abnormally*} $err + + # A single arg that unquotes to two arguments is also not expected + catch {run_cli --quoted-input set {"arg1" "arg2"}} err + assert_match {*exited abnormally*} $err + } + test_nontty_cli "Read last argument from pipe" { assert_equal "OK" [run_cli_with_input_pipe "echo foo" set key] assert_equal "foo\n" [r get key] @@ -247,6 +269,20 @@ start_server {tags {"cli"}} { test_redis_cli_rdb_dump } + test "Scan mode" { + r flushdb + populate 1000 key: 1 + + # basic use + assert_equal 1000 [llength [split [run_cli --scan]]] + + # pattern + assert_equal {key:2} [run_cli --scan --pattern "*:2"] + + # pattern matching with a quoted string + assert_equal {key:2} [run_cli --scan --quoted-pattern {"*:\x32"}] + } + test "Connecting as a replica" { set fd [open_cli "--replica"] wait_for_condition 500 500 {