From 8dd77d675a2753704153672027093eade3ba10e4 Mon Sep 17 00:00:00 2001 From: Marc Sallin Date: Fri, 4 Sep 2026 13:06:32 +0200 Subject: [PATCH] fix: resolve the input path against the root before making it virtual A Typst virtual path only accepts forward slashes, and `VirtualPath::new` was handed the caller's path verbatim. On Windows an ordinary relative path such as `templates\letter.typ` therefore failed to convert, and the `unwrap` turned that into a panic inside `create_compiler`. That function is `extern "C"` and has no unwind guard, so the panic aborted the whole host process rather than failing the single call. Relative paths are now joined onto the project root and converted with `VirtualPath::virtualize`, which splits on the platform separator and does the root containment check itself. A path that genuinely escapes the root reports an error through the existing channel instead of panicking, and the manual UTF-8 check is no longer needed because `virtualize` covers it. The pack job gains a `cargo test` step. The Rust tests exercise the FFI entry points directly, which is the only place a panic crossing the boundary surfaces as a failing test rather than as a crashed test runner. --- .github/workflows/pack.yml | 7 ++ RELEASENOTES.md | 3 + src/typst_core/src/world.rs | 34 +++--- src/typst_core/tests/input_path.rs | 186 +++++++++++++++++++++++++++++ src/typstsharp.tests/Tests.cs | 126 +++++++++++++++++++ 5 files changed, 342 insertions(+), 14 deletions(-) create mode 100644 src/typst_core/tests/input_path.rs diff --git a/.github/workflows/pack.yml b/.github/workflows/pack.yml index 1d91035..5aa4b89 100644 --- a/.github/workflows/pack.yml +++ b/.github/workflows/pack.yml @@ -49,6 +49,13 @@ jobs: with: package-version: ${{ env.PACKAGE_VERSION }} + # The Rust tests cover the FFI entry points directly. That is the only place a + # panic crossing the boundary shows up as a failing test rather than as a + # crashed test runner, so they have to run in their own right. + - name: Run native tests + shell: bash + run: cargo test --manifest-path src/typst_core/Cargo.toml --release + - name: Run tests shell: bash run: dotnet test --solution typstsharp.slnx diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 3da7d77..afcb262 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,6 +1,9 @@ # Release Notes ## [Unreleased] +### Fixed +- Fixed an input path in a subfolder aborting the process instead of compiling. The path was handed to Typst verbatim, and a Typst virtual path only accepts forward slashes, so on Windows an ordinary relative path such as `templates\letter.typ` panicked inside a native call and took the host process down with it. Paths are now resolved against the project root before being converted, and a path that genuinely leaves the root reports an error instead of panicking. Note that the root is matched against an absolute input path textually, so on Windows both have to be spelled with the same casing. + ### Added - Added `compiler.CompileToDocument(...)`, returning a disposable `TypstDocument` that exposes the rendered output while it is still in the memory the native library allocated. `GetOutputSpan`, `OpenOutputStream`, `CopyOutputTo`, `WriteOutputToFile` and `RentOutput` read it without putting a multi-megabyte PDF on the large object heap; `GetOutputBytes` copies when a `byte[]` is what you need. - Added easier PDF compilation APIs on `TypstCompiler`: diff --git a/src/typst_core/src/world.rs b/src/typst_core/src/world.rs index 699d286..1e94220 100644 --- a/src/typst_core/src/world.rs +++ b/src/typst_core/src/world.rs @@ -86,23 +86,29 @@ impl SystemWorld { fonts.extend(typst_kit::fonts::scan(path)); } - // Resolve the main file path relative to the root - // If the input path is absolute, try to make it relative to the root. - // If it's already relative, assume it's relative to the root. + // Resolve the main file path relative to the root. A relative input path is + // taken to be relative to the root, so joining it onto the root first leaves + // `virtualize` with a single job: translate a real path into a virtual one and + // check that it stays inside the root. + // + // Going through `Path` rather than handing the string to `VirtualPath::new` + // matters because a virtual path only accepts forward slashes. On Windows the + // separator in `templates\letter.typ` is an ordinary one, and `Path` splits on + // it; `VirtualPath::new` would instead reject the whole string. + // + // Input (Windows): root `C:\app`, path `templates\letter.typ` + // Output: virtual path `/templates/letter.typ` let main_id = if let Some(path) = input_path { - let relative_path = if path.is_absolute() { - path.strip_prefix(&root).map_err(|_| { - eco_format!("input file must be contained in the project root") - })? - } else { - &path - }; - let relative_str = relative_path.to_str().ok_or_else(|| { - eco_format!("input file path must be valid UTF-8") + let absolute = if path.is_absolute() { path } else { root.join(path) }; + let virtual_path = VirtualPath::virtualize(&root, &absolute).map_err(|err| { + eco_format!("invalid input file path `{}`: {err}", absolute.display()) })?; - RootedPath::new(VirtualRoot::Project, VirtualPath::new(relative_str).unwrap()).intern() + RootedPath::new(VirtualRoot::Project, virtual_path).intern() } else { - FileId::unique(RootedPath::new(VirtualRoot::Project, VirtualPath::new("
").unwrap())) + FileId::unique(RootedPath::new( + VirtualRoot::Project, + VirtualPath::new("
").expect("`
` is a valid virtual path"), + )) }; let mut slots = HashMap::new(); diff --git a/src/typst_core/tests/input_path.rs b/src/typst_core/tests/input_path.rs new file mode 100644 index 0000000..5d12d18 --- /dev/null +++ b/src/typst_core/tests/input_path.rs @@ -0,0 +1,186 @@ +//! Tests for how an on-disk input path is turned into Typst's virtual main file. +//! +//! A virtual path only accepts forward slashes and may not leave the project root. +//! Both constraints are properties of the path the caller supplies, so they have to +//! be reported as errors rather than taking the process down. + +use std::ffi::{c_char, CString}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use typst_core::{Compiler, compile, create_compiler, free_compile_result, free_compiler}; + +/// A throwaway directory holding a template, removed when the test ends. +struct Project { + root: PathBuf, +} + +impl Project { + /// Creates a project root containing `templates/letter.typ`. + fn new(name: &str) -> Self { + // A counter keeps parallel tests apart without pulling in a temp-file crate. + static COUNTER: AtomicUsize = AtomicUsize::new(0); + let unique = COUNTER.fetch_add(1, Ordering::Relaxed); + + let root = std::env::temp_dir().join(format!( + "typst_core-{}-{}-{}", + name, + std::process::id(), + unique + )); + std::fs::create_dir_all(root.join("templates")).unwrap(); + std::fs::write(root.join("templates").join("letter.typ"), "= Dear customer").unwrap(); + Self { root } + } +} + +impl Drop for Project { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.root); + } +} + +/// Creates a compiler over a file, the way `TypstCompiler.FromFile` does. +fn compiler_for_file(root: &Path, input_path: &str) -> *mut Compiler { + let root = CString::new(root.to_str().unwrap()).unwrap(); + let input_path = CString::new(input_path).unwrap(); + let sys_inputs = CString::new("{}").unwrap(); + + create_compiler( + root.as_ptr(), + input_path.as_ptr(), + std::ptr::null(), + 0, + std::ptr::null::<*const c_char>(), + 0, + std::ptr::null(), + sys_inputs.as_ptr(), + true, + true, + ) +} + +/// Compiles to a PDF and returns its length, failing the test on a compiler error. +fn compile_to_pdf_len(compiler: *mut Compiler) -> usize { + let result = compile(compiler, std::ptr::null(), 96.0, std::ptr::null()); + + assert!( + result.error_ptr.is_null(), + "compilation failed: {}", + unsafe { + String::from_utf8_lossy(std::slice::from_raw_parts( + result.error_ptr, + result.error_len, + )) + .into_owned() + } + ); + assert_eq!(result.buffers_len, 1, "expected exactly one PDF buffer"); + + let len = unsafe { (*result.buffers).len }; + free_compile_result(result); + len +} + +/// A template in a subfolder is addressed with the platform's own separator. On +/// Windows that is a backslash, which a virtual path rejects outright, so the path +/// has to be resolved through `Path` rather than handed over as a string. +#[test] +fn nested_relative_input_path_is_compiled() { + let project = Project::new("nested-relative"); + let input_path = Path::new("templates") + .join("letter.typ") + .to_str() + .unwrap() + .to_owned(); + + let compiler = compiler_for_file(&project.root, &input_path); + assert!( + !compiler.is_null(), + "`{input_path}` was rejected as an input path" + ); + + assert!(compile_to_pdf_len(compiler) > 0, "produced an empty PDF"); + free_compiler(compiler); +} + +/// The separator is spelled out rather than taken from `Path`, so this stays a +/// regression test even if the platform-independent one above is ever simplified to a +/// forward-slash literal, which the old code already accepted. +#[cfg(windows)] +#[test] +fn windows_separator_in_the_input_path_is_compiled() { + let project = Project::new("windows-separator"); + + let compiler = compiler_for_file(&project.root, "templates\\letter.typ"); + assert!( + !compiler.is_null(), + "`templates\\letter.typ` was rejected as an input path" + ); + + assert!(compile_to_pdf_len(compiler) > 0, "produced an empty PDF"); + free_compiler(compiler); +} + +/// A path that steps out of a subfolder and back in never leaves the root, so it +/// resolves rather than being refused. This is the boundary of the escape check. +#[test] +fn input_path_leaving_and_reentering_the_root_is_compiled() { + let project = Project::new("reentering"); + let input_path = Path::new("templates") + .join("..") + .join("templates") + .join("letter.typ") + .to_str() + .unwrap() + .to_owned(); + + let compiler = compiler_for_file(&project.root, &input_path); + assert!(!compiler.is_null(), "`{input_path}` was rejected"); + + assert!(compile_to_pdf_len(compiler) > 0, "produced an empty PDF"); + free_compiler(compiler); +} + +/// The same file named absolutely resolves to the same document. +#[test] +fn absolute_input_path_inside_the_root_is_compiled() { + let project = Project::new("absolute-inside"); + let input_path = project.root.join("templates").join("letter.typ"); + + let compiler = compiler_for_file(&project.root, input_path.to_str().unwrap()); + assert!(!compiler.is_null(), "absolute input path was rejected"); + + assert!(compile_to_pdf_len(compiler) > 0, "produced an empty PDF"); + free_compiler(compiler); +} + +/// A relative path that climbs out of the root has to be refused. Reaching outside +/// the root is what the root is for, and the refusal must be an error result rather +/// than a panic: this runs inside an `extern "C"` function, where an unwind aborts +/// the host process. +#[test] +fn relative_input_path_escaping_the_root_is_refused() { + let project = Project::new("relative-escape"); + let input_path = Path::new("..") + .join("outside.typ") + .to_str() + .unwrap() + .to_owned(); + + let compiler = compiler_for_file(&project.root, &input_path); + assert!( + compiler.is_null(), + "`{input_path}` escapes the root and must be refused" + ); +} + +/// An absolute path outside the root is refused for the same reason. +#[test] +fn absolute_input_path_outside_the_root_is_refused() { + let project = Project::new("absolute-outside"); + let outside = std::env::temp_dir().join("typst_core-outside.typ"); + + let compiler = compiler_for_file(&project.root, outside.to_str().unwrap()); + assert!(compiler.is_null(), "path outside the root must be refused"); +} diff --git a/src/typstsharp.tests/Tests.cs b/src/typstsharp.tests/Tests.cs index 475e46c..a4494bb 100644 --- a/src/typstsharp.tests/Tests.cs +++ b/src/typstsharp.tests/Tests.cs @@ -50,6 +50,105 @@ public async Task CompilePdfStaticFromFile() } } + /// + /// A template kept in a subfolder is addressed with the platform's own separator, + /// which on Windows is a backslash. Typst's virtual paths only accept forward + /// slashes, so the path has to be translated rather than passed through verbatim. + /// + [Test] + public async Task NestedRelativeInputPathIsCompiled() + { + using var project = new ProjectDirectory(); + project.AddTemplate(Path.Combine("templates", "letter.typ"), "= Dear customer"); + + using var compiler = TypstCompiler.FromFile( + Path.Combine("templates", "letter.typ"), + root: project.Path); + + await Assert.That(GetPlainText(compiler.CompilePdf())).Contains("Dear customer"); + } + + /// + /// The same template addressed absolutely resolves to the same document. + /// + [Test] + public async Task AbsoluteInputPathInsideTheRootIsCompiled() + { + using var project = new ProjectDirectory(); + var template = project.AddTemplate(Path.Combine("templates", "letter.typ"), "= Dear customer"); + + using var compiler = TypstCompiler.FromFile(template, root: project.Path); + + await Assert.That(GetPlainText(compiler.CompilePdf())).Contains("Dear customer"); + } + + /// + /// Reaching outside the project root has to fail as an exception. The check runs + /// inside a native call, where an unwinding panic would abort the whole process + /// instead of failing the single request. + /// + [Test] + public async Task InputPathEscapingTheRootIsRejected() + { + using var project = new ProjectDirectory(); + project.AddTemplate("letter.typ", "= Dear customer"); + var nested = Path.Combine(project.Path, "templates"); + Directory.CreateDirectory(nested); + + await Assert.That(() => + { + using var compiler = TypstCompiler.FromFile( + Path.Combine("..", "letter.typ"), + root: nested); + }).Throws() + .WithMessageContaining("Failed to create Typst compiler"); + } + + /// + /// A path that steps out of a subfolder and back in stays inside the root, so it + /// resolves rather than being refused. + /// + [Test] + public async Task InputPathLeavingAndReenteringTheRootIsCompiled() + { + using var project = new ProjectDirectory(); + project.AddTemplate("letter.typ", "= Dear customer"); + project.AddTemplate(Path.Combine("templates", "unused.typ"), "= Unused"); + + using var compiler = TypstCompiler.FromFile( + Path.Combine("templates", "..", "letter.typ"), + root: project.Path); + + await Assert.That(GetPlainText(compiler.CompilePdf())).Contains("Dear customer"); + } + + /// + /// The simplest way to reach this API passes no root at all, which leaves the + /// project root at the process working directory. The working directory is + /// process-wide state, so this test must not run alongside another. + /// + [Test] + [NotInParallel] + public async Task NestedRelativeInputPathWithoutARootIsCompiled() + { + using var project = new ProjectDirectory(); + project.AddTemplate(Path.Combine("templates", "letter.typ"), "= Dear customer"); + + var previous = Directory.GetCurrentDirectory(); + try + { + Directory.SetCurrentDirectory(project.Path); + + using var compiler = TypstCompiler.FromFile(Path.Combine("templates", "letter.typ")); + + await Assert.That(GetPlainText(compiler.CompilePdf())).Contains("Dear customer"); + } + finally + { + Directory.SetCurrentDirectory(previous); + } + } + [Test] public async Task CompilePdfToFileAndAsync() { @@ -751,6 +850,33 @@ private static string GetPlainText(byte[] pdf) } } +/// +/// A throwaway project root holding Typst templates, removed when the test ends. +/// +internal sealed class ProjectDirectory : IDisposable +{ + public ProjectDirectory() => Directory.CreateDirectory(Path); + + public string Path { get; } = System.IO.Path.Combine(System.IO.Path.GetTempPath(), $"typstsharp-project-{Guid.NewGuid():N}"); + + /// Writes a template at a root-relative path and returns its full path. + public string AddTemplate(string relativePath, string source) + { + var fullPath = System.IO.Path.Combine(Path, relativePath); + Directory.CreateDirectory(System.IO.Path.GetDirectoryName(fullPath)!); + File.WriteAllText(fullPath, source); + return fullPath; + } + + public void Dispose() + { + if (Directory.Exists(Path)) + { + Directory.Delete(Path, recursive: true); + } + } +} + /// /// A throwaway package directory laid out the way Typst expects: one directory level per /// namespace, package name and version.