search

informatikr / hedis

A Redis client library for Haskell.
http://hackage.haskell.org/package/hedis
BSD 3-Clause "New" or "Revised" License
329 stars 127 forks source link

Prevent most Redis commands when subscribed to PubSub channel #219

Open 414owen opened 10 months ago

414owen commented 10 months ago

I spent a long time tracking down a heisenbug recently, that I'm fairly confident comes down to the fact that you're not allowed to use most redis commands if you're subscribed to a pub/sub channel.

The docs say:

Once the client enters the subscribed state it is not supposed to issue any other commands, except for additional SUBSCRIBE, SSUBSCRIBE, PSUBSCRIBE, UNSUBSCRIBE, SUNSUBSCRIBE, PUNSUBSCRIBE, PING, RESET and QUIT commands. However, if RESP3 is used (see HELLO) it is possible for a client to issue any commands while in subscribed state.

Even if it didn't cause my specific bug, I think that it would be great for hedis to prevent this misuse of redis, preferably in the type system.

This limitation no longer applies to RESP3.

qnikst commented 10 months ago

Hello, thanks! I think we can do it, for example by introducing RedisPubSub context and using it instead of Redis, then we can allow only the allowed functions inside. First of all let me thank you for rising this issue!

It will change the API, but we next version will be a major one anyways, so it should not be a problem.

At this point I don't see any potential drawback in this solution.

Just a question, would you like to lead this work? I'm not sure how much time will I able to spend on this, also I don't have a good use-case scenario where I can test the approach and I'm afraid that small tests and artificial cases may not work well to validate the sanity of the implementation. I not I'll try to spend some time on a weekend.

414owen commented 10 months ago

Yes, I'm happy to lead the work on this.
I'm already in the process of implementing it, as part of my RESP3 work.
With RESP3, one can run both pub/sub and req/reply over some types of connections...
~My idea was to parameterize the operations one can perform over the connection type.~ (tbd)

One should be able to run redis request/reply commands over a pipelined connection, but not pub/sub. One should be able to run both pub/sub and req/rep over a non-pipelined connection. One should be able to run both pub/sub and req/rep over a combination of one pipelined connection and one not pipelined connection.

Adding the different types of connections is a bit unfortunate, but I think all the variations have a genuine use case.

qnikst commented 10 months ago

While I find the work magnificent, I'm not very easy on the massive change to the types in the library, so I want to spend some amount of time on understanding if this is necessary or if we can find some middle ground.

Unfortunately I'll do it only on weekend at best, so please sorry me for not spending as much attention to your work as it deserves.