Infinite Loop and Parser Panic on TCP Partial Data (Service Unavailable)

[ProjectRbt]

Overview

Two critical stability bugs are found. Both will cause service unavailable directly:

1. Redundant buffer logic leads to infinite connection loop.
2. Missing length check in bulk string parser causes out-of-bounds panic on TCP partial packet.
   Both need immediate fix.

1. Infinite Loop in Connection Handler Due to Redundant Buffer Preprocessing

Severity

Critical (connection stuck, high CPU usage, service completely unavailable)

Location

src/main.rs

Problematic Code

if !buffer.is_empty() {
    process_buffer(&mut buffer, &mut writer, &db, &mut client).await?;
    writer.flush().await?;
    continue;
}

This snippet exists in both subscribed and non-subscribed branch.

Trigger Condition

1. TCP packet fragmentation leaves incomplete partial data in buffer.
2. process_buffer cannot parse incomplete data and does not consume any bytes.
3. The continue statement skips reading new network data and returns to loop start immediately.
4. Repeatedly judge non-empty buffer and run the same logic, resulting in permanent infinite loop.

Root Cause

process_buffer already handles remaining buffer data, TCP sticky packets and fragmented packets natively.
The extra preprocessing plus continue breaks normal stream read flow, causing connection stuck.

Suggestion

Remove both redundant !buffer.is_empty() code blocks entirely.

Fixed Code

Delete the redundant snippet above, keep only normal read and process logic:

async fn handle_connection(stream: TcpStream, db: Db) -> Result<()> {
    let mut buffer = BytesMut::with_capacity(512);
    let mut client = ClientState::new();
    let (mut reader, mut writer) = stream.into_split();

    loop {
        // Check for pubsub messages if subscribed
        if client.is_subscribed() {
            tokio::select! {
                // Wait for pubsub messages
                Some(msg) = client.pubsub_rx.recv() => {
                    let response = Value::Array(vec![
                        Value::BulkString("message".into()),
                        Value::BulkString(msg.channel),
                        Value::BulkString(msg.message),
                    ]);
                    let serialized = resp::serialize(response);
                    writer.write_all(serialized.as_bytes()).await?;
                    writer.flush().await?;
                }
                // Read more data from client
                result = reader.read_buf(&mut buffer) => {
                    match result {
                        Ok(0) => {
                            println!("Connection closed");
                            return Ok(());
                        }
                        Ok(_) => {
                            process_buffer(&mut buffer, &mut writer, &db, &mut client).await?;
                            writer.flush().await?;
                        }
                        Err(e) => return Err(e.into()),
                    }
                }
            }
        } else {
            // Not subscribed - simple read loop
            let n = reader.read_buf(&mut buffer).await?;
            if n == 0 {
                println!("Connection closed");
                return Ok(());
            }
            process_buffer(&mut buffer, &mut writer, &db, &mut client).await?;
        }
    }
}

Panic in RESP Bulk String Parser (Array Index Out of Bounds) on TCP Partial Data

Severity

Critical (worker thread panic, service crash, connection drop)

Location

src/resp/parser.rs
Panic position: slice line in parse_bulk_string

Panic Log

thread 'tokio-runtime-worker' panicked at src\resp\parser.rs:57:51:
range end index 9 out of range for slice of length 4

Problematic Code

Value::BulkString(String::from_utf8(buffer[bytes_consumed..end_of_bulk_str].to_vec())?)

Trigger Condition

1. Client sends standard RESP bulk string command.
2. TCP splits packet, only incomplete partial data arrives.
3. Parser reads declared string length directly without checking actual buffer length.
4. Out-of-bounds slice access triggers runtime panic.

Root Cause

No buffer length validation before slicing. Partial TCP packets will definitely cause index out of bounds and crash the worker thread.

Suggestion

Add total length check before slice access; return error immediately if data is incomplete; properly handle null bulk string.

Fixed Code

fn parse_bulk_string(buffer: BytesMut) -> Result<(Value, usize)> {
    let (bulk_str_len, bytes_consumed) = if let Some((line, len)) = read_until_crlf(&buffer[1..]) {
        let bulk_str_len = parse_int(line)?;
        (bulk_str_len, len + 1)
    } else {
        return Err(anyhow::anyhow!("Invalid bulk string format {:?}", buffer));
    };

    if bulk_str_len == -1 {
        return Ok((Value::NullBulk, bytes_consumed));
    }
    if bulk_str_len < 0 {
        return Err(anyhow::anyhow!("Invalid bulk string length"));
    }

    let bulk_str_len = bulk_str_len as usize;
    let end_of_bulk_str = bytes_consumed + bulk_str_len;
    let total_parsed = end_of_bulk_str + 2;

    if buffer.len() < total_parsed {
        return Err(anyhow::anyhow!("Incomplete bulk string data"));
    }

    let data = &buffer[bytes_consumed..end_of_bulk_str];
    let s = String::from_utf8(data.to_vec())?;

    Ok((Value::BulkString(s), total_parsed))
}

Fix Priority

1. Infinite loop in connection handler — fix first to recover service availability
2. Bulk string parser out-of-bounds panic — fix immediately to avoid service crash