diff --git a/crates/noa-app/src/input/paste.rs b/crates/noa-app/src/input/paste.rs index 1700d4a..4e26564 100644 --- a/crates/noa-app/src/input/paste.rs +++ b/crates/noa-app/src/input/paste.rs @@ -25,7 +25,15 @@ pub fn encode_paste(text: &str, bracketed_paste: bool) -> Option> { /// (applescript Amendment 1.5). Sized to the terminal's OSC 52 clipboard cap /// (8 MiB decoded) so a scripted write can never queue more than an equivalent /// clipboard paste; anything longer is truncated on a UTF-8 boundary. -pub(crate) const APPLESCRIPT_INPUT_TEXT_CAP: usize = 8 * 1024 * 1024; +/// +/// This equals the pty writer's whole-queue budget (`WRITE_BYTE_CAP`), which +/// is reserved for the *framed* bytes in one go — so the bracketed-paste +/// markers must come out of the same cap ([`BRACKET_FRAME_LEN`]) or a +/// payload cut exactly to the cap is rejected outright by the writer. +pub(crate) const APPLESCRIPT_INPUT_TEXT_CAP: usize = noa_pty::WRITE_BYTE_CAP; + +/// Bytes added around a bracketed paste: `ESC[200~` + `ESC[201~`. +const BRACKET_FRAME_LEN: usize = b"\x1b[200~".len() + b"\x1b[201~".len(); /// Encode AppleScript `input text` for the pty (applescript R-7/AC-8). It /// travels the exact same path as a clipboard paste — bracketed when DECSET @@ -33,7 +41,12 @@ pub(crate) const APPLESCRIPT_INPUT_TEXT_CAP: usize = 8 * 1024 * 1024; /// [`APPLESCRIPT_INPUT_TEXT_CAP`] on a UTF-8 boundary. Pure and unit-tested so /// the byte-level contract can be verified without an Apple Event. pub(crate) fn applescript_input_bytes(text: &str, bracketed_paste: bool) -> Option> { - encode_paste(cap_input_text(text), bracketed_paste) + let cap = if bracketed_paste { + APPLESCRIPT_INPUT_TEXT_CAP - BRACKET_FRAME_LEN + } else { + APPLESCRIPT_INPUT_TEXT_CAP + }; + encode_paste(cap_input_text(text, cap), bracketed_paste) } /// Encode `noa.sendText`'s `paste: false` payload for the pty (noa-server @@ -45,7 +58,7 @@ pub(crate) fn applescript_input_bytes(text: &str, bracketed_paste: bool) -> Opti /// [`APPLESCRIPT_INPUT_TEXT_CAP`] on a UTF-8 boundary, matching the paste /// path's bound on how much one RPC call can queue to the pty. pub(crate) fn raw_input_bytes(text: &str) -> Option> { - let capped = cap_input_text(text); + let capped = cap_input_text(text, APPLESCRIPT_INPUT_TEXT_CAP); if capped.is_empty() { None } else { @@ -53,9 +66,9 @@ pub(crate) fn raw_input_bytes(text: &str) -> Option> { } } -fn cap_input_text(text: &str) -> &str { - if text.len() > APPLESCRIPT_INPUT_TEXT_CAP { - let mut end = APPLESCRIPT_INPUT_TEXT_CAP; +fn cap_input_text(text: &str, cap: usize) -> &str { + if text.len() > cap { + let mut end = cap; while end > 0 && !text.is_char_boundary(end) { end -= 1; } diff --git a/crates/noa-app/src/input/tests.rs b/crates/noa-app/src/input/tests.rs index 48e02c8..e172c31 100644 --- a/crates/noa-app/src/input/tests.rs +++ b/crates/noa-app/src/input/tests.rs @@ -854,6 +854,23 @@ fn applescript_input_caps_oversized_payload_on_char_boundary() { assert_eq!(bytes.len(), cap - 1); } +// A payload cut to the cap must still fit the pty writer's whole-queue +// budget *with* the bracketed-paste frame, or the writer rejects it outright. +#[test] +fn applescript_input_bracketed_frame_fits_within_pty_budget() { + let cap = super::paste::APPLESCRIPT_INPUT_TEXT_CAP; + let text = "a".repeat(cap + 100); + let bytes = applescript_input_bytes(&text, true).expect("non-empty"); + assert_eq!(bytes.len(), noa_pty::WRITE_BYTE_CAP); + assert!(bytes.starts_with(b"\x1b[200~") && bytes.ends_with(b"\x1b[201~")); + assert!( + noa_pty::PtyWriteBudget::default().reserve(&bytes).is_ok(), + "framed paste at the cap must be reservable on an empty queue" + ); + let raw = applescript_input_bytes(&text, false).expect("non-empty"); + assert_eq!(raw.len(), noa_pty::WRITE_BYTE_CAP); +} + // noa-server sendText paste:false: bytes pass through untouched, unlike the // paste path which strips embedded bracket markers and can wrap in ESC[200~. #[test] diff --git a/crates/noa-config/src/writer.rs b/crates/noa-config/src/writer.rs index e1883b4..dc3dc06 100644 --- a/crates/noa-config/src/writer.rs +++ b/crates/noa-config/src/writer.rs @@ -147,11 +147,47 @@ pub fn write_config_updates(path: &Path, updates: &[(String, String)]) -> io::Re })?; fs::create_dir_all(parent)?; - let tmp = target.with_extension("tmp"); - fs::write(&tmp, updated)?; - fs::rename(&tmp, &target)?; + // Create the temp file with a unique name so two concurrent writers never + // clobber each other's staging file, and 0600 so a config containing + // e.g. `server-token` is never briefly world-readable via the umask + // default. The existing file's mode (if any) is carried over before the + // rename so a user-tightened (0600) or user-loosened (0644) config keeps + // its permissions across a save. + let existing_mode = fs::metadata(&target).ok().map(|m| m.permissions()); + let tmp = parent.join(format!( + ".{}.{}.tmp", + target + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "config".to_string()), + std::process::id() + )); + let write_result = write_private(&tmp, updated.as_bytes()).and_then(|()| { + if let Some(perms) = existing_mode { + fs::set_permissions(&tmp, perms)?; + } + fs::rename(&tmp, &target) + }); + if write_result.is_err() { + let _ = fs::remove_file(&tmp); + } + write_result +} - Ok(()) +/// Creates `path` (truncating any stale leftover) with owner-only +/// permissions on unix and writes `contents` to it. +fn write_private(path: &Path, contents: &[u8]) -> io::Result<()> { + use std::io::Write; + let mut opts = fs::OpenOptions::new(); + opts.write(true).create(true).truncate(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut file = opts.open(path)?; + file.write_all(contents)?; + file.sync_all() } #[cfg(test)] @@ -298,7 +334,44 @@ theme = 3024 Day\r "font-size = 16\n" ); // No leftover temp file after a successful rename. - assert!(!config_path.with_extension("tmp").exists()); + assert_eq!(fs::read_dir(&dir).unwrap().count(), 1); + fs::remove_dir_all(dir).unwrap(); + } + + #[cfg(unix)] + #[test] + fn write_config_updates_preserves_existing_mode() { + use std::os::unix::fs::PermissionsExt; + + for mode in [0o600u32, 0o640, 0o644] { + let dir = unique_temp_dir(&format!("mode{mode:o}")); + fs::create_dir_all(&dir).unwrap(); + let config_path = dir.join("config"); + fs::write(&config_path, "font-size = 12\n").unwrap(); + fs::set_permissions(&config_path, fs::Permissions::from_mode(mode)).unwrap(); + + write_config_updates(&config_path, &[("font-size".to_string(), "16".to_string())]) + .unwrap(); + + let got = fs::metadata(&config_path).unwrap().permissions().mode() & 0o777; + assert_eq!(got, mode, "mode {mode:o} not preserved"); + fs::remove_dir_all(dir).unwrap(); + } + } + + #[cfg(unix)] + #[test] + fn write_config_updates_creates_new_file_private() { + use std::os::unix::fs::PermissionsExt; + + let dir = unique_temp_dir("newmode"); + fs::create_dir_all(&dir).unwrap(); + let config_path = dir.join("config"); + + write_config_updates(&config_path, &[("font-size".to_string(), "16".to_string())]).unwrap(); + + let got = fs::metadata(&config_path).unwrap().permissions().mode() & 0o777; + assert_eq!(got, 0o600); fs::remove_dir_all(dir).unwrap(); } diff --git a/crates/noa-grid/src/sixel.rs b/crates/noa-grid/src/sixel.rs index 9023fbc..bb2966c 100644 --- a/crates/noa-grid/src/sixel.rs +++ b/crates/noa-grid/src/sixel.rs @@ -20,9 +20,15 @@ pub struct SixelRaster { const COLOR_REGISTERS: usize = 256; +/// Growable RGBA canvas. Storage is over-allocated geometrically (row +/// stride `cap_width`, `cap_height` rows) so that a stream which widens the +/// image one column at a time costs amortized O(pixels) rather than the +/// O(height × width²) of reallocating an exact-fit buffer per column. struct Canvas { width: u32, height: u32, + cap_width: u32, + cap_height: u32, pixels: Vec, background: [u8; 4], } @@ -32,6 +38,8 @@ impl Canvas { Self { width: 0, height: 0, + cap_width: 0, + cap_height: 0, pixels: Vec::new(), background, } @@ -46,31 +54,55 @@ impl Canvas { if width > MAX_IMAGE_DIM || height > MAX_IMAGE_DIM { return Err(KittyError::TooBig); } - let bytes = (width as usize) - .checked_mul(height as usize) - .and_then(|px| px.checked_mul(4)) - .ok_or(KittyError::TooBig)?; - if bytes > TOTAL_BYTES_LIMIT { + if bytes_for(width, height)? > TOTAL_BYTES_LIMIT { return Err(KittyError::TooBig); } + if width <= self.cap_width && height <= self.cap_height { + self.width = width; + self.height = height; + return Ok(()); + } + + // Grow only the axes that exceed capacity; fall back to the exact + // requested size when doubling would overshoot the global byte budget + // (the request itself is known to fit). + let mut cap_w = if width > self.cap_width { + width + .max(self.cap_width.saturating_mul(2)) + .min(MAX_IMAGE_DIM) + } else { + self.cap_width + }; + let mut cap_h = if height > self.cap_height { + height + .max(self.cap_height.saturating_mul(2)) + .min(MAX_IMAGE_DIM) + } else { + self.cap_height + }; + if bytes_for(cap_w, cap_h)? > TOTAL_BYTES_LIMIT { + cap_w = width; + cap_h = height; + } + let bytes = bytes_for(cap_w, cap_h)?; - let old_width = self.width; - let old_height = self.height; let old = std::mem::take(&mut self.pixels); let mut new_pixels = vec![0u8; bytes]; for px in new_pixels.chunks_exact_mut(4) { px.copy_from_slice(&self.background); } - for y in 0..old_height as usize { - let old_start = y * old_width as usize * 4; - let old_end = old_start + old_width as usize * 4; - let new_start = y * width as usize * 4; - new_pixels[new_start..new_start + old_width as usize * 4] - .copy_from_slice(&old[old_start..old_end]); + let old_stride = self.cap_width as usize * 4; + let new_stride = cap_w as usize * 4; + let row_bytes = self.width as usize * 4; + for y in 0..self.height as usize { + new_pixels[y * new_stride..y * new_stride + row_bytes] + .copy_from_slice(&old[y * old_stride..y * old_stride + row_bytes]); } self.width = width; self.height = height; + self.cap_width = cap_w; + self.cap_height = cap_h; self.pixels = new_pixels; Ok(()) } @@ -84,7 +116,7 @@ impl Canvas { fn set_pixel(&mut self, x: u32, y: u32, color: Rgb) -> Result<(), KittyError> { self.ensure_size(x.saturating_add(1), y.saturating_add(1))?; - let i = ((y as usize * self.width as usize) + x as usize) * 4; + let i = ((y as usize * self.cap_width as usize) + x as usize) * 4; self.pixels[i..i + 4].copy_from_slice(&[color.r, color.g, color.b, 0xff]); Ok(()) } @@ -94,22 +126,44 @@ impl Canvas { if self.width == 0 || self.height == 0 { return Err(KittyError::Invalid); } + let rgba = if self.cap_width == self.width && self.cap_height == self.height { + self.pixels + } else { + let stride = self.cap_width as usize * 4; + let row_bytes = self.width as usize * 4; + let mut compact = Vec::with_capacity(row_bytes * self.height as usize); + for y in 0..self.height as usize { + compact.extend_from_slice(&self.pixels[y * stride..y * stride + row_bytes]); + } + compact + }; Ok(SixelRaster { width: self.width, height: self.height, - rgba: self.pixels, + rgba, }) } } +fn bytes_for(width: u32, height: u32) -> Result { + (width as usize) + .checked_mul(height as usize) + .and_then(|px| px.checked_mul(4)) + .ok_or(KittyError::TooBig) +} + /// Rasterize a parsed SIXEL command into straight RGBA8. -pub fn rasterize(cmd: &SixelGraphicsCommand) -> Result { +/// +/// `terminal_bg` is the terminal's current default background: per DEC +/// STD 070, `P2` = 0 (or omitted) and 2 paint blank pixels with the +/// background color, while `P2` = 1 leaves them transparent (the existing +/// screen content shows through). +pub fn rasterize(cmd: &SixelGraphicsCommand, terminal_bg: Rgb) -> Result { let mut palette = xterm_palette(); - let background = if cmd.background == 2 { - let c = palette[0]; - [c.r, c.g, c.b, 0xff] - } else { + let background = if cmd.background == 1 { [0, 0, 0, 0] + } else { + [terminal_bg.r, terminal_bg.g, terminal_bg.b, 0xff] }; let mut canvas = Canvas::new(background); let mut current_color = 0usize; @@ -158,6 +212,8 @@ pub fn rasterize(cmd: &SixelGraphicsCommand) -> Result if params.len() >= 4 { declared_width = params[2]; declared_height = params[3]; + // Pre-size so a declared image is allocated once. + canvas.ensure_size(declared_width, declared_height)?; } i = next; } @@ -257,8 +313,10 @@ fn percent(value: u32) -> u8 { ((value.min(100) * 255 + 50) / 100) as u8 } +/// DEC HLS: hue 0° is *blue* (120° red, 240° green), unlike the usual HSL +/// convention where 0° is red — rotate by 240° before the standard conversion. fn hls_to_rgb(hue: u32, lightness: u32, saturation: u32) -> Rgb { - let h = (hue % 360) as f64 / 360.0; + let h = ((hue % 360 + 240) % 360) as f64 / 360.0; let l = lightness.min(100) as f64 / 100.0; let s = saturation.min(100) as f64 / 100.0; if s == 0.0 { @@ -301,15 +359,26 @@ fn channel(p: f64, q: f64, mut t: f64) -> u8 { mod tests { use super::*; + const BG: Rgb = Rgb::new(10, 20, 30); + const BG_PX: [u8; 4] = [10, 20, 30, 255]; + fn cmd(data: &[u8]) -> SixelGraphicsCommand { + cmd_bg(data, 0) + } + + fn cmd_bg(data: &[u8], background: u16) -> SixelGraphicsCommand { SixelGraphicsCommand { aspect_ratio: 0, - background: 0, + background, horizontal_grid_size: 0, data: data.to_vec(), } } + fn rasterize(cmd: &SixelGraphicsCommand) -> Result { + super::rasterize(cmd, BG) + } + #[test] fn rasterizes_basic_sixel_columns() { let image = rasterize(&cmd(b"#1;2;100;0;0@A")).unwrap(); @@ -335,12 +404,107 @@ mod tests { } #[test] - fn raster_attributes_extend_transparent_canvas() { + fn raster_attributes_extend_canvas_with_background() { + // P2 omitted/0 → blank pixels take the terminal background (DEC STD 070). let image = rasterize(&cmd(br#""1;1;4;7#1;2;100;0;0@"#)).unwrap(); assert_eq!((image.width, image.height), (4, 7)); assert_eq!(&image.rgba[0..4], &[255, 0, 0, 255]); - assert_eq!(&image.rgba[(4 * 6 + 3) * 4..(4 * 6 + 4) * 4], &[0, 0, 0, 0]); + assert_eq!(&image.rgba[(4 * 6 + 3) * 4..(4 * 6 + 4) * 4], &BG_PX); + } + + #[test] + fn background_select_one_is_transparent_and_two_is_opaque() { + let transparent = rasterize(&cmd_bg(b"?", 1)).unwrap(); + assert_eq!(&transparent.rgba[0..4], &[0, 0, 0, 0]); + + let opaque = rasterize(&cmd_bg(b"?", 2)).unwrap(); + assert_eq!(&opaque.rgba[0..4], &BG_PX); + } + + #[test] + fn column_growth_preserves_height_capacity() { + let mut canvas = Canvas::new(BG_PX); + let red = Rgb::new(255, 0, 0); + for x in 0..4096 { + draw_sixel(&mut canvas, x, 0, b'@' - b'?', 1, red).unwrap(); + assert_eq!(canvas.cap_height, 6); + } + assert_eq!(canvas.pixels.len(), 4096 * 6 * 4); + + let image = canvas.finish(0, 0).unwrap(); + assert_eq!((image.width, image.height), (4096, 6)); + for (i, pixel) in image.rgba.chunks_exact(4).enumerate() { + assert_eq!(pixel, if i < 4096 { &[255, 0, 0, 255] } else { &BG_PX }); + } + } + + #[test] + fn row_growth_preserves_width_capacity() { + let mut canvas = Canvas::new(BG_PX); + let red = Rgb::new(255, 0, 0); + for y in (0..4096).step_by(6) { + draw_sixel(&mut canvas, 0, y, b'@' - b'?', 1, red).unwrap(); + assert_eq!(canvas.cap_width, 1); + } + assert!(canvas.pixels.len() < 2 * 4098 * 4); + + let image = canvas.finish(0, 0).unwrap(); + assert_eq!((image.width, image.height), (1, 4098)); + for (y, pixel) in image.rgba.chunks_exact(4).enumerate() { + assert_eq!( + pixel, + if y % 6 == 0 { + &[255, 0, 0, 255] + } else { + &BG_PX + } + ); + } + } + + #[test] + fn column_at_a_time_growth_matches_exact_fit_output() { + // Advance far down, then widen one column per sixel: the geometric + // growth path (stride ≠ width) must compact to the same pixels an + // exact-fit canvas would produce. + let mut data = Vec::new(); + for _ in 0..40 { + data.extend_from_slice(b"-"); + } + data.extend_from_slice(b"#1;2;100;0;0"); + for _ in 0..300 { + data.extend_from_slice(b"@"); + } + let image = rasterize(&cmd(&data)).unwrap(); + + assert_eq!((image.width, image.height), (300, 246)); + assert_eq!(image.rgba.len(), 300 * 246 * 4); + let top_left = &image.rgba[0..4]; + assert_eq!(top_left, &BG_PX); + let last_row_first_px = 240 * 300 * 4; + assert_eq!( + &image.rgba[last_row_first_px..last_row_first_px + 4], + &[255, 0, 0, 255] + ); + let last_row_last_px = (240 * 300 + 299) * 4; + assert_eq!( + &image.rgba[last_row_last_px..last_row_last_px + 4], + &[255, 0, 0, 255] + ); + let row_241_first = 241 * 300 * 4; + assert_eq!(&image.rgba[row_241_first..row_241_first + 4], &BG_PX); + } + + #[test] + fn dec_hls_hue_zero_is_blue() { + assert_eq!(hls_to_rgb(0, 50, 100), Rgb::new(0, 0, 255)); + assert_eq!(hls_to_rgb(120, 50, 100), Rgb::new(255, 0, 0)); + assert_eq!(hls_to_rgb(240, 50, 100), Rgb::new(0, 255, 0)); + assert_eq!(hls_to_rgb(360, 50, 100), Rgb::new(0, 0, 255)); + assert_eq!(hls_to_rgb(0, 50, 0), Rgb::new(128, 128, 128)); + // HLS and RGB color specs must agree on the primaries. + assert_eq!(hls_to_rgb(120, 50, 100), Rgb::new(percent(100), 0, 0)); } #[test] diff --git a/crates/noa-grid/src/terminal/kitty_graphics.rs b/crates/noa-grid/src/terminal/kitty_graphics.rs index 983b014..463b303 100644 --- a/crates/noa-grid/src/terminal/kitty_graphics.rs +++ b/crates/noa-grid/src/terminal/kitty_graphics.rs @@ -262,7 +262,11 @@ impl Terminal { return; } - let Ok(raster) = sixel::rasterize(&cmd) else { + let bg = self + .colors + .default_bg() + .unwrap_or_else(|| self.colors.base_default_bg()); + let Ok(raster) = sixel::rasterize(&cmd, bg) else { return; }; let cols = raster.width.div_ceil(cell_w).clamp(1, u16::MAX as u32) as u16; @@ -315,9 +319,12 @@ impl Terminal { return; } let free = kitty_delete_frees(spec); - let number_ids: Vec = match spec { - KittyDelete::ByNumber { .. } => self.kitty_images.ids_with_number(cmd.image_number), - _ => Vec::new(), + let number_id = match spec { + KittyDelete::ByNumber { .. } => self + .kitty_images + .get_by_number(cmd.image_number) + .map(|image| image.id), + _ => None, }; let (cursor_abs, cursor_col) = { let s = self.active(); @@ -341,7 +348,10 @@ impl Terminal { p.image_id == cmd.image_id && (cmd.placement_id == 0 || p.placement_id == cmd.placement_id) } - KittyDelete::ByNumber { .. } => number_ids.contains(&p.image_id), + KittyDelete::ByNumber { .. } => { + number_id == Some(p.image_id) + && (cmd.placement_id == 0 || p.placement_id == cmd.placement_id) + } KittyDelete::AtCursor { .. } => p.covers_abs(cursor_abs, cursor_col), KittyDelete::AtCell { .. } => p.covers_abs(target_abs, target_col), KittyDelete::AtCellZ { .. } => { @@ -359,7 +369,17 @@ impl Terminal { }); if free { - for id in removed { + // Candidates are the images whose placements were just removed + // *plus* the images the command named directly: an image that was + // transmitted but never placed has no placement to remove, yet + // `d=I`/`d=N` must still free its data. + let mut candidates = removed; + match spec { + KittyDelete::ById { .. } if cmd.image_id != 0 => candidates.push(cmd.image_id), + KittyDelete::ByNumber { .. } => candidates.extend(number_id), + _ => {} + } + for id in candidates { if !self.image_referenced(id) { self.kitty_images.remove(id); } diff --git a/crates/noa-grid/src/tests/kitty_graphics.rs b/crates/noa-grid/src/tests/kitty_graphics.rs index 964652a..2eec4a6 100644 --- a/crates/noa-grid/src/tests/kitty_graphics.rs +++ b/crates/noa-grid/src/tests/kitty_graphics.rs @@ -117,10 +117,7 @@ fn kitty_animation_flag_tracks_running_animation_through_vt_dispatch() { // Base image only: no animation yet. feed( &mut t, - &kitty_apc( - "a=t,f=32,s=2,v=1,i=1", - &[10, 20, 30, 255, 40, 50, 60, 255], - ), + &kitty_apc("a=t,f=32,s=2,v=1,i=1", &[10, 20, 30, 255, 40, 50, 60, 255]), ); assert!(!flag.load(std::sync::atomic::Ordering::Relaxed)); @@ -275,6 +272,89 @@ fn kitty_delete_by_id_uppercase_frees_data() { ); } +#[test] +fn kitty_delete_by_id_uppercase_frees_unplaced_image() { + let mut t = kitty_terminal(); + feed( + &mut t, + &kitty_apc("a=t,f=32,s=10,v=20,i=31", &vec![0u8; 10 * 20 * 4]), + ); + assert!(t.kitty_images.get(31).is_some()); + assert!(t.primary.kitty_placements.is_empty()); + feed(&mut t, b"\x1b_Ga=d,d=I,i=31\x1b\\"); + assert!( + t.kitty_images.get(31).is_none(), + "uppercase d frees an image that was never placed" + ); +} + +#[test] +fn kitty_delete_by_number_uppercase_preserves_older_unplaced_image() { + for action in ["t", "T"] { + let mut t = kitty_terminal(); + feed(&mut t, &kitty_apc("a=t,f=32,s=1,v=1,I=9", &[1, 2, 3, 255])); + let old_id = t.kitty_images.get_by_number(9).unwrap().id; + feed( + &mut t, + &kitty_apc(&format!("a={action},f=32,s=1,v=1,I=9"), &[4, 5, 6, 255]), + ); + let new_id = t.kitty_images.get_by_number(9).unwrap().id; + assert_ne!(old_id, new_id); + + feed(&mut t, b"\x1b_Ga=d,d=N,I=9\x1b\\"); + assert!(t.primary.kitty_placements.is_empty()); + assert!(t.kitty_images.get(new_id).is_none()); + assert!(t.kitty_images.get(old_id).is_some()); + + t.pending_writes.clear(); + feed(&mut t, format!("\x1b_Ga=p,i={old_id}\x1b\\").as_bytes()); + assert_eq!(t.pending_writes, format!("\x1b_Gi={old_id};OK\x1b\\").as_bytes()); + assert_eq!(t.primary.kitty_placements.len(), 1); + assert_eq!(t.primary.kitty_placements[0].image_id, old_id); + } +} + +#[test] +fn kitty_delete_by_number_only_removes_newest_placements() { + for spec in ["n", "N"] { + let mut t = kitty_terminal(); + feed(&mut t, &kitty_apc("a=T,f=32,s=1,v=1,I=9,p=7", &[0; 4])); + let old_id = t.kitty_images.get_by_number(9).unwrap().id; + feed(&mut t, &kitty_apc("a=T,f=32,s=1,v=1,I=9,p=7", &[0; 4])); + let new_id = t.kitty_images.get_by_number(9).unwrap().id; + assert_eq!(t.primary.kitty_placements.len(), 2); + + feed(&mut t, format!("\x1b_Ga=d,d={spec},I=9,p=7\x1b\\").as_bytes()); + assert_eq!(t.primary.kitty_placements.len(), 1); + assert_eq!(t.primary.kitty_placements[0].image_id, old_id); + assert!(t.kitty_images.get(old_id).is_some()); + assert_eq!(t.kitty_images.get(new_id).is_some(), spec == "n"); + } +} + +#[test] +fn kitty_delete_by_number_honours_placement_id() { + let mut t = kitty_terminal(); + feed( + &mut t, + &kitty_apc("a=T,f=32,s=10,v=20,I=9,p=7", &vec![0u8; 10 * 20 * 4]), + ); + feed(&mut t, b"\x1b_Ga=p,I=9,p=8\x1b\\"); + assert_eq!(t.primary.kitty_placements.len(), 2); + + feed(&mut t, b"\x1b_Ga=d,d=n,I=9,p=7\x1b\\"); + assert_eq!(t.primary.kitty_placements.len(), 1); + assert_eq!(t.primary.kitty_placements[0].placement_id, 8); + + // Data is still referenced by p=8, so even `d=N` keeps it ... + feed(&mut t, b"\x1b_Ga=d,d=N,I=9,p=7\x1b\\"); + assert_eq!(t.kitty_images.ids_with_number(9).len(), 1); + // ... until the last placement goes. + feed(&mut t, b"\x1b_Ga=d,d=N,I=9\x1b\\"); + assert!(t.primary.kitty_placements.is_empty()); + assert!(t.kitty_images.ids_with_number(9).is_empty()); +} + #[test] fn kitty_delete_at_cursor() { let mut t = kitty_terminal(); @@ -436,9 +516,9 @@ fn kitty_placement_pruned_when_its_row_is_evicted() { let mut t = Terminal::new(GridSize::new(20, 4)); t.set_pixel_metrics(10, 20, 200, 80); t.set_scrollback_limit_bytes(1); // keep essentially no history - // Place a 1×1 image at the top, then scroll far past it. Eviction is - // page-granular, so it takes more than a page of full-width rows to strand - // the anchor. + // Place a 1×1 image at the top, then scroll far past it. Eviction is + // page-granular, so it takes more than a page of full-width rows to strand + // the anchor. feed(&mut t, b"\x1b[1;1H"); feed( &mut t, @@ -491,10 +571,7 @@ fn sixel_dcs_rasterizes_and_places_image() { fn sixel_repeat_and_raster_attributes_determine_size() { let mut t = kitty_terminal(); - feed( - &mut t, - &sixel_dcs(br#"q"1;1;12;7#2;2;0;100;0!3@-?"#), - ); + feed(&mut t, &sixel_dcs(br#"q"1;1;12;7#2;2;0;100;0!3@-?"#)); let placement = &t.primary.kitty_placements[0]; let image = t.kitty_image(placement.image_id).unwrap(); @@ -642,7 +719,16 @@ fn kitty_rectangle_scroll_keeps_placements_outside_the_margins() { // DECSLRM 5..15 + DECSTBM 2..12, then LF at the region bottom scrolls only // the rectangle; columns outside the margins are untouched. feed(&mut t, b"\x1b[?69h\x1b[5;15s\x1b[2;12r\x1b[12;5H\n"); - let ids: Vec = t.primary.kitty_placements.iter().map(|p| p.image_id).collect(); - assert_eq!(ids, vec![1], "only the image inside the margins is scrolled away"); + let ids: Vec = t + .primary + .kitty_placements + .iter() + .map(|p| p.image_id) + .collect(); + assert_eq!( + ids, + vec![1], + "only the image inside the margins is scrolled away" + ); assert_eq!(t.kitty_visible_placements()[0].grid_y, 5); } diff --git a/crates/noa-ipc/src/server.rs b/crates/noa-ipc/src/server.rs index aed49b6..4d4cb02 100644 --- a/crates/noa-ipc/src/server.rs +++ b/crates/noa-ipc/src/server.rs @@ -495,7 +495,18 @@ enum ConnectionRoute { /// wall-clock deadline during the WS handshake (R-2), or past it and running /// under the normal fixed read-poll/write-timeout pair (R-4). enum StreamMode { - Handshake { deadline: Instant }, + Handshake { + deadline: Instant, + }, + /// Handshake done, `noa.hello` not yet accepted: reads fail fast once the + /// absolute hello deadline has passed. Without this, `tungstenite`'s + /// `read()` loops internally until a *complete* message arrives, so an + /// unauthenticated client trickling one incomplete frame (each byte + /// inside the 50ms poll) never returns control to the loop's top-of- + /// iteration deadline check. + AwaitingHello { + deadline: Instant, + }, Connected, } @@ -544,10 +555,13 @@ impl DeadlineStream { } /// Switches this stream from the handshake's absolute-deadline mode to - /// the connection's normal steady-state timeouts. Called once, - /// immediately after `accept_hdr_with_config` returns successfully. - fn mark_connected(&mut self) -> io::Result<()> { - self.mode = StreamMode::Connected; + /// the connection's normal steady-state timeouts, while keeping reads + /// bounded by `hello_deadline` until [`Self::mark_hello_done`]. Called + /// once, immediately after `accept_hdr_with_config` returns successfully. + fn mark_connected(&mut self, hello_deadline: Instant) -> io::Result<()> { + self.mode = StreamMode::AwaitingHello { + deadline: hello_deadline, + }; self.inner .set_read_timeout(Some(Duration::from_millis(50)))?; // R-4: a bounded write timeout for the connection's whole life, not @@ -556,6 +570,13 @@ impl DeadlineStream { Ok(()) } + /// Lifts the hello deadline once the session is authenticated. + fn mark_hello_done(&mut self) { + if matches!(self.mode, StreamMode::AwaitingHello { .. }) { + self.mode = StreamMode::Connected; + } + } + fn mark_attach_connected(&mut self) -> io::Result<()> { self.mode = StreamMode::Connected; self.inner.set_read_timeout(Some(ATTACH_READ_POLL))?; @@ -567,8 +588,15 @@ impl DeadlineStream { impl io::Read for DeadlineStream { fn read(&mut self, buf: &mut [u8]) -> io::Result { - if let StreamMode::Handshake { deadline } = self.mode { - self.arm_handshake(deadline)?; + match self.mode { + StreamMode::Handshake { deadline } => self.arm_handshake(deadline)?, + StreamMode::AwaitingHello { deadline } if Instant::now() >= deadline => { + return Err(io::Error::new( + io::ErrorKind::TimedOut, + "hello deadline exceeded", + )); + } + StreamMode::AwaitingHello { .. } | StreamMode::Connected => {} } self.inner.read(buf) } @@ -698,7 +726,8 @@ fn handle_connection( .unwrap_or(ConnectionRoute::Invalid); match selected { ConnectionRoute::Control { authority } => { - ws.get_mut().mark_connected()?; + let connected_at = Instant::now(); + ws.get_mut().mark_connected(connected_at + hello_deadline)?; let (conn_id, queue) = broadcaster.register_connection(); guard.conn_id = Some(conn_id); let mut session = Session { @@ -708,7 +737,6 @@ fn handle_connection( attach_authority: authority, attach_leases: HashMap::new(), }; - let connected_at = Instant::now(); run_connection_loop( &mut ws, &backend, @@ -784,6 +812,9 @@ fn run_connection_loop( ) { ws.send(Message::Text(response)).map_err(ws_err_to_io)?; } + if session.hello_done { + ws.get_mut().mark_hello_done(); + } } Ok(Message::Ping(payload)) => { ws.send(Message::Pong(payload)).map_err(ws_err_to_io)?; @@ -1625,7 +1656,9 @@ mod tests { fn deadline_stream_mark_connected_applies_the_steady_state_timeouts() { let (_client, server) = tcp_pair(); let mut stream = DeadlineStream::new_handshake(server, Duration::from_secs(5)); - stream.mark_connected().unwrap(); + stream + .mark_connected(Instant::now() + Duration::from_secs(10)) + .unwrap(); assert_eq!( stream.inner.read_timeout().unwrap(), Some(Duration::from_millis(50)) @@ -1633,6 +1666,26 @@ mod tests { assert_eq!(stream.inner.write_timeout().unwrap(), Some(WRITE_TIMEOUT)); } + #[test] + fn deadline_stream_read_fails_fast_past_the_hello_deadline_until_hello_is_done() { + let (mut client, server) = tcp_pair(); + let mut stream = DeadlineStream::new_handshake(server, Duration::from_secs(5)); + stream + .mark_connected(Instant::now() + Duration::from_millis(1)) + .unwrap(); + std::thread::sleep(Duration::from_millis(20)); + // Bytes are available, but the hello deadline has passed: fail fast + // rather than feeding tungstenite's read loop. + io::Write::write_all(&mut client, b"x").unwrap(); + let mut buf = [0u8; 8]; + let err = io::Read::read(&mut stream, &mut buf).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::TimedOut); + + // Once hello completes the same stream reads normally. + stream.mark_hello_done(); + assert_eq!(io::Read::read(&mut stream, &mut buf).unwrap(), 1); + } + #[test] fn connection_guard_decrements_connection_count_on_drop_even_with_no_registered_conn_id() { // Covers an exit path before `register_connection` ever ran (e.g. an diff --git a/crates/noa-vt/src/sixel.rs b/crates/noa-vt/src/sixel.rs index 15fd936..c2c83c7 100644 --- a/crates/noa-vt/src/sixel.rs +++ b/crates/noa-vt/src/sixel.rs @@ -9,8 +9,8 @@ pub struct SixelGraphicsCommand { /// `Pa` — pixel aspect ratio. Kept for future scaling parity; v1 ignores it. pub aspect_ratio: u16, - /// `Pb` — background option. `2` requests an opaque background; other - /// values leave zero bits transparent in the v1 rasterizer. + /// `Pb` — background select. `0` (omitted) and `2` paint blank pixels + /// with the terminal background; `1` leaves them transparent. pub background: u16, /// `Ph` — horizontal grid size, kept for parity but ignored by xterm too. pub horizontal_grid_size: u16,