RedisLabsModules/redismodule-rs

Incorrect Return Type from `Context::call()`

phartleyp opened this issue · 2 comments

The following Redis Module command simply wraps the Redis "GET" command and returns the value of the simple string key with the name "key". It contains a bug for some string values.

pub fn cmd_example(ctx: &Context, _args: Vec<RedisString>) -> RedisResult {
    ctx.call("GET", &["key"])
}
  • For single-line strings, it works fine.
  • For multi-line strings (i.e. those containing \r\n), it will cause the Redis client to fail. On the redis-py client, for example, the client hangs because RESP protocol is not respected.

Root cause is that Context::call() always returns strings as RedisValue::SimpleString. But strings containing \r\n should be RedisValue::BulkString.

This is fixed by pull request #93.

There seems to be some hesitation by maintainers to implement the fix. Until then, here is a workaround that can be applied to the output of Context::call() that will work around this issue:

/// Remap `Context::call()` results to convert `SimpleString` into `BulkString`.
/// All other types are left alone.
pub fn fix_call_reply(result: RedisValue) -> RedisValue {
    match result {
        RedisValue::SimpleString(v) => RedisValue::BulkString(v),
        RedisValue::Array(a) => {
            RedisValue::Array(a.into_iter().map(|v| fix_call_reply(v)).collect())
        }
        v @ _ => v,
    }
}

Related #200