Skip to content

Commit 420d49b

Browse files
committed
fix(terminal): add fd sanity check and handle Disconnected vs Timeout
- PtyWriter::new: add runtime fd >= 0 check after extraction from Box<File> - PtyWriter::new: add compile-time const asserting Box<File> is 8 bytes (thin ptr) - terminal_write: replace recv_timeout map_err with explicit match to distinguish RecvTimeoutError::Timeout ("terminal_write_timeout") from RecvTimeoutError::Disconnected ("terminal_stdin_closed") - PtyWriteRequest.result_tx: switch from std::sync::mpsc::Sender to crossbeam_channel::Sender so the local result_rx can use crossbeam, enabling RecvTimeoutError::Timeout/Disconnected discrimination - Add crossbeam-channel = "0.5" to Cargo.toml
1 parent aa31ee4 commit 420d49b

2 files changed

Lines changed: 30 additions & 10 deletions

File tree

apps/server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ getrandom = "0.2"
1212
libc = "0.2"
1313
notify = "8.2.0"
1414
portable-pty = "0.8"
15+
crossbeam-channel = "0.5"
1516
rusqlite = { version = "0.31", features = ["bundled"] }
1617
serde = { version = "1.0", features = ["derive"] }
1718
serde_json = "1.0"

apps/server/src/services/terminal.rs

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
use crate::app::TerminalIo;
22
use crate::services::utf8_stream::Utf8StreamDecoder;
33
use crate::*;
4+
use crossbeam_channel;
45
use std::collections::BTreeMap;
56
use std::io::Read;
67

8+
// Compile-time sanity: PtyWriter::new reads the fd by casting through Box<dyn Write>
9+
// (fat pointer, 16 bytes on 64-bit) -> data word (Box<File> thin pointer, 8 bytes) ->
10+
// *const Box<File> -> dereference to Box<File> -> cast to i32 (fd at offset 0 in File).
11+
// This assertion guards the Box<File> thin-pointer assumption.
12+
const _: [(); 8] = [(); std::mem::size_of::<Box<std::fs::File>>()];
13+
714
const DEFAULT_PTY_COLS: u16 = 120;
815
const DEFAULT_PTY_ROWS: u16 = 30;
916
const TERMINAL_RUNTIME_OUTPUT_LIMIT: usize = 2 * 1024 * 1024;
@@ -38,6 +45,14 @@ impl PtyWriter {
3845
// Read the fd field from Box<File> (the only field, at offset 0).
3946
let fd: i32 = unsafe { *file_box_ptr.cast::<i32>() };
4047

48+
// Sanity: fd must be non-negative (valid)
49+
if fd < 0 {
50+
return Err(std::io::Error::new(
51+
std::io::ErrorKind::InvalidInput,
52+
format!("PtyWriter: invalid fd {} from writer", fd),
53+
));
54+
}
55+
4156
// Duplicate the fd so both the original Box<File> and PtyWriter own a reference.
4257
let dup_fd = unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) };
4358
if dup_fd < 0 {
@@ -140,7 +155,7 @@ impl Drop for PtyWriter {
140155
/// Request sent to the PTY writer thread to write data.
141156
pub(crate) struct PtyWriteRequest {
142157
pub data: Vec<u8>,
143-
pub result_tx: std::sync::mpsc::Sender<std::io::Result<usize>>,
158+
pub result_tx: crossbeam_channel::Sender<std::io::Result<usize>>,
144159
}
145160

146161
const PTY_WRITE_TIMEOUT_MS: u64 = 2000;
@@ -836,9 +851,9 @@ pub(crate) fn terminal_write(
836851
// Send the write request to the writer thread with a timeout.
837852
// The writer thread owns the PTY writer and handles writes synchronously.
838853
let (result_tx, result_rx): (
839-
std::sync::mpsc::Sender<std::io::Result<usize>>,
840-
std::sync::mpsc::Receiver<std::io::Result<usize>>,
841-
) = std::sync::mpsc::channel();
854+
crossbeam_channel::Sender<std::io::Result<usize>>,
855+
crossbeam_channel::Receiver<std::io::Result<usize>>,
856+
) = crossbeam_channel::bounded(0);
842857
let req = PtyWriteRequest {
843858
data: decorated_input.as_bytes().to_vec(),
844859
result_tx,
@@ -847,12 +862,16 @@ pub(crate) fn terminal_write(
847862
// Writer thread has exited (channel closed)
848863
return Err("terminal_stdin_closed".to_string());
849864
}
850-
let result = result_rx
851-
.recv_timeout(std::time::Duration::from_millis(PTY_WRITE_TIMEOUT_MS))
852-
.map_err(|_| "terminal_write_timeout".to_string())?
853-
.map_err(|e| e.to_string())?;
854-
// result is the number of bytes written; we don't use it
855-
let _ = result;
865+
match result_rx.recv_timeout(std::time::Duration::from_millis(PTY_WRITE_TIMEOUT_MS)) {
866+
Ok(Ok(_)) => {}
867+
Ok(Err(e)) => return Err(e.to_string()),
868+
Err(crossbeam_channel::RecvTimeoutError::Timeout) => {
869+
return Err("terminal_write_timeout".to_string())
870+
}
871+
Err(crossbeam_channel::RecvTimeoutError::Disconnected) => {
872+
return Err("terminal_stdin_closed".to_string())
873+
}
874+
}
856875
}
857876
TerminalIo::TmuxAttached { writer, .. } => {
858877
let mut writer = writer.lock().map_err(|e| e.to_string())?;

0 commit comments

Comments
 (0)