Actions: loading an action set checks its size before reading it

Load Actions… read the whole file before checking the 8 MiB limit, so
picking a very large file could take a lot of memory before it was
refused. A new read_head service reads at most a given number of bytes;
the desktop app provides it, and loading an action set reads one byte
past 8 MiB with it. A larger file is refused without reading the rest.
Hosts without the service keep reading the whole file. Automation
roots apply to read_head as they do to read.
This commit is contained in:
dotCooCoo
2026-11-05 20:55:58 +07:00
committed by bflatastic
parent d63ce36b05
commit 2cd68fbdcb
5 changed files with 68 additions and 2 deletions
+26
View File
@@ -198,6 +198,30 @@ fn read_file_refuses_what_is_not_a_regular_file() {
assert!(read_file("/dev/zero").is_err());
}
/// Read the first `max` bytes of a file (all of it when it is shorter), and nothing past them. Only
/// a regular file is read.
fn read_file_head(path: &str, max: usize) -> Result<Vec<u8>, String> {
use std::io::Read;
if !std::fs::metadata(path).map_err(|e| e.to_string())?.is_file() {
return Err(format!("{path}: not a file"));
}
let mut out = vec![];
std::fs::File::open(path).and_then(|f| f.take(max as u64).read_to_end(&mut out)).map_err(|e| e.to_string())?;
Ok(out)
}
#[cfg(test)]
#[test]
fn read_file_head_reads_no_more_than_it_is_asked_for() {
let path = std::env::temp_dir().join(format!("vectorcraft-read-head-{}", std::process::id()));
std::fs::write(&path, b"0123456789").unwrap();
let p = path.to_string_lossy();
assert_eq!(read_file_head(&p, 4).unwrap(), b"0123");
assert_eq!(read_file_head(&p, 20).unwrap(), b"0123456789", "a shorter file is read whole");
let _ = std::fs::remove_file(&path);
assert!(read_file_head(&std::env::temp_dir().to_string_lossy(), 4).is_err(), "a folder is not read");
}
/// Write a file the safe way: a failed write keeps the old file ([`fileio::write_atomic`]).
fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> {
fileio::write_atomic(std::path::Path::new(path), bytes).map_err(|e| e.to_string())
@@ -325,6 +349,8 @@ pub(crate) fn services(parent: Parent, ui: egui::Context) -> Services {
#[cfg(windows)]
stylus: Some(windows_stylus::start()),
read: Some(Box::new(read_file)),
// Load Actions… refuses a file over its size limit without reading all of it.
read_head: Some(Box::new(read_file_head)),
write: Some(Box::new(write_file)),
exists: Some(Box::new(|p: &str| std::path::Path::new(p).exists())),
// Background Save and Export write from a worker thread.
+3
View File
@@ -196,6 +196,9 @@ impl VectorcraftApp {
if let Some(read) = s.read.take() {
s.read = Some(Box::new(move |p: &str| file_access::check_read(p).and_then(|()| read(p))));
}
if let Some(read_head) = s.read_head.take() {
s.read_head = Some(Box::new(move |p: &str, max: usize| file_access::check_read(p).and_then(|()| read_head(p, max))));
}
if let Some(mut write) = s.write.take() {
s.write = Some(Box::new(move |p: &str, b: &[u8]| file_access::check_write(p).and_then(|()| write(p, b))));
}
+23
View File
@@ -2291,6 +2291,29 @@ mod tests {
assert_eq!(set_names(&app), before, "no set added");
}
/// Where the host can read part of a file, Load Actions… reads one byte past 8 MiB: a larger
/// file is refused without reading all of it, and a smaller one loads.
#[test]
fn load_actions_reads_a_file_no_further_than_one_byte_past_its_limit() {
let mut app = app_with_sets();
let files = with_files(&mut app, "/sets/big.vcactions");
app.run(SAVE_SET, json!({"set": 1, "path": "/sets/small.vcactions"})).unwrap();
files.borrow_mut().insert("/sets/big.vcactions".into(), vec![b' '; files::MAX_SET_FILE + 100]);
app.services.read = Some(Box::new(|_: &str| Err("the whole file was read".to_string())));
let asked = std::rc::Rc::new(std::cell::RefCell::new(vec![]));
let (f, a) = (files.clone(), asked.clone());
app.services.read_head = Some(Box::new(move |path: &str, max: usize| {
a.borrow_mut().push(max);
let bytes = f.borrow().get(path).cloned().ok_or_else(|| "no such file".to_string())?;
Ok(bytes.into_iter().take(max).collect())
}));
let before = set_names(&app);
assert_eq!(app.run(LOAD_SET, json!({"path": "/sets/big.vcactions"})).unwrap_err(), "the action set file is larger than 8 MiB");
assert_eq!(set_names(&app), before, "no set added");
assert_eq!(app.run(LOAD_SET, json!({"path": "/sets/small.vcactions"})).unwrap()["name"], json!("Mine 2"));
assert_eq!(*asked.borrow(), [files::MAX_SET_FILE + 1; 2], "each read stops one byte past the limit");
}
/// Save Actions… does not write a set that Load Actions… would refuse: more than 1,000 actions.
#[test]
fn save_actions_does_not_write_a_set_load_actions_refuses() {
+8 -2
View File
@@ -181,7 +181,8 @@ pub(crate) fn can_load(app: &VectorcraftApp) -> bool {
/// the file's text `data`, and adds its set after the others, with its actions. A set with the
/// same name as one in the panel gets the next free number (`Print 2`). Loaded actions keep their
/// function keys. The current set and the other sets stay as they are →
/// `{set: index, name, actions: n}`.
/// `{set: index, name, actions: n}`. Where the host can read part of a file (the desktop), a file
/// larger than 8 MiB is refused after its first 8 MiB and one byte are read.
pub fn load_set(app: &mut VectorcraftApp, p: &Value) -> Result<Value, String> {
let (text, stem) = match p.get("data") {
Some(Value::String(data)) => {
@@ -200,7 +201,12 @@ pub fn load_set(app: &mut VectorcraftApp, p: &Value) -> Result<Value, String> {
crate::picks::open(app, &pick).ok_or("cancelled")?
}
};
let bytes = app.services.read.as_ref().ok_or("no file reader")?(&path)?;
// One byte past the limit tells a larger file apart without reading all of it.
let bytes = match (&app.services.read_head, &app.services.read) {
(Some(read_head), _) => read_head(&path, MAX_SET_FILE + 1)?,
(None, Some(read)) => read(&path)?,
(None, None) => return Err("no file reader".into()),
};
if bytes.len() > MAX_SET_FILE {
return Err("the action set file is larger than 8 MiB".into());
}
+8
View File
@@ -37,6 +37,10 @@ pub type PickSave = Box<dyn FnMut(&FilePick) -> Option<String>>;
pub type ReadFn = Box<dyn Fn(&str) -> Result<Vec<u8>, String>>;
/// The first `max` bytes of the file at a path (all of it when it is shorter); the rest of the file
/// is not read.
pub type ReadHeadFn = Box<dyn Fn(&str, usize) -> Result<Vec<u8>, String>>;
pub type WriteFn = Box<dyn FnMut(&str, &[u8]) -> Result<(), String>>;
pub type ExistsFn = Box<dyn Fn(&str) -> bool>;
@@ -76,6 +80,10 @@ pub struct Services {
/// Show a save dialog (suggested name, folder, file types); returns a path.
pub pick_save: Option<PickSave>,
pub read: Option<ReadFn>,
/// Read at most a number of bytes from a file (desktop). A file with a size limit is read one
/// byte past the limit, so a larger file is refused without reading all of it. Without it, the
/// whole file is read with `read`.
pub read_head: Option<ReadHeadFn>,
pub write: Option<WriteFn>,
/// Is there a file or folder at a path (desktop)? The app asks before replacing a file that a
/// save panel's name names once the app gives it an extension ([`picks::ask_to_replace`]).