diff --git a/source/extensions/filters/network/common/redis/supported_commands.h b/source/extensions/filters/network/common/redis/supported_commands.h index 478c476eeb9b..970e8cd3f9a0 100644 --- a/source/extensions/filters/network/common/redis/supported_commands.h +++ b/source/extensions/filters/network/common/redis/supported_commands.h @@ -81,6 +81,10 @@ struct SupportedCommands { CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "subscribe", "psubscribe", "unsubscribe", "punsubscribe","quit", "ping"); } + static const absl::flat_hash_set& subcrStateNoArgallowedCommands() { + CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "unsubscribe", "punsubscribe","quit", "ping"); + } + /** * @return commands that make a client to enter subscribed state. */ @@ -113,7 +117,7 @@ struct SupportedCommands { * @return client sub commands thats supported */ static const absl::flat_hash_set& clientSubCommands() { - CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "getname","setname"); + CONSTRUCT_ON_FIRST_USE(absl::flat_hash_set, "getname","setname", "setinfo"); } /** diff --git a/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc b/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc index 3fe578139211..2a3fb517869b 100644 --- a/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc +++ b/source/extensions/filters/network/redis_proxy/command_splitter_impl.cc @@ -936,9 +936,13 @@ SplitRequestPtr PubSubRequest::create(Router& router, Common::Redis::RespValuePt int32_t redisShardsCount=0; if (Common::Redis::SupportedCommands::subscriptionCommands().contains(command_name) && incoming_request->asArray().size() < 2) { - ENVOY_LOG(debug, "Invalid request: '{}'", incoming_request->toString()); - callbacks.onResponse(Common::Redis::Utility::makeError(Response::get().InvalidRequest)); - return nullptr; + if (!Common::Redis::SupportedCommands::subcrStateNoArgallowedCommands().contains(command_name)) { + ENVOY_LOG(debug, "Invalid request: '{}'", incoming_request->toString()); + callbacks.onResponse(Common::Redis::Utility::makeError(Response::get().InvalidRequest)); + return nullptr; + } else { + ENVOY_LOG(debug, "No arguments acceptable for command: '{}'", command_name); + } } @@ -1118,9 +1122,21 @@ void PubSubMessageHandler::handleChannelMessageCustom(Common::Redis::RespValuePt ENVOY_LOG(debug, "message received on channel '{}' on shardIndex : '{}'", value->toString(),shardIndex); if (value->type() != Common::Redis::RespType::Array || - value->asArray()[0].type() != Common::Redis::RespType::BulkString) { - ENVOY_LOG(error, "unexpected message format: '{}'", value->toString()); - return; + value->asArray()[0].type() != Common::Redis::RespType::BulkString) { + // In pubsub flow, commands like subscribe or unsubscribe are dealt with private client and during creation of new private client, Envoy + // sends auth requests along with the actual command, This was verified in packet capture and that is the reason for this check and thereby + // ignoring the OK response in this flow. + // Sample packet capture: + // Command was subscribe a but below is the request and response captured in packet capture + // Request: 55 15.416512 127.0.0.1 127.0.0.1 RESP 122 Request: auth default pwd@123 subscribe a + // Response: 57 15.416650 127.0.0.1 127.0.0.1 RESP 91 Response: OK Array(3) + if (value->type() == Common::Redis::RespType::SimpleString && value->asString() == "OK") { + ENVOY_LOG(debug, "This is a private client creation auth command response - '{}'", value->toString()); + return; + } else { + ENVOY_LOG(error, "unexpected message format: '{}'", value->toString()); + return; + } } std::string message_type = value->asArray()[0].asString(); @@ -2007,6 +2023,9 @@ SplitRequestPtr InstanceImpl::makeRequest(Common::Redis::RespValuePtr&& request, ClientResp->type(Common::Redis::RespType::BulkString); ClientResp->asString() = callbacks.getClientname(); } + } else if (sub_command == "setinfo") { + ClientResp->type(Common::Redis::RespType::SimpleString); + ClientResp->asString() = "OK"; } callbacks.onResponse(std::move(ClientResp)); return nullptr;