fix: reject non-STRING arguments in RESP protocol to prevent crash (#5955)

* fix: reject non-STRING arguments in RESP protocol to prevent crash

* fix: review comments

Author: vyavdoshenko

src/facade/dragonfly_connection.cc

@@ -1231,12 +1231,11 @@ Connection::ParserStatus Connection::ParseRedis(unsigned max_busy_cycles) {
     result = redis_parser_->Parse(read_buffer.slice, &consumed, &tmp_parse_args_);
     request_consumed_bytes_ += consumed;
     if (result == RedisParser::OK && !tmp_parse_args_.empty()) {
-      // Check if the first argument is a STRING type. Commands must be strings.
-      // If we get a non-STRING type (e.g., ARRAY from empty array *0), it's a protocol error.
-      if (tmp_parse_args_.front().type != RespExpr::STRING) {
-        LOG(WARNING) << "Invalid command - expected STRING type, got type: "
-                     << tmp_parse_args_.front().type;
-        // Treat this as a parser error
+      // If we get a non-STRING type (e.g., NIL, ARRAY), it's a protocol error.
+      bool valid_input = std::all_of(tmp_parse_args_.begin(), tmp_parse_args_.end(),
+                                     [](const auto& arg) { return arg.type == RespExpr::STRING; });
+      if (!valid_input) {
+        LOG(WARNING) << "Invalid argument - expected all STRING types";
         result = RedisParser::BAD_STRING;
         break;
       }

tests/dragonfly/connection_test.py

@@ -1427,3 +1427,48 @@ async def test_issue_5931_malformed_protocol_crash(df_server: DflyInstance):
     client = df_server.client()
     await client.ping()
     assert await client.ping() == True
+
+
+@dfly_args({})
+async def test_issue_5949_nil_bulk_string_crash(df_server: DflyInstance):
+    """
+    Regression test for #5949
+
+    The crash1.txt and crash2.txt files contain malformed RESP protocol with NIL bulk
+    strings ($-1) as command arguments, which caused the server to crash with:
+    "Check failed: RespExpr::STRING == arg.type" in FromArgs()
+
+    According to RESP protocol spec, NIL bulk strings are valid for server responses
+    but NOT for command arguments sent by clients. Commands must be arrays of bulk strings.
+    """
+    # Open raw TCP connection to send malformed protocol
+    reader, writer = await asyncio.open_connection("127.0.0.1", df_server.port)
+
+    try:
+        # Test crash1.txt: MULTI followed by SET with NIL bulk string argument
+        # *1\r\n$5\r\nMULTI\r\n*3\r\n$3\r\nSET\r\n$1\r\na\r\n$-1\r\n1\r\n*1\r\n$4\r\nEXEC\r\n
+        crash_data = (
+            b"*1\r\n$5\r\nMULTI\r\n*3\r\n$3\r\nSET\r\n$1\r\na\r\n$-1\r\n1\r\n*1\r\n$4\r\nEXEC\r\n"
+        )
+
+        writer.write(crash_data)
+        await writer.drain()
+
+        try:
+            response = await asyncio.wait_for(reader.read(1024), timeout=2.0)
+            # If we get a response, it should be an error, not a crash
+        except asyncio.TimeoutError:
+            # Timeout is acceptable - connection might be closed
+            pass
+        except ConnectionError:
+            # Connection closed is acceptable - server detected bad protocol
+            pass
+
+    finally:
+        writer.close()
+        await writer.wait_closed()
+
+    # Verify server is still running by making a normal request
+    client = df_server.client()
+    await client.ping()
+    assert await client.ping() == True