DiceDB / dice

DiceDB is an in-memory, real-time, and reactive database with Redis and SQL support optimized for modern hardware and building real-time applications.
https://dicedb.io/
Other
5.17k stars 665 forks source link

Enforce value to be stored / returned as `json.marshal` in `set` cmd via Http protocol #750

Open vinitparekh17 opened 2 days ago

vinitparekh17 commented 2 days ago

Steps to reproduce

curl --location 'http://localhost:8082/set' \
--header 'Content-Type: application/json' \
--header 'Cache-Control: no-cache' \
--data '{
    "key": "foo",
    "value": {
        "foo": {
            "bar": "fooBar"
        }
    }
}'

curl --location 'http://localhost:8082/get' \
--header 'Content-Type: application/json' \
--header 'Cache-Control: no-cache' \
--data '{
    "key": "foo"
}'

Expected output

"{"foo":{"bar":"fooBar"}}"

Observed output

"map[foo:map[bar:fooBar]]"

Suggestion

Please ensure that the value is properly serialized using json.Marshal before being sent as a response in the HTTP protocol.

Another Solution (Not Recommended, but Supported by Redis)

Redis does allow storing JSON-like values using the SET command. If clients attempt to set a key with a JSON object directly, they will encounter an error due to invalid arguments.

To store JSON values, clients must manually stringify the value before passing it to the SET command.

If a client provides a JSON object without stringifying it, the server should return an error indicating invalid arguments. It's generally advisable to use a JSON.SET for handling JSON data.

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

rishavvajpayee commented 2 days ago

can i take this up ?

TheMonkDev commented 2 days ago

Hey, I am fairly new to this project and I want to take this opportunity to get into contributing to DiceDB. Can I take up this task and return with a PR?

lovish2525 commented 2 days ago

In expected response we need a json response or a json string?

 {
     "foo": {
         "bar": 1
     }
 } 

or

"{"foo":{"bar":"fooBar"}}"

vinitparekh17 commented 1 day ago

In expected response we need a json response or a json string?

 {
     "foo": {
         "bar": 1
     }
 } 

or

"{"foo":{"bar":"fooBar"}}"

strigified response 2nd one

lovish2525 commented 1 day ago

@vinitparekh17 Is it fine if the stringified version has escape characters in response?

lovish2525 commented 1 day ago

Created a PR here - https://github.com/DiceDB/dice/pull/757/files

vinitparekh17 commented 1 day ago

cc: @lucifercr07