Fix crash on dump command (#440)

* Initial plan

* Fix crash on binary data from Redis commands like DUMP

Handle non-UTF-8 binary data by using StringBuffer variant instead of panicking on unwrap.

Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>

* Add defensive error handling for BigNumber and VerbatimString conversions

Apply the same graceful error handling pattern to BigNumber and VerbatimString to prevent potential panics.

Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>

* fix fmt

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: gkorland <753206+gkorland@users.noreply.github.com>

Author: gkorland

examples/call.rs

@@ -156,6 +156,34 @@ fn call_blocking_from_detach_ctx(ctx: &Context, _: Vec<RedisString>) -> RedisRes
     Ok(RedisValue::NoReply)
 }
 
+fn call_dump_test(ctx: &Context, _: Vec<RedisString>) -> RedisResult {
+    // Set a key with a value
+    ctx.call("SET", &["test_dump_key", "test_value"])?;
+
+    // Call DUMP which returns binary data (may not be valid UTF-8)
+    let dump_result = ctx.call("DUMP", &["test_dump_key"])?;
+
+    // Verify we got a result (should be StringBuffer for binary data)
+    match dump_result {
+        RedisValue::StringBuffer(data) => {
+            if data.is_empty() {
+                return Err(RedisError::Str("DUMP returned empty data"));
+            }
+        }
+        RedisValue::SimpleString(_) => {
+            // Also acceptable if the binary data happens to be valid UTF-8
+        }
+        _ => {
+            return Err(RedisError::Str("DUMP returned unexpected type"));
+        }
+    }
+
+    // Clean up
+    ctx.call("DEL", &["test_dump_key"])?;
+
+    Ok("pass".into())
+}
+
 //////////////////////////////////////////////////////
 
 redis_module! {
@@ -167,5 +195,6 @@ redis_module! {
         ["call.test", call_test, "", 0, 0, 0, ""],
         ["call.blocking", call_blocking, "", 0, 0, 0, ""],
         ["call.blocking_from_detached_ctx", call_blocking_from_detach_ctx, "", 0, 0, 0, ""],
+        ["call.dump_test", call_dump_test, "", 0, 0, 0, ""],
     ],
 }

src/redisvalue.rs

@@ -233,7 +233,10 @@ impl<'root> From<&CallReply<'root>> for RedisValue {
                 RedisValue::Array(reply.iter().map(|v| (&v).into()).collect())
             }
             CallReply::I64(reply) => RedisValue::Integer(reply.to_i64()),
-            CallReply::String(reply) => RedisValue::SimpleString(reply.to_string().unwrap()),
+            CallReply::String(reply) => reply
+                .to_string()
+                .map(RedisValue::SimpleString)
+                .unwrap_or_else(|| RedisValue::StringBuffer(reply.as_bytes().to_vec())),
             CallReply::Null(_) => RedisValue::Null,
             CallReply::Map(reply) => RedisValue::Map(
                 reply
@@ -260,10 +263,20 @@ impl<'root> From<&CallReply<'root>> for RedisValue {
             ),
             CallReply::Bool(reply) => RedisValue::Bool(reply.to_bool()),
             CallReply::Double(reply) => RedisValue::Float(reply.to_double()),
-            CallReply::BigNumber(reply) => RedisValue::BigNumber(reply.to_string().unwrap()),
-            CallReply::VerbatimString(reply) => {
-                RedisValue::VerbatimString(reply.to_parts().unwrap())
-            }
+            CallReply::BigNumber(reply) => reply
+                .to_string()
+                .map(RedisValue::BigNumber)
+                .unwrap_or_else(|| {
+                    // BigNumber should always be valid UTF-8, but if not, treat as raw bytes
+                    RedisValue::StaticError("Invalid BigNumber: not valid UTF-8")
+                }),
+            CallReply::VerbatimString(reply) => reply
+                .to_parts()
+                .map(RedisValue::VerbatimString)
+                .unwrap_or_else(|| {
+                    // VerbatimString format should always be valid UTF-8, but if not, treat as error
+                    RedisValue::StaticError("Invalid VerbatimString: format not valid UTF-8")
+                }),
         }
     }
 }

tests/integration.rs

@@ -633,6 +633,23 @@ fn test_call_blocking() -> Result<()> {
     Ok(())
 }
 
+#[test]
+#[cfg(any(
+    feature = "min-redis-compatibility-version-7-4",
+    feature = "min-redis-compatibility-version-7-2"
+))]
+fn test_call_dump() -> Result<()> {
+    let mut con = TestConnection::new("call");
+
+    let res: String = redis::cmd("call.dump_test")
+        .query(&mut con)
+        .with_context(|| "failed to run call.dump_test")?;
+
+    assert_eq!(&res, "pass");
+
+    Ok(())
+}
+
 #[test]
 fn test_open_key_with_flags() -> Result<()> {
     let mut con = TestConnection::new("open_key_with_flags");