fix: ten pre-release review findings for 0.9.0

src/cli.rs

@@ -247,8 +247,7 @@ pub fn run() -> Result<()> {
     // `auto` asks the terminal for its background colour (OSC 11) once, now,
     // before anything owns the screen. Only with a terminal on both ends:
     // the reply arrives on stdin, and a pipe would never answer.
-    let wants_terminal = matches!(cli.command, None | Some(Commands::Doctor { .. }));
-    if theme == "auto" && wants_terminal {
+    if theme == "auto" && uses_background(&cli, color) {
         theme::detect::init_background(stdout_tty && std::io::stdin().is_terminal());
     }
 
@@ -564,6 +563,20 @@ pub enum ThemeOrigin {
     Auto,
 }
 
+/// Will this run draw with the `auto` theme's colours? Only then is the
+/// terminal worth asking for its background: `ink doctor` reports it, the
+/// reader and `--plain` draw with it when colour is on. `--list-themes` and
+/// the other subcommands never look at it.
+fn uses_background(cli: &Cli, plain_color: bool) -> bool {
+    match cli.command {
+        Some(Commands::Doctor { .. }) => true,
+        Some(_) => false,
+        None if cli.list_themes => false,
+        None if cli.plain => plain_color,
+        None => theme::caps::color_enabled(cli.color, true),
+    }
+}
+
 /// The theme to use: an explicit `--theme` beats the config file, which beats
 /// `auto`. (`--theme auto` is the clap default, so it means "not given".)
 fn theme_choice(flag: &str, config: &Option<config::Config>) -> (String, ThemeOrigin) {

src/clipboard.rs

@@ -237,20 +237,48 @@ const CLIP_EXE: Helper = Helper {
     args: &[],
     encoding: Encoding::Utf16LeBom,
 };
-/// WSL with `clip.exe` missing from PATH (Windows PATH appending turned
-/// off): PowerShell, told to read stdin as UTF-8, sets the clipboard.
+/// WSL with `clip.exe` missing (Windows PATH appending turned off and no
+/// `clip.exe` at the usual mount points): PowerShell, told to read stdin as
+/// UTF-8, sets the clipboard.
 const POWERSHELL: Helper = Helper {
     program: "powershell.exe",
-    args: &[
-        "-NoProfile",
-        "-NonInteractive",
-        "-Command",
-        "[Console]::InputEncoding = [Text.Encoding]::UTF8; Set-Clipboard -Value ([Console]::In.ReadToEnd())",
-    ],
+    args: POWERSHELL_ARGS,
     encoding: Encoding::Utf8,
 };
-
-/// Pick the clipboard helper, given the session and which programs exist.
+const POWERSHELL_ARGS: &[&str] = &[
+    "-NoProfile",
+    "-NonInteractive",
+    "-Command",
+    "[Console]::InputEncoding = [Text.Encoding]::UTF8; Set-Clipboard -Value ([Console]::In.ReadToEnd())",
+];
+
+/// `appendWindowsPath = false` in `/etc/wsl.conf` takes the Windows
+/// directories off PATH, so neither `clip.exe` nor `powershell.exe` is found
+/// by name. WSL still mounts the Windows drive: at `/mnt/c` by default, at
+/// `/c` with `[automount] root = /`.
+const WSL_CLIP_EXE: [Helper; 2] = [
+    Helper {
+        program: "/mnt/c/Windows/System32/clip.exe",
+        ..CLIP_EXE
+    },
+    Helper {
+        program: "/c/Windows/System32/clip.exe",
+        ..CLIP_EXE
+    },
+];
+const WSL_POWERSHELL: [Helper; 2] = [
+    Helper {
+        program: "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe",
+        ..POWERSHELL
+    },
+    Helper {
+        program: "/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe",
+        ..POWERSHELL
+    },
+];
+
+/// Pick the clipboard helper, given the session and which programs exist
+/// (`available` gets a bare name to look up on PATH, or an absolute path).
 ///
 /// Ordered by how specific the signal is: a Wayland session that also exports
 /// `DISPLAY` (XWayland) should still get `wl-copy`, and Termux (which may run
@@ -263,7 +291,11 @@ pub fn choose_helper(env: HelperEnv, available: &dyn Fn(&str) -> bool) -> Option
         (XCLIP, env.x11),
         (XSEL, env.x11),
         (CLIP_EXE, true),
+        (WSL_CLIP_EXE[0], env.wsl),
+        (WSL_CLIP_EXE[1], env.wsl),
         (POWERSHELL, env.wsl),
+        (WSL_POWERSHELL[0], env.wsl),
+        (WSL_POWERSHELL[1], env.wsl),
     ];
     candidates
         .into_iter()
@@ -273,7 +305,16 @@ pub fn choose_helper(env: HelperEnv, available: &dyn Fn(&str) -> bool) -> Option
 
 /// The clipboard helper to use on this machine.
 pub fn native_helper() -> Option<Helper> {
-    choose_helper(HelperEnv::detect(), &on_path)
+    choose_helper(HelperEnv::detect(), &program_exists)
+}
+
+/// A bare program name is looked up on PATH; a path is checked directly.
+fn program_exists(program: &str) -> bool {
+    if program.contains('/') {
+        std::path::Path::new(program).is_file()
+    } else {
+        on_path(program)
+    }
 }
 
 fn on_path(program: &str) -> bool {
@@ -485,6 +526,48 @@ mod tests {
         assert_eq!(pick(HelperEnv::default(), &["clip.exe"]), Some("clip.exe"));
     }
 
+    #[test]
+    fn wsl_without_the_windows_path_finds_the_helpers_on_the_c_drive() {
+        let wsl = HelperEnv {
+            wsl: true,
+            ..Default::default()
+        };
+        // appendWindowsPath=false: nothing by name, the drive still mounted.
+        let mounted = &[
+            "/mnt/c/Windows/System32/clip.exe",
+            "/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe",
+        ];
+        let clip = choose_helper(wsl, &only(mounted)).unwrap();
+        assert_eq!(clip.program, "/mnt/c/Windows/System32/clip.exe");
+        assert_eq!(clip.encoding, Encoding::Utf16LeBom);
+        // No clip.exe at all: PowerShell by its full path, same arguments.
+        let ps = pick(
+            wsl,
+            &["/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe"],
+        );
+        assert_eq!(
+            ps,
+            Some("/mnt/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe")
+        );
+        let ps = choose_helper(
+            wsl,
+            &only(&["/c/Windows/System32/WindowsPowerShell/v1.0/powershell.exe"]),
+        )
+        .unwrap();
+        assert_eq!(ps.args, POWERSHELL.args);
+        // `[automount] root = /` mounts the drive at /c.
+        assert_eq!(
+            pick(wsl, &["/c/Windows/System32/clip.exe"]),
+            Some("/c/Windows/System32/clip.exe")
+        );
+        // On PATH still wins; outside WSL the mount paths are never probed.
+        assert_eq!(
+            pick(wsl, &["clip.exe", "/mnt/c/Windows/System32/clip.exe"]),
+            Some("clip.exe")
+        );
+        assert_eq!(pick(HelperEnv::default(), mounted), None);
+    }
+
     #[test]
     fn helper_input_bytes_match_the_helper() {
         let wsl = HelperEnv {

src/image.rs

@@ -42,13 +42,19 @@ const IMAGE_CACHE_CAP: usize = 64;
 /// decodes could pin gigabytes.
 const IMAGE_CACHE_MAX_BYTES: usize = 256 * 1024 * 1024;
 
-/// Decode limits for raster images. Anything wider or taller than this is
-/// refused before its pixels are allocated: a tiny, highly compressible PNG
-/// can declare 11000×11000 and expand to hundreds of MB (a decompression
-/// bomb), and inline `data:` URIs deliver one with no network at all.
-/// 8192 px still admits every real photo (a 48 MP sensor is 8000×6000).
-const RASTER_MAX_SIDE_PX: u32 = 8192;
-/// Hard cap on the bytes a single raster decode may allocate (8192² RGBA8).
+/// Per-side limit for raster images: far beyond any real image (full-page
+/// screenshots run 1280×12000+, panoramas 30000 px wide), so it rejects only
+/// nonsense headers. It is not what stops decompression bombs — a tiny,
+/// highly compressible PNG can declare 11000×11000 and expand to hundreds of
+/// MB — [`RASTER_MAX_ALLOC`] is.
+const RASTER_MAX_SIDE_PX: u32 = 65_535;
+/// Hard cap on the bytes a single raster decode may allocate, checked
+/// against the decoder's declared output size before any pixels exist. It
+/// admits a 61 MP photo (9504×6336: ≈180 MB as RGB8, ≈241 MB as RGBA8) and
+/// refuses e.g. 11000×11000 RGBA (≈484 MB). The trade-off: a bomb under the
+/// cap (11000×11000 grey, 121 MB) does decode, transiently, one image at a
+/// time; it is downscaled to [`CACHED_MAX_SIDE_PX`] before caching, so what
+/// stays in memory is a few MB.
 const RASTER_MAX_ALLOC: u64 = 256 * 1024 * 1024;
 /// Decoded rasters are downscaled to at most this many pixels per side
 /// before caching. Both consumers — half-block rendering (≤60 cells wide) and
@@ -868,33 +874,82 @@ mod tests {
         out
     }
 
+    /// A PNG whose header declares `w`×`h` RGBA pixels, with no pixel data:
+    /// enough for the decoder to report its output size.
+    fn rgba_png_header(w: u32, h: u32) -> Vec<u8> {
+        let mut png = blank_png(1, 1);
+        // IHDR data starts after the 8-byte signature and 8-byte chunk head.
+        png[16..20].copy_from_slice(&w.to_be_bytes());
+        png[20..24].copy_from_slice(&h.to_be_bytes());
+        png[25] = 6; // RGBA
+        let crc = {
+            let mut c = flate2::Crc::new();
+            c.update(&png[12..29]);
+            c.sum()
+        };
+        png[29..33].copy_from_slice(&crc.to_be_bytes());
+        png
+    }
+
     // Regression: decode_raster set no limits, so a ~100 KB PNG declaring
     // 11000×11000 expanded to hundreds of MB (and 64 of them stayed cached).
-    // It must be refused before allocation, as a normal decode failure.
+    // The allocation cap bounds every decode; a grey 11000×11000 (121 MB)
+    // fits under it, so it decodes once and is cached at ≤ 2048 px a side.
     #[test]
-    fn decompression_bomb_png_is_rejected() {
+    fn decompression_bomb_png_stays_bounded() {
         let png = blank_png(11_000, 11_000);
         assert!(png.len() < 1024 * 1024, "bomb fixture should be tiny");
-        assert!(
-            decode_raster(&png).is_none(),
-            "over-limit PNG must not decode"
-        );
-        // Through the public path (inline data: URI, no flag needed) it is
-        // the ordinary "could not be decoded" failure, not a panic.
+        // Through the public path (inline data: URI, no flag needed): what
+        // the cache keeps is ≤ 2048 px a side, so six copies cost six small
+        // entries, not 6 × 121 MB.
         use base64::Engine;
         let uri = format!(
             "data:image/png;base64,{}",
             base64::engine::general_purpose::STANDARD.encode(&png)
         );
-        assert_eq!(
-            load_decoded(&uri, None, ImageMode::LocalOnly, None).err(),
-            Some(ImageUnavailable::Failed)
+        let img = load_decoded(&uri, None, ImageMode::LocalOnly, None)
+            .expect("a 121 MB grey decode is under the cap");
+        assert_eq!((img.width(), img.height()), (2048, 2048));
+        assert!(
+            img.as_bytes().len() <= 2048 * 2048 * 4,
+            "cached size is small"
         );
+        // Past the allocation cap it is refused before any pixel exists: the
+        // same 11000×11000 as RGBA is ≈484 MB.
+        assert!(decode_raster(&rgba_png_header(11_000, 11_000)).is_none());
+        assert!(decode_raster(&rgba_png_header(65_535, 65_535)).is_none());
+        // Beyond the per-side limit, too.
+        assert!(decode_raster(&rgba_png_header(70_000, 1)).is_none());
+    }
+
+    #[test]
+    fn tall_screenshots_and_wide_panoramas_decode() {
+        // Regression: an 8192 px side limit refused full-page screenshots.
+        let tall = decode_raster(&blank_png(1280, 12_000)).expect("1280×12000 decodes");
+        assert_eq!(tall.height(), CACHED_MAX_SIDE_PX);
+        assert_eq!(tall.width(), 218, "aspect ratio preserved");
+        let wide = decode_raster(&blank_png(20_000, 1_000)).expect("panorama decodes");
+        assert_eq!(wide.width(), CACHED_MAX_SIDE_PX);
+        // A 61 MP RGB photo declares ≈180 MB: under the cap. (Checked on the
+        // header alone; encoding a real one here would be slow.)
+        let mut photo = rgba_png_header(9504, 6336);
+        photo[25] = 2; // RGB
+        let mut c = flate2::Crc::new();
+        c.update(&photo[12..29]);
+        photo[29..33].copy_from_slice(&c.sum().to_be_bytes());
+        let reader = image::ImageReader::new(std::io::Cursor::new(&photo))
+            .with_guessed_format()
+            .unwrap();
+        use image::ImageDecoder;
+        let decoder = reader.into_decoder().unwrap();
+        assert!(decoder.total_bytes() <= RASTER_MAX_ALLOC);
+        let rgba_61mp = 9504u64 * 6336 * 4;
+        assert!(rgba_61mp <= RASTER_MAX_ALLOC, "even as RGBA8");
     }
 
     #[test]
     fn image_at_the_limit_decodes_and_is_downscaled_for_the_cache() {
-        let png = blank_png(RASTER_MAX_SIDE_PX, 16);
+        let png = blank_png(16_384, 16);
         let img = decode_raster(&png).expect("an image within the limits decodes");
         assert_eq!(img.width(), CACHED_MAX_SIDE_PX, "wide image downscaled");
         assert!(img.height() >= 1);