From cf640c4b08df98a60669141cd97e8e3e4e22598c Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Wed, 14 Oct 2026 13:07:49 +0200 Subject: [PATCH] CLI: convert and run refuse output formats DeckCraft can't write (#715) convert, run --save and run --export picked the format from the output extension and fell back to native .deckcraft bytes for any extension they didn't know, so out.odp, out.gif, out.svg and out.outline were zips with exit status 0. They now fail before writing anything, with the extension and the supported ones in the error. --- apps/deckcraft-cli/src/main.rs | 15 +++++++- apps/deckcraft-cli/tests/convert.rs | 57 +++++++++++++++++++++++++++++ crates/engine/src/cmd/file.rs | 11 ++++++ crates/engine/src/tests.rs | 13 +++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 apps/deckcraft-cli/tests/convert.rs diff --git a/apps/deckcraft-cli/src/main.rs b/apps/deckcraft-cli/src/main.rs index fd1d96ea..3c4d618b 100644 --- a/apps/deckcraft-cli/src/main.rs +++ b/apps/deckcraft-cli/src/main.rs @@ -240,9 +240,9 @@ fn render_transition(args: &[String]) -> Result<(), String> { fn convert(args: &[String]) -> Result<(), String> { let (Some(input), Some(out)) = (args.first(), args.get(1)) else { return Err("convert needs IN and OUT".into()) }; + let fmt = output_format(out)?; let mut s = Session::new(); open(&mut s, Some(input), false)?; - let fmt = deckcraft_engine::cmd::file::format_for_path(out); let r = if matches!(fmt, "png" | "jpeg") { s.execute("file.export", &json!({"path": out, "slide": 0})) } else { @@ -253,6 +253,16 @@ fn convert(args: &[String]) -> Result<(), String> { Ok(()) } +/// The format `convert`, `run --save` and `run --export` write to `path`, or an error naming the +/// supported extensions when `path`'s extension isn't one (rather than native bytes under that name). +fn output_format(path: &str) -> Result<&'static str, String> { + use deckcraft_engine::cmd::file::{WRITABLE_EXTENSIONS, writable_format_for_path}; + writable_format_for_path(path).ok_or_else(|| { + let ext = std::path::Path::new(path).extension().map(|e| format!(".{}", e.to_string_lossy())).unwrap_or_else(|| "(none)".into()); + format!("{path}: unsupported output format {ext} (supported: .{})", WRITABLE_EXTENSIONS.join(", .")) + }) +} + fn run(args: &[String]) -> Result<(), String> { let mut input = None; let mut sample = false; @@ -272,6 +282,9 @@ fn run(args: &[String]) -> Result<(), String> { other => return Err(format!("unknown option `{other}`")), } } + for p in save.iter().chain(&export) { + output_format(p)?; + } let mut s = Session::new(); open(&mut s, input.as_deref(), sample)?; for c in cmds { diff --git a/apps/deckcraft-cli/tests/convert.rs b/apps/deckcraft-cli/tests/convert.rs new file mode 100644 index 00000000..5de25e51 --- /dev/null +++ b/apps/deckcraft-cli/tests/convert.rs @@ -0,0 +1,57 @@ +//! `deckcraft-cli convert` writes the format OUT's extension names, or fails without writing. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn cli(args: &[&Path]) -> Output { + Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(args).output().unwrap() +} + +fn temp_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("deckcraft-cli-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +#[test] +fn convert_rejects_output_formats_it_cannot_write() { + let dir = temp_dir("convert-unsupported"); + let input = dir.join("in.pptx"); + let out = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(["run", "--sample", "--export"]).arg(&input).output().unwrap(); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + + for name in ["out.odp", "out.gif", "out.svg", "out.outline", "out"] { + let target = dir.join(name); + let out = cli(&[Path::new("convert"), &input, &target]); + assert!(!out.status.success(), "{name}: convert should fail"); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains("unsupported output format") && err.contains(".pptx"), "{name}: {err}"); + assert!(!target.exists(), "{name}: nothing is written"); + } + + let target = dir.join("out.deckcraft"); + let out = cli(&[Path::new("convert"), &input, &target]); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + assert!(std::fs::read(&target).unwrap().starts_with(b"PK\x03\x04")); + + let target = dir.join("out.txt"); + let out = cli(&[Path::new("convert"), &input, &target]); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + assert!(!std::fs::read(&target).unwrap().starts_with(b"PK"), "an outline is text, not a zip"); + + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn run_rejects_unsupported_save_and_export_paths_before_running() { + let dir = temp_dir("run-unsupported"); + for flag in ["--save", "--export"] { + let target = dir.join("deck.odp"); + let out = Command::new(env!("CARGO_BIN_EXE_deckcraft-cli")).args(["run", "--sample", flag]).arg(&target).output().unwrap(); + assert!(!out.status.success(), "{flag}: run should fail"); + assert!(String::from_utf8_lossy(&out.stderr).contains("unsupported output format .odp"), "{flag}"); + assert!(!target.exists(), "{flag}: nothing is written"); + } + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index f6424f7c..89dea68f 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -182,6 +182,17 @@ pub fn pdf_bytes(doc: &Presentation, p: &Value) -> Result> { deckcraft_pdf::export(doc, &pdf_options(p, doc.slides.len())?).map_err(|e| EngineError::Other(e.to_string())) } +/// The output extensions [`format_for_path`] maps to a writer of that format. +pub const WRITABLE_EXTENSIONS: &[&str] = &["deckcraft", "pptx", "potx", "ppsx", "pdf", "png", "jpg", "jpeg", "txt", "md"]; + +/// The format a file named `path` is written in, or `None` when its extension names no format +/// DeckCraft writes (`.odp`, `.gif`, `.svg`, no extension …): [`format_for_path`] would save +/// those as native `.deckcraft` bytes under the wrong name. +pub fn writable_format_for_path(path: &str) -> Option<&'static str> { + let ext = std::path::Path::new(path).extension()?.to_str()?.to_ascii_lowercase(); + WRITABLE_EXTENSIONS.contains(&ext.as_str()).then(|| format_for_path(path)) +} + pub fn format_for_path(path: &str) -> &'static str { let l = path.to_ascii_lowercase(); if l.ends_with(".pptx") || l.ends_with(".potx") || l.ends_with(".ppsx") { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e83f72ef..2727a717 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -181,6 +181,19 @@ fn sample_deck_builds_and_renders() { assert!(v["slides"].as_array().unwrap().len() >= 8); } +#[test] +fn writable_format_for_path_rejects_formats_deckcraft_does_not_write() { + use crate::cmd::file::writable_format_for_path as f; + assert_eq!(f("a.deckcraft"), Some("deckcraft")); + assert_eq!(f("dir/A.PPTX"), Some("pptx")); + assert_eq!(f("a.pdf"), Some("pdf")); + assert_eq!(f("a.jpg"), Some("jpeg")); + assert_eq!(f("a.txt"), Some("outline")); + for path in ["a.odp", "a.gif", "a.svg", "a.outline", "a", "dir.v2/a"] { + assert_eq!(f(path), None, "{path}"); + } +} + #[test] fn save_and_reopen_native() { let mut s = Session::new();