Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 6ce9ecd161 Merge branch 'main' into fix/sec-lisp-flush-linear 2026-11-03 17:36:07 -05:00
Dylan M. Taylor 8505cee19b fix(engine): move LISP output to the command history in one pass
lisp_flush drained each line from the front of the output buffer, shifting everything after it every time, so newline-dense PRINC output cost quadratic time outside the interpreter's step budget. It now walks the buffer once and drops the printed prefix once.
2026-11-03 13:55:27 +00:00
4 changed files with 25 additions and 78 deletions
-43
View File
@@ -71,8 +71,6 @@ pub fn write_atomic_with(path: &Path, permissions: Option<Permissions>, fill: im
let (mut file, tmp) = create_temp(&dir, permissions.as_ref())?;
let write = (|| {
if let Some(permissions) = permissions {
#[cfg(unix)]
let permissions = keep_group(&file, path, permissions);
file.set_permissions(permissions)?;
}
fill(&mut file)?;
@@ -92,20 +90,6 @@ pub fn write_atomic_with(path: &Path, permissions: Option<Permissions>, fill: im
Ok(())
}
/// `permissions` for the new file when `path` already exists: the group bits mean "this group" only
/// if the file is still in the old file's group, so the group is restored first. When it can't be
/// (the user is not a member of it), the group bits are dropped rather than handed to another group.
#[cfg(unix)]
fn keep_group(file: &File, path: &Path, permissions: Permissions) -> Permissions {
use std::os::unix::fs::{MetadataExt, PermissionsExt, fchown};
let Ok(old) = std::fs::symlink_metadata(path) else { return permissions };
// A file already in the old group needs no change (and a restrictive syscall policy may refuse a no-op fchown).
if !old.is_file() || file.metadata().is_ok_and(|m| m.gid() == old.gid()) || fchown(file, None, Some(old.gid())).is_ok() {
return permissions;
}
Permissions::from_mode(permissions.mode() & !0o070)
}
/// Flush `dir` so a rename just made in it survives a crash or power loss (POSIX records a
/// rename in the directory, not in the file). Best effort: some file systems (network shares)
/// refuse to sync a directory, and the file itself is already on disk.
@@ -247,33 +231,6 @@ mod tests {
std::fs::remove_dir_all(&dir).ok();
}
/// The replacement keeps the old file's group (when the user is in another group, as `id -G`
/// says) together with its mode, so the group bits never name a different group.
#[cfg(unix)]
#[test]
fn keeps_the_group_and_mode_of_the_file_it_replaces() {
use std::os::unix::fs::{MetadataExt, PermissionsExt, chown};
let dir = std::env::temp_dir().join(format!("cadcraft-aw-grp-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed)));
std::fs::create_dir_all(&dir).unwrap();
let dest = dir.join("out.bin");
std::fs::write(&dest, b"old").unwrap();
std::fs::set_permissions(&dest, Permissions::from_mode(0o640)).unwrap();
let own = std::fs::metadata(&dest).unwrap().gid();
let other = std::process::Command::new("id")
.arg("-G")
.output()
.ok()
.and_then(|o| String::from_utf8_lossy(&o.stdout).split_whitespace().filter_map(|g| g.parse::<u32>().ok()).find(|g| *g != own));
if let Some(gid) = other {
chown(&dest, None, Some(gid)).unwrap();
}
let want = std::fs::metadata(&dest).unwrap();
write_atomic(&dest, b"new").unwrap();
let got = std::fs::metadata(&dest).unwrap();
assert_eq!((got.gid(), got.mode() & 0o777), (want.gid(), 0o640));
std::fs::remove_dir_all(&dir).ok();
}
#[cfg(unix)]
#[test]
fn does_not_follow_a_planted_temp_symlink() {
+8 -32
View File
@@ -601,21 +601,6 @@ pub fn save_keeps_current(_path: &std::path::Path) -> bool {
false
}
/// The mode of a new backup: no more open than the drawing (`src_mode`, group `src_gid`) nor than the
/// `.bak` it replaces (`old`: mode and group). The backup keeps the old `.bak`'s group, so when that
/// differs from the drawing's group the group bits are dropped: that group could not read the drawing.
#[cfg(unix)]
fn backup_mode(src_mode: u32, src_gid: u32, old: Option<(u32, u32)>) -> u32 {
let mut mode = src_mode & 0o777;
if let Some((old_mode, old_gid)) = old {
mode &= old_mode;
if old_gid != src_gid {
mode &= !0o070;
}
}
mode
}
/// Before `path` is replaced: copy it to its `.bak`, streamed and written atomically too
/// ([`crate::atomic_write::write_atomic_with`]). Nothing
/// to keep when there is no file yet. A file that doesn't look like a drawing
@@ -640,10 +625,14 @@ fn backup(path: &std::path::Path) -> std::io::Result<()> {
// written through.
#[cfg(unix)]
let permissions = {
use std::os::unix::fs::{MetadataExt, PermissionsExt};
let sm = src.metadata()?;
let old = std::fs::symlink_metadata(&bak).ok().filter(|m| m.is_file());
Some(std::fs::Permissions::from_mode(backup_mode(sm.mode(), sm.gid(), old.map(|m| (m.mode(), m.gid())))))
use std::os::unix::fs::PermissionsExt;
let mut mode = src.metadata()?.permissions().mode() & 0o777;
if let Ok(m) = std::fs::symlink_metadata(&bak)
&& m.is_file()
{
mode &= m.permissions().mode();
}
Some(std::fs::Permissions::from_mode(mode))
};
#[cfg(not(unix))]
let permissions = None;
@@ -1004,19 +993,6 @@ mod tests {
use crate::Session;
use crate::temp_dir::TempDir;
/// A backup is never more open than the drawing or the old `.bak`, and loses the group bits when
/// it would keep the old `.bak`'s group while the drawing is in another.
#[cfg(unix)]
#[test]
fn backup_mode_drops_group_bits_for_a_different_group() {
use super::backup_mode;
assert_eq!(backup_mode(0o640, 10, None), 0o640);
assert_eq!(backup_mode(0o640, 10, Some((0o664, 10))), 0o640);
assert_eq!(backup_mode(0o640, 10, Some((0o664, 20))), 0o600);
assert_eq!(backup_mode(0o644, 10, Some((0o644, 20))), 0o604);
assert_eq!(backup_mode(0o600, 10, Some((0o664, 10))), 0o600);
}
fn active_title(s: &Session) -> String {
s.state().unwrap().title.clone()
}
+7 -3
View File
@@ -415,9 +415,12 @@ impl Session {
/// Move printed text to the command history: complete lines, and with `all` the rest too.
fn lisp_flush(&mut self, env: &mut Env, all: bool) {
while let Some(i) = env.out.find('\n') {
let line: String = env.out.drain(..=i).collect();
let line = line.trim_end_matches(['\n', '\r']).to_string();
// One pass over the buffer, dropping the printed lines once at the end: draining each line
// from the front shifted the rest every time, quadratic in newline-dense output.
let mut done = 0;
while let Some(i) = env.out.get(done..).and_then(|rest| rest.find('\n')) {
let line = env.out.get(done..done + i).unwrap_or_default().trim_end_matches('\r').to_string();
done += i + 1;
// `(princ "\nText")` after a prompt or a command's echo ends that line instead of
// leaving an empty one; after LISP's own line it does print an empty line.
if line.is_empty() && env.printed_at != Some(self.log.len()) {
@@ -427,6 +430,7 @@ impl Session {
self.echo(line);
env.printed_at = Some(self.log.len());
}
env.out.drain(..done);
if all && !env.out.is_empty() {
let line = std::mem::take(&mut env.out);
self.echo(line);
+10
View File
@@ -307,6 +307,16 @@ fn printing() {
assert_eq!(ok(&mut s, "(rtos 12.5 2 2)"), "\"12.50\"");
}
/// Output with many lines moves to the history in one pass, not one buffer shift per line.
#[test]
fn printing_many_lines_is_linear() {
let mut s = session();
let started = std::time::Instant::now();
ok(&mut s, "(setq x \"a\\r\\n\") (repeat 17 (setq x (strcat x x))) (princ x) (princ)");
assert!(started.elapsed() < std::time::Duration::from_secs(10), "took {:?}", started.elapsed());
assert_eq!(s.log.last().map(String::as_str), Some("a"));
}
#[test]
fn command_draws_and_one_routine_is_one_undo_step() {
let mut s = session();