fix(tines): harden IPC against malformed frames and lock poison
serve() now replies with Response::Error on a malformed JSON frame and keeps the connection serving instead of dropping it on the parse error; the per-connection loop only breaks on a clean disconnect (UnexpectedEof). node.lock().unwrap() is replaced with poison-safe into_inner() so one panicking handler can't take down the daemon. Drop duplicate tines-core/tines-store/serde/serde_json from dev-dependencies (only heapless is genuinely test-only).
This commit is contained in:
@@ -18,8 +18,4 @@ serde = { version = "1", features = ["derive"] }
|
|||||||
serde_json = "1"
|
serde_json = "1"
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
tines-core = { path = "../tines-core", version = "0.3.0" }
|
|
||||||
tines-store = { path = "../tines-store", version = "0.3.0" }
|
|
||||||
heapless = "0.8"
|
heapless = "0.8"
|
||||||
serde = { version = "1", features = ["derive"] }
|
|
||||||
serde_json = "1"
|
|
||||||
|
|||||||
+29
-12
@@ -199,18 +199,35 @@ where
|
|||||||
|
|
||||||
for stream in listener.incoming() {
|
for stream in listener.incoming() {
|
||||||
let mut stream = stream?;
|
let mut stream = stream?;
|
||||||
// Serve this connection request-by-request until the peer disconnects
|
// Serve this connection request-by-request until the peer cleanly
|
||||||
// or sends a malformed frame, then move on to the next connection.
|
// disconnects, then move on to the next connection.
|
||||||
while let Ok(req) = read_request(&mut stream) {
|
loop {
|
||||||
let now = now_ms();
|
match read_request(&mut stream) {
|
||||||
let resp = {
|
Ok(req) => {
|
||||||
let mut guard = node.lock().unwrap();
|
let now = now_ms();
|
||||||
handle(&mut guard, req, now)
|
let resp = {
|
||||||
};
|
// Recover from a poisoned lock so one panicking
|
||||||
// A client that vanished mid-connection shouldn't take down the
|
// handler can't take down the daemon.
|
||||||
// whole service; just drop this one socket.
|
let mut guard =
|
||||||
if write_response(&mut stream, &resp).is_err() {
|
node.lock().unwrap_or_else(|poisoned| poisoned.into_inner());
|
||||||
break;
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -313,3 +313,37 @@ fn confidence_event_direct_probe() {
|
|||||||
assert_eq!(evs[0].id, x);
|
assert_eq!(evs[0].id, x);
|
||||||
assert_eq!(evs[0].peer, [7; 16]);
|
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);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user