DiceDB / dice

DiceDB is a redis-compliant, reactive, scalable, highly-available, unified cache optimized for modern hardware.
https://dicedb.io/
Other
6.79k stars 1.07k forks source link

Inconsistent `SET`: Insertion of escape characters is not handled properly #851

Closed swaingotnochill closed 1 month ago

swaingotnochill commented 1 month ago

Steps to reproduce

> SET key\5 "value5"
OK
> KEYS *
1) "key\\5"

Expected output

1) "key5"

The expected output when the above set of commands (maybe when run on Redis)

REDIS OUTPUT:

redis> SET key\5 "value4"
"OK"
redis> KEYS *
1) "key5"

Observed output

The observed output when the above set of commands when run on DiceDB

127.0.0.1:7379> SET key\5 "value5"
OK
127.0.0.1:7379> KEYS key\5
KEYS will hang redis server, use SCAN instead.
Do you want to proceed? (y/n): y
Your Call!!
(empty list or set)
127.0.0.1:7379> KEYS key5
KEYS will hang redis server, use SCAN instead.
Do you want to proceed? (y/n): y
Your Call!!
(empty list or set)
127.0.0.1:7379> KEYS key*
KEYS will hang redis server, use SCAN instead.
Do you want to proceed? (y/n): y
Your Call!!
1) "key\\5"

Expectations for resolution

This issue will be considered resolved when the following things are done

  1. changes in the dice code to meet the expected behavior
  2. addition of relevant test case to ensure we catch the regression

You can find the tests under the tests directory of the dice repository and the steps to run are in the README file. Refer to the following links to set up DiceDB and Redis 7.2.5 locally

swaingotnochill commented 1 month ago

I can pick this up. Assign it to me.

JyotinderSingh commented 1 month ago

I can pick this up. Assign it to me.

Assigned.

swaingotnochill commented 1 month ago

I was looking into the codebase, and could not find any specific places where the parsing of escape characters are mishandled. I tried running and seems like the expected output(in the description) itself is wrong. Turns out, it is working as expected.

I verified running a latest instance of redis.

redis-15039.c257.us-east-1-3.ec2.redns.redis-cloud.com:15039> SET key\5 "value5"
OK
(0.74s)
redis-15039.c257.us-east-1-3.ec2.redns.redis-cloud.com:15039> keys *
1) "key\\5"

First observation:

  1. 'key\5' is actually stored as "key\5"
  2. 'key\5' is actually stored as 'key\5'

Based on this, lets see how redis behaves:

redis-15039.c257.us-east-1-3.ec2.redns.redis-cloud.com:15039> get key\5
"value5"

redis-15039.c257.us-east-1-3.ec2.redns.redis-cloud.com:15039> keys key\5
(empty array)

redis-15039.c257.us-east-1-3.ec2.redns.redis-cloud.com:15039> keys key\\5
1) "key\\5"

From the above observation, it is clear that the actual key value is 'key\5' but when it is used with 'keys' command, the actual key does not work if passed in pattern.

DiceDB behaves like this:

127.0.0.1:7379> set key\5 "value5"
OK
127.0.0.1:7379> get key\5
"value5"
127.0.0.1:7379> keys key\5
KEYS will hang redis server, use SCAN instead.
Do you want to proceed? (y/n): y
Your Call!!
(empty list or set)
127.0.0.1:7379> keys key\\5
KEYS will hang redis server, use SCAN instead.
Do you want to proceed? (y/n): y
Your Call!!
1) "key\\5"

My inference is an additional "\" is added when escape sequence or forward slash used. And keys need that to specially match it.

Since the behaviour is same, we can safely close the issue.