diff --git a/tines/Cargo.toml b/tines/Cargo.toml index ed10ce4e..762fcd9d 100644 --- a/tines/Cargo.toml +++ b/tines/Cargo.toml @@ -18,8 +18,4 @@ serde = { version = "1", features = ["derive"] } serde_json = "1" [dev-dependencies] -tines-core = { path = "../tines-core", version = "0.3.0" } -tines-store = { path = "../tines-store", version = "0.3.0" } heapless = "0.8" -serde = { version = "1", features = ["derive"] } -serde_json = "1" diff --git a/tines/src/ipc.rs b/tines/src/ipc.rs index 499ec64c..940e55a0 100644 --- a/tines/src/ipc.rs +++ b/tines/src/ipc.rs @@ -199,18 +199,35 @@ where for stream in listener.incoming() { let mut stream = stream?; - // Serve this connection request-by-request until the peer disconnects - // or sends a malformed frame, then move on to the next connection. - while let Ok(req) = read_request(&mut stream) { - let now = now_ms(); - let resp = { - let mut guard = node.lock().unwrap(); - handle(&mut guard, req, now) - }; - // A client that vanished mid-connection shouldn't take down the - // whole service; just drop this one socket. - if write_response(&mut stream, &resp).is_err() { - break; + // Serve this connection request-by-request until the peer cleanly + // disconnects, then move on to the next connection. + loop { + match read_request(&mut stream) { + Ok(req) => { + let now = now_ms(); + let resp = { + // Recover from a poisoned lock so one panicking + // handler can't take down the daemon. + let mut guard = + node.lock().unwrap_or_else(|poisoned| poisoned.into_inner()); + handle(&mut guard, req, now) + }; + if write_response(&mut stream, &resp).is_err() { + // A client that vanished mid-connection shouldn't + // take down the whole service; just drop this socket. + break; + } + } + // A clean disconnect (peer closed the socket) ends the loop. + Err(e) if e.kind() == std::io::ErrorKind::UnexpectedEof => break, + // A malformed frame isn't a disconnect: tell the client and + // keep serving this connection rather than dropping it. The + // frame can't be identified, so the error is generic. + Err(_) => { + if write_response(&mut stream, &error("malformed request")).is_err() { + break; + } + } } } } diff --git a/tines/tests/ipc.rs b/tines/tests/ipc.rs index b53e51a7..42987c94 100644 --- a/tines/tests/ipc.rs +++ b/tines/tests/ipc.rs @@ -313,3 +313,37 @@ fn confidence_event_direct_probe() { assert_eq!(evs[0].id, x); assert_eq!(evs[0].peer, [7; 16]); } + +/// A malformed JSON frame must yield an `Error` response and leave the +/// connection alive, so a following valid request is still served. +#[test] +fn malformed_frame_gets_error_and_connection_survives() { + let dir = unique_dir("malformed"); + let socket = dir.join("ipc.sock"); + let node = build_node([3; 16], &dir); + let serve_path = socket.clone(); + let _handle = thread::spawn(move || serve(node, serve_path).unwrap()); + let mut stream = connect(&socket); + + // A length-prefixed frame whose body is not valid JSON. + let body = b"{ this is not valid json"; + let len = (body.len() as u32).to_le_bytes(); + stream.write_all(&len).unwrap(); + stream.write_all(body).unwrap(); + stream.flush().unwrap(); + + // The server must reply with an Error, not drop the socket. + match read_response(&mut stream) { + Response::Error { .. } => {} + other => panic!("expected Error for malformed frame, got {other:?}"), + } + + // The connection is still alive: a valid request now succeeds. + rpc(&mut stream, &Request::Status); + match read_response(&mut stream) { + Response::Status { .. } => {} + other => panic!("expected Status after a malformed frame, got {other:?}"), + } + + let _ = std::fs::remove_file(&socket); +}