diff --git a/apps/solvecraft-cli/src/main.rs b/apps/solvecraft-cli/src/main.rs index 47a9fe97..973657b7 100644 --- a/apps/solvecraft-cli/src/main.rs +++ b/apps/solvecraft-cli/src/main.rs @@ -97,7 +97,9 @@ fn pretty(v: &Value) -> String { } pub fn read_json(path: &str) -> Result { - let s = std::fs::read_to_string(path).map_err(|e| format!("{path}: {e}"))?; + // Through the engine's file access, which refuses anything that is not a regular file and + // reads no more than the cap: a character device or a FIFO would otherwise never end. + let s = solvecraft_engine::io::vfs::read_to_string(path, solvecraft_engine::io::MAX_DESIGN_BYTES).map_err(|e| format!("{path}: {e}"))?; serde_json::from_str(&s).map_err(|e| format!("{path}: {e}")) } @@ -359,13 +361,9 @@ fn cmd_bench_edit(args: &[String]) -> Result<(), String> { fn cmd_bench_open(args: &[String]) -> Result<(), String> { let path = args.first().ok_or("usage: solvecraft-cli bench-open [--parse-only]")?; let t = std::time::Instant::now(); - let len = std::fs::metadata(path).map_err(|e| format!("{path}: {e}"))?.len(); let step = solvecraft_engine::io::is_step_path(path); let limit = if step { solvecraft_engine::io::MAX_STEP_BYTES } else { solvecraft_engine::io::MAX_DESIGN_BYTES }; - if len > limit as u64 { - return Err(format!("{path}: file too large")); - } - let bytes = std::fs::read(path).map_err(|e| format!("{path}: {e}"))?; + let bytes = solvecraft_engine::io::vfs::read(path, limit).map_err(|e| format!("{path}: {e}"))?; let read_ms = t.elapsed().as_secs_f64() * 1000.0; println!("{}", json!({"step": "read", "ms": read_ms, "bytes": bytes.len()})); let t = std::time::Instant::now(); diff --git a/apps/solvecraft-web/src/files.rs b/apps/solvecraft-web/src/files.rs index 8f20438e..41106b99 100644 --- a/apps/solvecraft-web/src/files.rs +++ b/apps/solvecraft-web/src/files.rs @@ -349,7 +349,7 @@ pub fn rename(files: &Rc, from: &str, to: &str) -> Result<(), String> { if files.entries.borrow().iter().any(|e| e.name == to) { return Err(format!("there is already a file named {to}")); } - let bytes = vfs::read(&format!("{OPFS}{from}")).map_err(|e| e.to_string())?; + let bytes = vfs::read(&format!("{OPFS}{from}"), solvecraft_engine::io::MAX_DESIGN_BYTES).map_err(|e| e.to_string())?; vfs::write(&format!("{OPFS}{to}"), &bytes).map_err(|e| e.to_string())?; delete(files, from); flush(files); diff --git a/apps/solvecraft-web/src/main.rs b/apps/solvecraft-web/src/main.rs index a62b041b..28a3f1fd 100644 --- a/apps/solvecraft-web/src/main.rs +++ b/apps/solvecraft-web/src/main.rs @@ -234,7 +234,7 @@ mod web { self.app.insert_path(&path); self.files.panel.set(false); } - "download" => match vfs::read(&path) { + "download" => match vfs::read(&path, solvecraft_engine::io::MAX_DESIGN_BYTES) { Ok(b) => { if let Err(e) = files::download(&name, &b) { self.app.set_status(e, true); diff --git a/apps/solvecraft/src/control_server.rs b/apps/solvecraft/src/control_server.rs index 530e19a2..834818d3 100644 --- a/apps/solvecraft/src/control_server.rs +++ b/apps/solvecraft/src/control_server.rs @@ -3,12 +3,22 @@ use std::io::{BufRead, BufReader, Write}; use std::net::{TcpListener, TcpStream}; +use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::mpsc::{Receiver, Sender, channel}; use std::time::Duration; use serde_json::{Value, json}; +use solvecraft_engine::MAX_LINE; use solvecraft_ui_egui::ControlRequest; +/// Connections served at once. The channel has no authentication beyond being bound to +/// loopback, so a flood must not be able to spawn threads without end. +const MAX_CONNECTIONS: usize = 16; +/// Lines that are not JSON before the connection is closed, so garbage cannot loop forever. +const MAX_BAD_LINES: usize = 8; + +static OPEN: AtomicUsize = AtomicUsize::new(0); + pub fn start(port: u16, ctx: egui::Context) -> Receiver { let (tx, rx) = channel::(); let listener = match TcpListener::bind(("127.0.0.1", port)) { @@ -21,19 +31,69 @@ pub fn start(port: u16, ctx: egui::Context) -> Receiver { eprintln!("solvecraft: control server listening on 127.0.0.1:{port}"); std::thread::spawn(move || { for stream in listener.incoming().flatten() { + if OPEN.fetch_add(1, Ordering::Relaxed) >= MAX_CONNECTIONS { + OPEN.fetch_sub(1, Ordering::Relaxed); + let mut refused = stream; + let _ = writeln!(refused, "{}", json!({"ok": false, "error": "too many connections"})); + continue; + } let tx = tx.clone(); let ctx = ctx.clone(); - std::thread::spawn(move || serve(stream, tx, ctx)); + std::thread::spawn(move || { + serve(stream, tx, ctx); + OPEN.fetch_sub(1, Ordering::Relaxed); + }); } }); rx } +/// One line without its newline, or `None` at the end of the stream. A client that sends +/// gigabytes with no newline would otherwise make this thread allocate until the process is +/// killed, so the line is refused as soon as it passes `max`. +fn read_line(r: &mut impl BufRead, max: usize) -> std::io::Result>> { + let mut line = Vec::new(); + loop { + let (done, used) = { + let available = r.fill_buf()?; + if available.is_empty() { + return Ok((!line.is_empty()).then_some(line)); + } + match available.iter().position(|b| *b == b'\n') { + Some(i) => { + line.extend_from_slice(available.get(..i).unwrap_or(&[])); + (true, i + 1) + } + None => { + line.extend_from_slice(available); + (false, available.len()) + } + } + }; + r.consume(used); + if done { + return Ok(Some(line)); + } + if line.len() > max { + return Err(std::io::Error::new(std::io::ErrorKind::InvalidData, "request too large")); + } + } +} + fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { let Ok(read) = stream.try_clone() else { return }; let mut out = stream; - for line in BufReader::new(read).lines() { - let Ok(line) = line else { break }; + let mut reader = BufReader::new(read); + let mut bad = 0usize; + loop { + let line = match read_line(&mut reader, MAX_LINE) { + Ok(Some(l)) => String::from_utf8_lossy(&l).into_owned(), + Ok(None) => break, + Err(e) => { + let _ = writeln!(out, "{}", json!({"ok": false, "error": e.to_string()})); + break; + } + }; if line.trim().is_empty() { continue; } @@ -53,10 +113,58 @@ fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { } r } - Err(e) => json!({"ok": false, "error": format!("bad JSON: {e}")}), + Err(e) => { + bad += 1; + json!({"ok": false, "error": format!("bad JSON: {e}")}) + } }; if writeln!(out, "{reply}").is_err() { break; } + if bad >= MAX_BAD_LINES { + break; + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A request longer than the cap is refused instead of allocated. + #[test] + fn lines_past_the_cap_are_refused() { + let flood = vec![b'A'; MAX_LINE + 2]; + let mut r = BufReader::new(flood.as_slice()); + let e = read_line(&mut r, MAX_LINE).unwrap_err(); + assert_eq!(e.kind(), std::io::ErrorKind::InvalidData); + assert!(e.to_string().contains("request too large"), "{e}"); + } + + /// Ordinary framing: lines split on newlines, blank lines kept, no trailing newline needed. + #[test] + fn lines_split_on_newlines() { + let text = b"{\"a\":1}\n\n{\"b\":2}\nlast".to_vec(); + let mut r = BufReader::new(text.as_slice()); + let mut got = Vec::new(); + while let Some(line) = read_line(&mut r, MAX_LINE).unwrap() { + got.push(String::from_utf8(line).unwrap()); + } + assert_eq!(got, vec!["{\"a\":1}", "", "{\"b\":2}", "last"]); + assert!(read_line(&mut r, MAX_LINE).unwrap().is_none()); + } + + /// A line that spans several reads of the underlying stream still arrives whole. + #[test] + fn lines_longer_than_one_buffer_arrive_whole() { + let long = "x".repeat(200_000); + let text = format!("{long}\n{long}\n").into_bytes(); + let mut r = BufReader::with_capacity(64, text.as_slice()); + assert_eq!(read_line(&mut r, MAX_LINE).unwrap().unwrap().len(), long.len()); + assert_eq!(read_line(&mut r, MAX_LINE).unwrap().unwrap().len(), long.len()); + assert!(read_line(&mut r, MAX_LINE).unwrap().is_none()); + // The same line against a smaller cap is refused. + let mut r = BufReader::with_capacity(64, text.as_slice()); + assert!(read_line(&mut r, 1000).is_err()); } } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 21bf8fbb..a3ada5e7 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -51,11 +51,7 @@ fn path_arg<'a>(p: &'a Value, cmd: &str) -> Result<&'a str> { /// Read a STEP file as an Import feature. fn read_step(path: &str) -> Result { - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta > solvecraft_io::MAX_STEP_BYTES as u64 { - return Err(EngineError::Other(format!("{path}: file too large"))); - } - let bytes = solvecraft_io::vfs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = solvecraft_io::vfs::read(path, solvecraft_io::MAX_STEP_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; solvecraft_io::step_import_feature(&bytes, path).map_err(|e| EngineError::Other(format!("{path}: {e}"))) } @@ -80,11 +76,7 @@ fn add_step(s: &mut Session, path: &str, name: Option<&str>) -> Result { /// Add a MeshImport feature for a 3MF or STL file. fn add_mesh(s: &mut Session, path: &str, name: Option<&str>) -> Result { - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta > solvecraft_io::MAX_3MF_BYTES as u64 { - return Err(EngineError::Other(format!("{path}: file too large"))); - } - let bytes = solvecraft_io::vfs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = solvecraft_io::vfs::read(path, solvecraft_io::MAX_3MF_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let mf = solvecraft_io::mesh_import_feature(&bytes, path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let fname = name.filter(|n| !n.trim().is_empty()).unwrap_or(&mf.name).to_string(); let id = s.doc_mut().add_feature(mf.kind, Some(&fname))?; @@ -132,11 +124,7 @@ fn open(s: &mut Session, p: &Value) -> Result { let errors = s.model.results.iter().filter(|r| r.error.is_some()).count(); return Ok(json!({"path": path, "features": s.doc.features.len(), "errors": errors, "bodies": r["bodies"], "warnings": r["warnings"]})); } - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta as usize > solvecraft_io::MAX_DESIGN_BYTES { - return Err(EngineError::Other(format!("{path}: file too large"))); - } - let bytes = solvecraft_io::vfs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = solvecraft_io::vfs::read(path, solvecraft_io::MAX_DESIGN_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let doc = solvecraft_io::read_design(&bytes)?; *s = Session::new(doc); s.path = Some(path.to_string()); diff --git a/crates/engine/src/cmd/parameters.rs b/crates/engine/src/cmd/parameters.rs index 5437627f..73a48a81 100644 --- a/crates/engine/src/cmd/parameters.rs +++ b/crates/engine/src/cmd/parameters.rs @@ -226,16 +226,15 @@ fn export(s: &mut Session, p: &Value) -> Result { Ok(json!({"format": format, "path": path, "text": text})) } +/// Largest parameter table we read from a file. +const MAX_PARAMETER_BYTES: usize = 16 << 20; + fn import(s: &mut Session, p: &Value) -> Result { let cmd = "parameters.import"; let text = match (str_(p, "text"), str_(p, "path")) { (Some(t), _) => t.to_string(), (None, Some(path)) => { - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta > 16 << 20 { - return Err(bad(cmd, "file too large")); - } - solvecraft_io::vfs::read_to_string(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))? + solvecraft_io::vfs::read_to_string(path, MAX_PARAMETER_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))? } _ => return Err(bad(cmd, "give `text` or `path`")), }; diff --git a/crates/engine/src/cmd/sketch_canvas.rs b/crates/engine/src/cmd/sketch_canvas.rs index 8c6061f0..3b34abf1 100644 --- a/crates/engine/src/cmd/sketch_canvas.rs +++ b/crates/engine/src/cmd/sketch_canvas.rs @@ -31,11 +31,7 @@ fn image_bytes(p: &Value, cmd: &str) -> Result> { base64_decode(d).ok_or_else(|| bad(cmd, "`data` is not base64"))? } else { let path = str_(p, "path").filter(|x| !x.trim().is_empty() && x.len() < 4096).ok_or_else(|| bad(cmd, "give `path` or `data`"))?; - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta as usize > MAX_CANVAS_BYTES { - return Err(bad(cmd, "the image is too large")); - } - solvecraft_io::vfs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))? + solvecraft_io::vfs::read(path, MAX_CANVAS_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))? }; if b.len() > MAX_CANVAS_BYTES { return Err(bad(cmd, "the image is too large")); diff --git a/crates/engine/src/cmd/sketch_import.rs b/crates/engine/src/cmd/sketch_import.rs index e776852a..cbb9046a 100644 --- a/crates/engine/src/cmd/sketch_import.rs +++ b/crates/engine/src/cmd/sketch_import.rs @@ -34,11 +34,7 @@ fn source_text(p: &Value, cmd: &str) -> Result { return Ok(t.to_string()); } let path = str_(p, "path").filter(|x| !x.trim().is_empty() && x.len() < 4096).ok_or_else(|| bad(cmd, "give `path` or `text`"))?; - let meta = solvecraft_io::vfs::len(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; - if meta as usize > MAX_DRAWING_BYTES { - return Err(EngineError::Other(format!("{path}: file too large"))); - } - let bytes = solvecraft_io::vfs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; + let bytes = solvecraft_io::vfs::read(path, MAX_DRAWING_BYTES).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; Ok(String::from_utf8_lossy(&bytes).to_string()) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 59c72e92..bccd68ab 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -33,6 +33,10 @@ pub use solvecraft_kernel as kernel; pub use solvecraft_render as render; pub use solvecraft_sketch as sketch; +/// Longest accepted request line on the JSON-lines channels (MCP over stdio and the control +/// server over loopback), so both refuse runaway input at the same size. +pub const MAX_LINE: usize = 16 << 20; + #[derive(Debug, thiserror::Error)] pub enum EngineError { #[error("unknown command `{0}`")] diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index 9f316c55..5eeea0ed 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -100,7 +100,9 @@ pub fn load(dir: &Path, id: &str) -> Result<(Document, Entry)> { .ok() .and_then(|b| serde_json::from_slice(&b).ok()) .ok_or_else(|| EngineError::Other(format!("no recovered design `{id}`")))?; - let bytes = fs::read(file(dir, id, "solvecraft")).map_err(|e| EngineError::Other(format!("recovered design `{id}`: {e}")))?; + let path = file(dir, id, "solvecraft"); + let bytes = solvecraft_io::vfs::read(&path.to_string_lossy(), solvecraft_io::MAX_DESIGN_BYTES) + .map_err(|e| EngineError::Other(format!("recovered design `{id}`: {e}")))?; Ok((solvecraft_io::read_design(&bytes)?, entry)) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e087b067..f27e3947 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2255,3 +2255,29 @@ fn adopting_a_computed_component_keeps_its_warm_world_geometry() { s.invalidate_view(100); assert!(std::sync::Arc::ptr_eq(&prepared, &s.world_state())); } + +/// Every file command checked the size with the metadata and then read to the end. A character +/// device reports a length of 0, so the cap passed and the read never finished; a FIFO blocked +/// the calling thread, which over the control channel is the thread drawing the window. +#[cfg(unix)] +#[test] +fn special_files_are_refused_by_every_file_command() { + let mut s = Session::default(); + for cmd in ["doc.open", "parameters.import"] { + let r = s.execute(cmd, &json!({"path": "/dev/zero"})); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("not a regular file")), "{cmd}: {r:?}"); + } + // The commands that check the extension first need a special file with the right name. + let dir = std::env::temp_dir().join(format!("solvecraft-special-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + for (cmd, name) in [("file.insert_step", "x.step"), ("file.insert_mesh", "x.3mf"), ("sketch.insert_dxf", "x.dxf")] { + let p = dir.join(name); + let _ = std::fs::remove_file(&p); + if !std::process::Command::new("mkfifo").arg(&p).status().map(|st| st.success()).unwrap_or(false) { + continue; + } + let r = s.execute(cmd, &json!({"path": p.to_string_lossy()})); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("not a regular file")), "{cmd}: {r:?}"); + } + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/io/src/vfs.rs b/crates/io/src/vfs.rs index 839a246d..b1c2c343 100644 --- a/crates/io/src/vfs.rs +++ b/crates/io/src/vfs.rs @@ -9,8 +9,30 @@ use std::io::{Error, ErrorKind, Result}; mod imp { use super::*; - pub fn read(path: &str) -> Result> { - std::fs::read(path) + /// At most `max` bytes of a regular file. A character device such as `/dev/zero` reports a + /// length of 0, so checking the metadata and then reading to the end is unbounded; a FIFO + /// blocks the caller instead. Refuse anything that is not a regular file, and never read + /// more than the caller's cap plus the byte that proves it was exceeded. + pub fn read(path: &str, max: usize) -> Result> { + // Cheap and non-blocking: refuse before opening, in case the open itself would block. + let meta = std::fs::metadata(path)?; + if !meta.is_file() { + return Err(Error::new(ErrorKind::InvalidInput, "not a regular file")); + } + if meta.len() > max as u64 { + return Err(Error::new(ErrorKind::InvalidData, "file too large")); + } + let f = std::fs::File::open(path)?; + // The path may have been swapped between the two calls; the handle decides. + if !f.metadata()?.is_file() { + return Err(Error::new(ErrorKind::InvalidInput, "not a regular file")); + } + let mut out = Vec::with_capacity(meta.len().min(16 << 20) as usize); + std::io::Read::read_to_end(&mut std::io::Read::take(f, max as u64 + 1), &mut out)?; + if out.len() > max { + return Err(Error::new(ErrorKind::InvalidData, "file too large")); + } + Ok(out) } pub fn write(path: &str, bytes: &[u8]) -> Result<()> { std::fs::write(path, bytes) @@ -42,8 +64,13 @@ mod imp { Error::new(ErrorKind::NotFound, format!("{path}: no such file in this browser")) } - pub fn read(path: &str) -> Result> { - FILES.with(|f| f.borrow().get(path).cloned()).ok_or_else(|| missing(path)) + /// At most `max` bytes, to match the native signature. + pub fn read(path: &str, max: usize) -> Result> { + let bytes = FILES.with(|f| f.borrow().get(path).cloned()).ok_or_else(|| missing(path))?; + if bytes.len() > max { + return Err(Error::new(ErrorKind::InvalidData, "file too large")); + } + Ok(bytes) } pub fn write(path: &str, bytes: &[u8]) -> Result<()> { FILES.with(|f| f.borrow_mut().insert(path.to_string(), bytes.to_vec())); @@ -77,9 +104,9 @@ mod imp { pub use imp::*; -/// A file's text. -pub fn read_to_string(path: &str) -> Result { - String::from_utf8(read(path)?).map_err(|e| Error::new(ErrorKind::InvalidData, e)) +/// A file's text, at most `max` bytes. +pub fn read_to_string(path: &str, max: usize) -> Result { + String::from_utf8(read(path, max)?).map_err(|e| Error::new(ErrorKind::InvalidData, e)) } #[cfg(test)] @@ -90,13 +117,50 @@ mod tests { let _ = std::fs::create_dir_all(&dir); let p = dir.join("a.txt").to_string_lossy().to_string(); assert!(!super::exists(&p)); - assert!(super::read(&p).is_err()); + assert!(super::read(&p, 16).is_err()); super::write(&p, b"hello").unwrap(); assert!(super::exists(&p)); assert_eq!(super::len(&p).unwrap(), 5); - assert_eq!(super::read_to_string(&p).unwrap(), "hello"); + assert_eq!(super::read_to_string(&p, 16).unwrap(), "hello"); + // The cap is the caller's, not the file's. + assert!(super::read(&p, 4).is_err()); + assert_eq!(super::read(&p, 5).unwrap(), b"hello"); super::remove(&p).unwrap(); assert!(!super::exists(&p)); let _ = std::fs::remove_dir_all(&dir); } + + /// `metadata().len()` is 0 for a character device and for a FIFO, so a cap checked against + /// it passes and the read then runs to an end that never comes. Refuse what is not a + /// regular file, and do it before opening: opening a FIFO blocks until a writer appears. + #[cfg(all(unix, not(target_arch = "wasm32")))] + #[test] + fn special_files_are_refused_without_blocking() { + let e = super::read("/dev/zero", 1024).unwrap_err(); + assert!(e.to_string().contains("not a regular file"), "{e}"); + assert!(super::read_to_string("/dev/zero", 1024).is_err()); + + let dir = std::env::temp_dir().join(format!("solvecraft-vfs-fifo-{}", std::process::id())); + let _ = std::fs::create_dir_all(&dir); + let fifo = dir.join("p"); + let made = std::process::Command::new("mkfifo").arg(&fifo).status().map(|s| s.success()).unwrap_or(false); + if made { + let path = fifo.to_string_lossy().to_string(); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(super::read(&path, 1024).is_err()); + }); + let refused = rx.recv_timeout(std::time::Duration::from_secs(5)); + assert_eq!(refused, Ok(true), "reading a FIFO must be refused, not block"); + } + let _ = std::fs::remove_dir_all(&dir); + } + + /// A directory is not a file either. + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn directories_are_not_files() { + let dir = std::env::temp_dir().to_string_lossy().to_string(); + assert!(super::read(&dir, 1024).is_err()); + } } diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs index f9b01d6b..87223b27 100644 --- a/crates/mcp/src/server.rs +++ b/crates/mcp/src/server.rs @@ -3,6 +3,8 @@ use std::io::{BufRead, Write}; use serde_json::{Value, json}; +/// Longest accepted request line, shared with the control server. +use solvecraft_engine::MAX_LINE; use crate::backend::Backend; use crate::tools::{call_tool, check_arguments, tool_definitions}; @@ -18,8 +20,6 @@ const INVALID_PARAMS: i64 = -32602; const INTERNAL_ERROR: i64 = -32603; const RESOURCE_NOT_FOUND: i64 = -32002; -/// Longest accepted request line (a guard against runaway input). -const MAX_LINE: usize = 16 << 20; /// Most messages in one JSON-RPC batch. const MAX_BATCH: usize = 1000; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index dd9767b5..abd728e9 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -921,9 +921,16 @@ impl SolveApp { } fn drain_control(&mut self, ctx: &egui::Context) { + /// Requests run on this thread, so a flood would starve rendering. The rest stay + /// queued for the next frame. + const PER_FRAME: usize = 64; + let Some(rx) = self.control_rx.take() else { return }; // While the agent cursor acts out a request, later ones wait their turn. - while !self.agent.busy() { + for _ in 0..PER_FRAME { + if self.agent.busy() { + break; + } let Ok(req) = rx.try_recv() else { break }; let reply = req.reply.clone(); if agent_cursor::intercept(self, &req.method, &req.params, &reply) { diff --git a/docs/control-protocol.md b/docs/control-protocol.md index de6ff528..8e187ef7 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -6,6 +6,14 @@ line `{"id": …, "ok": true, "result": …}` or `{"id": …, "ok": false, "erro handled on the UI thread between frames; pointer and keyboard input is injected as real egui events, one pointer event per frame. +The channel is untrusted input. A request line may be at most 16 MiB; a longer one is answered +with `{"ok": false, "error": "request too large"}` and the connection is closed, as are +connections that send eight lines that are not JSON. At most 16 connections are served at once +(further ones get `{"ok": false, "error": "too many connections"}`), at most 64 queued requests +run per frame so a flood cannot starve drawing, and a request that the UI thread has not +answered within 60 seconds is replied to with `{"ok": false, "error": "timeout"}`. The same +16 MiB line limit applies to the MCP server. + ## Engine | Method | Params | Result |