diff --git a/src/config.c b/src/config.c index 5b9616733..957c20fc2 100644 --- a/src/config.c +++ b/src/config.c @@ -384,6 +384,62 @@ static int updateOOMScoreAdjValues(sds *args, const char **err, int apply) { return C_OK; } +/* Parse an array of `arg_len` sds strings, validate and populate + * server.client_obuf_limits if valid. + * Used in CONFIG SET and configuration file parsing. */ +static int updateClientOutputBufferLimit(sds *args, int arg_len, const char **err) { + int j; + int class; + unsigned long long hard, soft; + int hard_err, soft_err; + int soft_seconds; + char *soft_seconds_eptr; + clientBufferLimitsConfig values[CLIENT_TYPE_OBUF_COUNT]; + int classes[CLIENT_TYPE_OBUF_COUNT] = {0}; + + /* We need a multiple of 4: */ + if (arg_len % 4) { + if (err) *err = "Wrong number of arguments in " + "buffer limit configuration."; + return C_ERR; + } + + /* Sanity check of single arguments, so that we either refuse the + * whole configuration string or accept it all, even if a single + * error in a single client class is present. */ + for (j = 0; j < arg_len; j += 4) { + class = getClientTypeByName(args[j]); + if (class == -1 || class == CLIENT_TYPE_MASTER) { + if (err) *err = "Invalid client class specified in " + "buffer limit configuration."; + return C_ERR; + } + + hard = memtoull(args[j+1], &hard_err); + soft = memtoull(args[j+2], &soft_err); + soft_seconds = strtoll(args[j+3], &soft_seconds_eptr, 10); + if (hard_err || soft_err || + soft_seconds < 0 || *soft_seconds_eptr != '\0') + { + if (err) *err = "Error in hard, soft or soft_seconds setting in " + "buffer limit configuration."; + return C_ERR; + } + + values[class].hard_limit_bytes = hard; + values[class].soft_limit_bytes = soft; + values[class].soft_limit_seconds = soft_seconds; + classes[class] = 1; + } + + /* Finally set the new config. */ + for (j = 0; j < CLIENT_TYPE_OBUF_COUNT; j++) { + if (classes[j]) server.client_obuf_limits[j] = values[j]; + } + + return C_OK; +} + void initConfigValues() { for (standardConfig *config = configs; config->name != NULL; config++) { config->interface.init(config->data); @@ -550,25 +606,7 @@ void loadServerConfigFromString(char *config) { } else if (!strcasecmp(argv[0],"client-output-buffer-limit") && argc == 5) { - int class = getClientTypeByName(argv[1]); - unsigned long long hard, soft; - int soft_seconds; - - if (class == -1 || class == CLIENT_TYPE_MASTER) { - err = "Unrecognized client limit class: the user specified " - "an invalid one, or 'master' which has no buffer limits."; - goto loaderr; - } - hard = memtoull(argv[2],NULL); - soft = memtoull(argv[3],NULL); - soft_seconds = atoi(argv[4]); - if (soft_seconds < 0) { - err = "Negative number of seconds in soft limit is invalid"; - goto loaderr; - } - server.client_obuf_limits[class].hard_limit_bytes = hard; - server.client_obuf_limits[class].soft_limit_bytes = soft; - server.client_obuf_limits[class].soft_limit_seconds = soft_seconds; + if (updateClientOutputBufferLimit(&argv[1], 4, &err) == C_ERR) goto loaderr; } else if (!strcasecmp(argv[0],"oom-score-adj-values") && argc == 1 + CONFIG_OOM_COUNT) { if (updateOOMScoreAdjValues(&argv[1], &err, 0) == C_ERR) goto loaderr; } else if (!strcasecmp(argv[0],"notify-keyspace-events") && argc == 2) { @@ -760,7 +798,6 @@ void loadServerConfig(char *filename, char config_from_stdin, char *options) { void configSetCommand(client *c) { robj *o; long long ll; - int err; const char *errstr = NULL; serverAssertWithInfo(c,c->argv[2],sdsEncodedObject(c->argv[2])); serverAssertWithInfo(c,c->argv[3],sdsEncodedObject(c->argv[3])); @@ -842,54 +879,14 @@ void configSetCommand(client *c) { return; } } config_set_special_field("client-output-buffer-limit") { - int vlen, j; + int vlen; sds *v = sdssplitlen(o->ptr,sdslen(o->ptr)," ",1,&vlen); - /* We need a multiple of 4: */ - if (vlen % 4) { - sdsfreesplitres(v,vlen); + if (updateClientOutputBufferLimit(v, vlen, &errstr) == C_ERR) { + sdsfreesplitres(v, vlen); goto badfmt; } - /* Sanity check of single arguments, so that we either refuse the - * whole configuration string or accept it all, even if a single - * error in a single client class is present. */ - for (j = 0; j < vlen; j++) { - if ((j % 4) == 0) { - int class = getClientTypeByName(v[j]); - if (class == -1 || class == CLIENT_TYPE_MASTER) - break; - } else if ((j % 4) == 3) { - char *endptr; - long l = strtoll(v[j],&endptr,10); - if (l < 0 || *endptr != '\0') - break; - } else { - memtoull(v[j], &err); - if (err) - break; - } - } - if (j < vlen) { - sdsfreesplitres(v,vlen); - goto badfmt; - } - - /* Finally set the new config */ - for (j = 0; j < vlen; j += 4) { - int class; - unsigned long long hard, soft; - int soft_seconds; - - class = getClientTypeByName(v[j]); - hard = memtoull(v[j+1],NULL); - soft = memtoull(v[j+2],NULL); - soft_seconds = strtoll(v[j+3],NULL,10); - - server.client_obuf_limits[class].hard_limit_bytes = hard; - server.client_obuf_limits[class].soft_limit_bytes = soft; - server.client_obuf_limits[class].soft_limit_seconds = soft_seconds; - } sdsfreesplitres(v,vlen); } config_set_special_field("oom-score-adj-values") { int vlen; diff --git a/tests/unit/obuf-limits.tcl b/tests/unit/obuf-limits.tcl index 1277d6035..bbb1140d6 100644 --- a/tests/unit/obuf-limits.tcl +++ b/tests/unit/obuf-limits.tcl @@ -1,4 +1,32 @@ start_server {tags {"obuf-limits external:skip"}} { + test {CONFIG SET client-output-buffer-limit} { + set oldval [lindex [r config get client-output-buffer-limit] 1] + + catch {r config set client-output-buffer-limit "wrong number"} e + assert_match {*Wrong*arguments*} $e + + catch {r config set client-output-buffer-limit "invalid_class 10mb 10mb 60"} e + assert_match {*Invalid*client*class*} $e + catch {r config set client-output-buffer-limit "master 10mb 10mb 60"} e + assert_match {*Invalid*client*class*} $e + + catch {r config set client-output-buffer-limit "normal 10mbs 10mb 60"} e + assert_match {*Error*hard*} $e + + catch {r config set client-output-buffer-limit "replica 10mb 10mbs 60"} e + assert_match {*Error*soft*} $e + + catch {r config set client-output-buffer-limit "pubsub 10mb 10mb 60s"} e + assert_match {*Error*soft_seconds*} $e + + r config set client-output-buffer-limit "normal 1mb 2mb 60 replica 3mb 4mb 70 pubsub 5mb 6mb 80" + set res [lindex [r config get client-output-buffer-limit] 1] + assert_equal $res "normal 1048576 2097152 60 slave 3145728 4194304 70 pubsub 5242880 6291456 80" + + # Set back to the original value. + r config set client-output-buffer-limit $oldval + } + test {Client output buffer hard limit is enforced} { r config set client-output-buffer-limit {pubsub 100000 0 0} set rd1 [redis_deferring_client]