From 3f6bd4fc2543975da76112e45601d4f5058a5176 Mon Sep 17 00:00:00 2001 From: Chen Tianjie Date: Thu, 21 Sep 2023 23:41:32 +0800 Subject: [PATCH] Use server.current_client to decide whether cluster commands should return TLS info. (#12569) Starting a change in #12233 (released in 7.2), CLUSTER commands use client's connection to decide whether to return TLS port or non-TLS port, but commands called by Lua script and module's RM_Call don't have a real client with connection, and would currently be regarded as non-TLS connections. We can use server.current_client instead when it is available. When it is not (module calls commands without a real client), we may see this as an undefined behavior, and return null or default port (currently in this PR it returns default port, judged by server.tls_cluster). (cherry picked from commit 2aad03fa399f520febb8b7db972459bc97484104) --- src/cluster.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index 27d109914..9cfa99a32 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -119,6 +119,20 @@ static inline int defaultClientPort(void) { return server.tls_cluster ? server.tls_port : server.port; } +/* When a cluster command is called, we need to decide whether to return TLS info or + * non-TLS info by the client's connection type. However if the command is called by + * a Lua script or RM_call, there is no connection in the fake client, so we use + * server.current_client here to get the real client if available. And if it is not + * available (modules may call commands without a real client), we return the default + * info, which is determined by server.tls_cluster. */ +static int shouldReturnTlsInfo(void) { + if (server.current_client && server.current_client->conn) { + return connIsTLS(server.current_client->conn); + } else { + return server.tls_cluster; + } +} + /* Links to the next and previous entries for keys in the same slot are stored * in the dict entry metadata. See Slot to Key API below. */ #define dictEntryNextInSlot(de) \ @@ -5570,7 +5584,7 @@ void addNodeToNodeReply(client *c, clusterNode *node) { } /* Report TLS ports to TLS client, and report non-TLS port to non-TLS client. */ - addReplyLongLong(c, getNodeClientPort(node, connIsTLS(c->conn))); + addReplyLongLong(c, getNodeClientPort(node, shouldReturnTlsInfo())); addReplyBulkCBuffer(c, node->name, CLUSTER_NAMELEN); /* Add the additional endpoint information, this is all the known networking information @@ -5946,7 +5960,7 @@ NULL } else if (!strcasecmp(c->argv[1]->ptr,"nodes") && c->argc == 2) { /* CLUSTER NODES */ /* Report TLS ports to TLS client, and report non-TLS port to non-TLS client. */ - sds nodes = clusterGenNodesDescription(c, 0, connIsTLS(c->conn)); + sds nodes = clusterGenNodesDescription(c, 0, shouldReturnTlsInfo()); addReplyVerbatim(c,nodes,sdslen(nodes),"txt"); sdsfree(nodes); } else if (!strcasecmp(c->argv[1]->ptr,"myid") && c->argc == 2) { @@ -6312,7 +6326,7 @@ NULL /* Report TLS ports to TLS client, and report non-TLS port to non-TLS client. */ addReplyArrayLen(c,n->numslaves); for (j = 0; j < n->numslaves; j++) { - sds ni = clusterGenNodeDescription(c, n->slaves[j], connIsTLS(c->conn)); + sds ni = clusterGenNodeDescription(c, n->slaves[j], shouldReturnTlsInfo()); addReplyBulkCString(c,ni); sdsfree(ni); } @@ -7438,7 +7452,7 @@ void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_co error_code == CLUSTER_REDIR_ASK) { /* Report TLS ports to TLS client, and report non-TLS port to non-TLS client. */ - int port = getNodeClientPort(n, connIsTLS(c->conn)); + int port = getNodeClientPort(n, shouldReturnTlsInfo()); addReplyErrorSds(c,sdscatprintf(sdsempty(), "-%s %d %s:%d", (error_code == CLUSTER_REDIR_ASK) ? "ASK" : "MOVED",