Trim verbatim type header from bulk string reply

[supercaracal]

Hello,

I'm trying to enable test cases of the hiredis-client driver for the redis-cluster-client. But they always fail.

• https://github.com/redis-rb/redis-cluster-client/pull/60

I found that meta chars of a verbatim type are included in the CLUSTER NODES command reply. I'd say that it should be trimmed.

• https://github.com/antirez/RESP3/blob/master/spec.md

Verbatim string

Normal client libraries may ignore completely the difference between this type and the String type, and return a string in both cases. However interactive clients such as command line interfaces (for instance redis-cli), knows that the output must be presented to the human user as it is, without quoting the string.

The telnet prints txt: header:

$ telnet 127.0.0.1 6379
Trying 127.0.0.1...
Connected to 127.0.0.1.
Escape character is '^]'.
hello 3
%7
$6
server
$5
redis
$7
version
$5
7.0.0
$5
proto
:3
$2
id
:954
$4
mode
$7
cluster
$4
role
$6
master
$7
modules
*0
cluster nodes
=767
txt:787c45a4708e6c564b85068ae315016c6e8d2618 192.168.80.4:6379@16379 myself,master - 0 1655888918000 1 connected 0-5460
d7be37f5e487283634c3ae8a9a45d8cca56094be 192.168.80.5:6379@16379 master - 0 1655888918455 3 connected 10923-16383
a9a8cd6cb41c720f9412bd7ff1479a145e651700 192.168.80.3:6379@16379 slave 2e6fd341161080cbe703e79230e315d0d461f2e9 0 1655888917547 2 connected
35e11c9ba3fe1728f3a9780f0084555e3e956d8a 192.168.80.2:6379@16379 slave 787c45a4708e6c564b85068ae315016c6e8d2618 0 1655888919463 1 connected
9f94a5ec980ac12163a3adc88a82b22794d3fe37 192.168.80.7:6379@16379 slave d7be37f5e487283634c3ae8a9a45d8cca56094be 0 1655888918000 3 connected
2e6fd341161080cbe703e79230e315d0d461f2e9 192.168.80.6:6379@16379 master - 0 1655888919000 2 connected 5461-10922

Also, the current redis-client returns with txt: header:

irb(main):005:0> require 'hiredis-client'
=> true

irb(main):006:0> cli = RedisClient.config.new_client
=> #<RedisClient redis://localhost:6379/0>

irb(main):007:0> cli.call('cluster','nodes').split("\n")
=>
["txt:0b1c4fd1e748c3623d51dca39819f8e0b9d7d724 192.168.96.5:6379@16379 slave 8a418160c83bca1c1c3c7408a486f906a772a54f 0 1655897430000 3 connected",
 "dedbb248592aead36557ae249fc6ea8500bbacc2 192.168.96.6:6379@16379 myself,master - 0 1655897430000 1 connected 0-5460",
 "a7646e9225ac77a7add95d30a6c80e75b24df97d 192.168.96.2:6379@16379 slave dedbb248592aead36557ae249fc6ea8500bbacc2 0 1655897431575 1 connected",
 "d0311a60bd72c7051e2a2ac225f195b265fcf17f 192.168.96.4:6379@16379 slave 7c66848377b7937bc8897c8d2d7c1f6dcb748e06 0 1655897431876 2 connected",
 "8a418160c83bca1c1c3c7408a486f906a772a54f 192.168.96.3:6379@16379 master - 0 1655897430866 3 connected 10923-16383",
 "7c66848377b7937bc8897c8d2d7c1f6dcb748e06 192.168.96.7:6379@16379 master - 0 1655897430000 2 connected 5461-10922"]
irb(main):008:0>

But redis-cli doesn't:

$ docker compose exec node1 redis-cli -c cluster nodes
787c45a4708e6c564b85068ae315016c6e8d2618 192.168.80.4:6379@16379 myself,master - 0 1655891788000 1 connected 0-5460
d7be37f5e487283634c3ae8a9a45d8cca56094be 192.168.80.5:6379@16379 master - 0 1655891788950 3 connected 10923-16383
a9a8cd6cb41c720f9412bd7ff1479a145e651700 192.168.80.3:6379@16379 slave 2e6fd341161080cbe703e79230e315d0d461f2e9 0 1655891788000 2 connected
35e11c9ba3fe1728f3a9780f0084555e3e956d8a 192.168.80.2:6379@16379 slave 787c45a4708e6c564b85068ae315016c6e8d2618 0 1655891787541 1 connected
9f94a5ec980ac12163a3adc88a82b22794d3fe37 192.168.80.7:6379@16379 slave d7be37f5e487283634c3ae8a9a45d8cca56094be 0 1655891787541 3 connected
2e6fd341161080cbe703e79230e315d0d461f2e9 192.168.80.6:6379@16379 master - 0 1655891788000 2 connected 5461-10922

https://github.com/redis/redis/blob/2237131e15c84689f2cd990455111e222f5164f6/src/resp_parser.c#L117-L128

The default driver is legitimate. https://github.com/redis-rb/redis-client/blob/f85e2a8c172a512e5c92dac52e6fed6e1f641d52/lib/redis_client/ruby_connection/resp3.rb#L194-L197 https://github.com/redis-rb/redis-client/blob/f85e2a8c172a512e5c92dac52e6fed6e1f641d52/test/redis_client/resp3_test.rb#L115-L117

[casperisfine]

Hum, curious why it wasn't caught by tests. Can we add one?

[supercaracal]

• [x] Add test cases

[supercaracal]

@casperisfine I've added a test case.

[byroot]

Let's rebase to have a CI run, but LGTM.

[supercaracal]

🎉