diff --git a/00-RELEASENOTES b/00-RELEASENOTES index 7ce88c556..c6ee44246 100644 --- a/00-RELEASENOTES +++ b/00-RELEASENOTES @@ -11,6 +11,76 @@ CRITICAL: There is a critical bug affecting MOST USERS. Upgrade ASAP. SECURITY: There are security fixes in the release. -------------------------------------------------------------------------------- +================================================================================ +Redis 6.0.5 Released Tue Jun 09 11:56:08 CEST 2020 +================================================================================ + +Upgrade urgency MODERATE: several bugs with moderate impact are fixed here. + +The most important issues are listed here: + +* Fix handling of speical chars in ACL LOAD. +* Make Redis Cluster more robust about operation errors that may lead + to two clusters to mix together. +* Revert the sendfile() implementation of RDB transfer. It causes some delay. +* Fix TLS certificate loading for chained certificates. +* Fix AOF rewirting of KEEPTTL SET option. +* Fix MULTI/EXEC behavior during -BUSY script errors. + +And this is the full list of commits: + +antirez in commit ee8dd01bb: + Temporary fix for #7353 issue about EVAL during -BUSY. + 1 file changed, 9 insertions(+) + +xhe in commit a4a856d53: + return the correct proto version HELLO should return the current proto version, while the code hardcoded 3 + 1 file changed, 1 insertion(+), 1 deletion(-) + +Oran Agra in commit e2046b300: + Don't queue commands in an already aborted MULTI state + 1 file changed, 7 insertions(+) + +Oran Agra in commit b35fdf1de: + Avoid rejecting WATCH / UNWATCH, like MULTI/EXEC/DISCARD + 1 file changed, 4 insertions(+), 2 deletions(-) + +zhaozhao.zz in commit 1d7bf208c: + AOF: append origin SET if no expire option + 2 files changed, 23 insertions(+), 8 deletions(-) + +Oran Agra in commit 676445ad9: + fix disconnectSlaves, to try to free each slave. + 1 file changed, 1 deletion(-) + +zhaozhao.zz in commit 4846c0c8a: + donot free protected client in freeClientsInAsyncFreeQueue + 1 file changed, 9 insertions(+), 3 deletions(-) + +Oran Agra in commit f33de403e: + fix pingoff test race + 1 file changed, 1 insertion(+) + +Kevin Fwu in commit 49af4d07e: + Fix TLS certificate loading for chained certificates. + 1 file changed, 1 insertion(+), 1 deletion(-) + +antirez in commit 329fddbda: + Revert "Implements sendfile for redis." + 2 files changed, 2 insertions(+), 55 deletions(-) + +antirez in commit 925a2cd5a: + Revert "avoid using sendfile if tls-replication is enabled" + 1 file changed, 27 insertions(+), 34 deletions(-) + +Liu Zhen in commit 84a7a9058: + fix clusters mixing accidentally by gossip + 1 file changed, 10 insertions(+), 2 deletions(-) + +antirez in commit cd63359a1: + Fix handling of special chars in ACL LOAD. + 1 file changed, 8 insertions(+), 4 deletions(-) + ================================================================================ Redis 6.0.4 Released Thu May 28 11:36:45 CEST 2020 ================================================================================ diff --git a/src/acl.cpp b/src/acl.cpp index b51dcc295..509dd0776 100644 --- a/src/acl.cpp +++ b/src/acl.cpp @@ -735,10 +735,11 @@ void ACLAddAllowedSubcommand(user *u, unsigned long id, const char *sub) { * EEXIST: You are adding a key pattern after "*" was already added. This is * almost surely an error on the user side. * ENODEV: The password you are trying to remove from the user does not exist. - * EBADMSG: The hash you are trying to add is not a valid hash. + * EBADMSG: The hash you are trying to add is not a valid hash. */ int ACLSetUser(user *u, const char *op, ssize_t oplen) { if (oplen == -1) oplen = strlen(op); + if (oplen == 0) return C_OK; /* Empty string is a no-operation. */ if (!strcasecmp(op,"on")) { u->flags |= USER_FLAG_ENABLED; u->flags &= ~USER_FLAG_DISABLED; @@ -1300,7 +1301,7 @@ sds ACLLoadFromFile(const char *filename) { if (lines[i][0] == '\0') continue; /* Split into arguments */ - argv = sdssplitargs(lines[i],&argc); + argv = sdssplitlen(lines[i],sdslen(lines[i])," ",1,&argc); if (argv == NULL) { errors = sdscatprintf(errors, "%s:%d: unbalanced quotes in acl line. ", @@ -1332,11 +1333,14 @@ sds ACLLoadFromFile(const char *filename) { continue; } - /* Try to process the line using the fake user to validate iif - * the rules are able to apply cleanly. */ + /* Try to process the line using the fake user to validate if + * the rules are able to apply cleanly. At this stage we also + * trim trailing spaces, so that we don't have to handle that + * in ACLSetUser(). */ ACLSetUser(fakeuser,"reset",-1); int j; for (j = 2; j < argc; j++) { + argv[j] = sdstrim(argv[j],"\t\r\n"); if (ACLSetUser(fakeuser,argv[j],sdslen(argv[j])) != C_OK) { const char *errmsg = ACLSetUserStringError(); errors = sdscatprintf(errors, diff --git a/src/aof.cpp b/src/aof.cpp index 9b33caf39..e8d4930b8 100644 --- a/src/aof.cpp +++ b/src/aof.cpp @@ -671,19 +671,23 @@ sds catCommandForAofAndActiveReplication(sds buf, struct redisCommand *cmd, robj } else if (cmd->proc == setCommand && argc > 3) { int i; robj *exarg = NULL, *pxarg = NULL; - /* Translate SET [EX seconds][PX milliseconds] to SET and PEXPIREAT */ - buf = catAppendOnlyGenericCommand(buf,3,argv); for (i = 3; i < argc; i ++) { if (!strcasecmp(szFromObj(argv[i]), "ex")) exarg = argv[i+1]; if (!strcasecmp(szFromObj(argv[i]), "px")) pxarg = argv[i+1]; } serverAssert(!(exarg && pxarg)); - if (exarg) - buf = catAppendOnlyExpireAtCommand(buf,cserver.expireCommand,argv[1], - exarg); - if (pxarg) - buf = catAppendOnlyExpireAtCommand(buf,cserver.pexpireCommand,argv[1], - pxarg); + if (exarg || pxarg) { + /* Translate SET [EX seconds][PX milliseconds] to SET and PEXPIREAT */ + buf = catAppendOnlyGenericCommand(buf,3,argv); + if (exarg) + buf = catAppendOnlyExpireAtCommand(buf,cserver.expireCommand,argv[1], + exarg); + if (pxarg) + buf = catAppendOnlyExpireAtCommand(buf,cserver.pexpireCommand,argv[1], + pxarg); + } else { + buf = catAppendOnlyGenericCommand(buf,argc,argv); + } } else if (cmd->proc == expireMemberCommand || cmd->proc == expireMemberAtCommand || cmd->proc == pexpireMemberAtCommand) { /* Translate subkey expire commands to PEXPIREMEMBERAT */ diff --git a/src/cluster.cpp b/src/cluster.cpp index 15c030162..a91d0d62a 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -1501,7 +1501,10 @@ void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) { } } else { /* If it's not in NOADDR state and we don't have it, we - * start a handshake process against this IP/PORT pairs. + * add it to our trusted dict with exact nodeid and flag. + * Note that we cannot simply start a handshake against + * this IP/PORT pairs, since IP/PORT can be reused already, + * otherwise we risk joining another cluster. * * Note that we require that the sender of this gossip message * is a well known node in our cluster, otherwise we risk @@ -1510,7 +1513,12 @@ void clusterProcessGossipSection(clusterMsg *hdr, clusterLink *link) { !(flags & CLUSTER_NODE_NOADDR) && !clusterBlacklistExists(g->nodename)) { - clusterStartHandshake(g->ip,ntohs(g->port),ntohs(g->cport)); + clusterNode *node; + node = createClusterNode(g->nodename, flags); + memcpy(node->ip,g->ip,NET_IP_STR_LEN); + node->port = ntohs(g->port); + node->cport = ntohs(g->cport); + clusterAddNode(node); } } diff --git a/src/multi.cpp b/src/multi.cpp index ac45eebb9..63cf8f122 100644 --- a/src/multi.cpp +++ b/src/multi.cpp @@ -59,6 +59,13 @@ void queueMultiCommand(client *c) { multiCmd *mc; int j; + /* No sense to waste memory if the transaction is already aborted. + * this is useful in case client sends these in a pipeline, or doesn't + * bother to read previous responses and didn't notice the multi was already + * aborted. */ + if (c->flags & CLIENT_DIRTY_EXEC) + return; + c->mstate.commands = (multiCmd*)zrealloc(c->mstate.commands, sizeof(multiCmd)*(c->mstate.count+1), MALLOC_LOCAL); mc = c->mstate.commands+c->mstate.count; @@ -131,6 +138,15 @@ void execCommand(client *c) { return; } + /* If we are in -BUSY state, flag the transaction and return the + * -BUSY error, like Redis <= 5. This is a temporary fix, may be changed + * ASAP, see issue #7353 on Github. */ + if (g_pserver->lua_timedout) { + flagTransaction(c); + addReply(c, shared.slowscripterr); + return; + } + /* Check if we need to abort the EXEC because: * 1) Some WATCHed key was touched. * 2) There was a previous error while queueing commands. diff --git a/src/networking.cpp b/src/networking.cpp index f54870795..3aab1e62c 100644 --- a/src/networking.cpp +++ b/src/networking.cpp @@ -1645,7 +1645,7 @@ int freeClientsInAsyncFreeQueue(int iel) { while((ln = listNext(&li))) { client *c = (client*)listNodeValue(ln); - if (c->iel == iel) + if (c->iel == iel && !(c->flags & CLIENT_PROTECTED)) { vecclientsFree.push_back(c); listDelNode(g_pserver->clients_to_close, ln); @@ -3060,7 +3060,7 @@ void helloCommand(client *c) { addReplyBulkCString(c,KEYDB_SET_VERSION); addReplyBulkCString(c,"proto"); - addReplyLongLong(c,3); + addReplyLongLong(c,ver); addReplyBulkCString(c,"id"); addReplyLongLong(c,c->id); diff --git a/src/replication.cpp b/src/replication.cpp index c664b6012..2299a7530 100644 --- a/src/replication.cpp +++ b/src/replication.cpp @@ -1370,30 +1370,28 @@ void sendBulkToSlave(connection *conn) { * try to use sendfile system call if supported, unless tls is enabled. * fallback to normal read+write otherwise. */ nwritten = 0; - if (!nwritten) { - ssize_t buflen; - char buf[PROTO_IOBUF_LEN]; + ssize_t buflen; + char buf[PROTO_IOBUF_LEN]; - lseek(replica->repldbfd,replica->repldboff,SEEK_SET); - buflen = read(replica->repldbfd,buf,PROTO_IOBUF_LEN); - if (buflen <= 0) { - serverLog(LL_WARNING,"Read error sending DB to replica: %s", - (buflen == 0) ? "premature EOF" : strerror(errno)); + lseek(replica->repldbfd,replica->repldboff,SEEK_SET); + buflen = read(replica->repldbfd,buf,PROTO_IOBUF_LEN); + if (buflen <= 0) { + serverLog(LL_WARNING,"Read error sending DB to replica: %s", + (buflen == 0) ? "premature EOF" : strerror(errno)); + ul.unlock(); + aeLock.arm(nullptr); + freeClient(replica); + return; + } + if ((nwritten = connWrite(conn,buf,buflen)) == -1) { + if (connGetState(conn) != CONN_STATE_CONNECTED) { + serverLog(LL_WARNING,"Write error sending DB to replica: %s", + connGetLastError(conn)); ul.unlock(); aeLock.arm(nullptr); freeClient(replica); - return; - } - if ((nwritten = connWrite(conn,buf,buflen)) == -1) { - if (connGetState(conn) != CONN_STATE_CONNECTED) { - serverLog(LL_WARNING,"Write error sending DB to replica: %s", - connGetLastError(conn)); - ul.unlock(); - aeLock.arm(nullptr); - freeClient(replica); - } - return; } + return; } replica->repldboff += nwritten; diff --git a/src/server.cpp b/src/server.cpp index c38ca1efa..95b805363 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -813,11 +813,11 @@ struct redisCommand redisCommandTable[] = { 0,NULL,0,0,0,0,0,0}, {"watch",watchCommand,-2, - "no-script fast @transaction", + "no-script fast ok-loading ok-stale @transaction", 0,NULL,1,-1,1,0,0,0}, {"unwatch",unwatchCommand,1, - "no-script fast @transaction", + "no-script fast ok-loading ok-stale @transaction", 0,NULL,0,0,0,0,0,0}, {"cluster",clusterCommand,-2, @@ -3870,6 +3870,8 @@ int processCommand(client *c, int callFlags, AeLocker &locker) { c->cmd->proc != multiCommand && c->cmd->proc != execCommand && c->cmd->proc != discardCommand && + c->cmd->proc != watchCommand && + c->cmd->proc != unwatchCommand && !(c->cmd->proc == shutdownCommand && c->argc == 2 && tolower(((char*)ptrFromObj(c->argv[1]))[0]) == 'n') && diff --git a/src/tls.cpp b/src/tls.cpp index 4284a27bf..25ca0bd31 100644 --- a/src/tls.cpp +++ b/src/tls.cpp @@ -226,7 +226,7 @@ int tlsConfigure(redisTLSContextConfig *ctx_config) { SSL_CTX_set_ecdh_auto(ctx, 1); #endif - if (SSL_CTX_use_certificate_file(ctx, ctx_config->cert_file, SSL_FILETYPE_PEM) <= 0) { + if (SSL_CTX_use_certificate_chain_file(ctx, ctx_config->cert_file) <= 0) { ERR_error_string_n(ERR_get_error(), errbuf, sizeof(errbuf)); serverLog(LL_WARNING, "Failed to load certificate: %s: %s", ctx_config->cert_file, errbuf); goto error; diff --git a/src/tracking.cpp b/src/tracking.cpp index 2bb53e615..2a5afcd15 100644 --- a/src/tracking.cpp +++ b/src/tracking.cpp @@ -101,8 +101,8 @@ void disableTracking(client *c) { /* Set the client 'c' to track the prefix 'prefix'. If the client 'c' is * already registered for the specified prefix, no operation is performed. */ -void enableBcastTrackingForPrefix(client *c, const char *prefix, size_t plen) { - bcastState *bs = (bcastState*)raxFind(PrefixTable,(unsigned char*)prefix,sdslen(prefix)); +static void enableBcastTrackingForPrefix(client *c, const char *prefix, size_t plen) { + bcastState *bs = (bcastState*)raxFind(PrefixTable,(unsigned char*)prefix,plen); /* If this is the first client subscribing to such prefix, create * the prefix in the table. */ if (bs == raxNotFound) { diff --git a/tests/integration/psync2-pingoff.tcl b/tests/integration/psync2-pingoff.tcl index 5a9a46d16..cdecfc5c6 100644 --- a/tests/integration/psync2-pingoff.tcl +++ b/tests/integration/psync2-pingoff.tcl @@ -64,6 +64,7 @@ start_server {} { # make sure replication is still alive and kicking $R(1) incr x wait_for_condition 50 1000 { + [status $R(0) loading] == 0 && [$R(0) get x] == 1 } else { fail "replica didn't get incr" diff --git a/tests/unit/expire.tcl b/tests/unit/expire.tcl index 16006d7da..362c7d8d4 100644 --- a/tests/unit/expire.tcl +++ b/tests/unit/expire.tcl @@ -313,4 +313,14 @@ start_server {tags {"expire"}} { after 3000 assert_equal [r dbsize] 0 } + + test {SET - use KEEPTTL option, TTL should not be removed after loadaof} { + r config set appendonly yes + r set foo bar EX 100 + r set foo bar2 KEEPTTL + after 2000 + r debug loadaof + set ttl [r ttl foo] + assert {$ttl <= 98 && $ttl > 90} + } } diff --git a/tests/unit/multi.tcl b/tests/unit/multi.tcl index 55f18bec8..0c70fbde7 100644 --- a/tests/unit/multi.tcl +++ b/tests/unit/multi.tcl @@ -363,6 +363,9 @@ start_server {tags {"multi"}} { set xx [r get xx] # make sure that either the whole transcation passed or none of it (we actually expect none) assert { $xx == 1 || $xx == 3} + # Discard the transaction since EXEC likely got -BUSY error + # so the client is still in MULTI state. + catch { $rd2 discard ;$rd2 read } e # check that the connection is no longer in multi state $rd2 ping asdf set pong [$rd2 read]