diff --git a/crates/rw-confluence/src/renderer.rs b/crates/rw-confluence/src/renderer.rs index 63a21c1e..930100c9 100644 --- a/crates/rw-confluence/src/renderer.rs +++ b/crates/rw-confluence/src/renderer.rs @@ -12,7 +12,8 @@ use rw_diagrams::{ ResolveContext, Resolved, }; use rw_renderer::{ - IncludeError, IncludeReader, MarkdownRenderer, ProjectFiles, RenderResult, image_file, + IncludeError, IncludeReader, IncludedFile, MarkdownRenderer, ProjectFiles, RenderResult, + image_file, }; use rw_storage::file_content_type; @@ -119,7 +120,7 @@ pub(crate) fn render_page( struct SharedIncludes(Arc); impl IncludeReader for SharedIncludes { - fn read(&self, path: &str) -> Result { + fn read(&self, path: &str) -> Result { self.0.read(path) } } diff --git a/crates/rw-confluence/tests/render_integration.rs b/crates/rw-confluence/tests/render_integration.rs index 65ad2382..497fa5bc 100644 --- a/crates/rw-confluence/tests/render_integration.rs +++ b/crates/rw-confluence/tests/render_integration.rs @@ -12,7 +12,7 @@ use rw_confluence::{ }; use rw_error::error_chain; use rw_kroki::test_support::{KrokiStub, PNG_HEADER, Reply, Request}; -use rw_renderer::IncludeError; +use rw_renderer::{IncludeError, IncludedFile}; use rw_storage::{MockStorage, Storage, image_content_type}; #[test] @@ -409,10 +409,15 @@ fn render_bundles_a_local_image_as_an_attachment_named_after_its_content() { struct Includes(Arc); impl IncludeReader for Includes { - fn read(&self, path: &str) -> Result { - self.0 + fn read(&self, path: &str) -> Result { + let include = self + .0 .read_include(path, None) - .map_err(|_| IncludeError::NotFound) + .map_err(|_| IncludeError::NotFound)?; + Ok(IncludedFile { + text: include.text, + canonical_path: include.canonical_path, + }) } } diff --git a/crates/rw-renderer/src/include.rs b/crates/rw-renderer/src/include.rs index 1a234971..8611bdb5 100644 --- a/crates/rw-renderer/src/include.rs +++ b/crates/rw-renderer/src/include.rs @@ -15,14 +15,17 @@ use crate::file::{SEPARATORS, fold, is_external}; /// # Examples /// /// ``` -/// use rw_renderer::{HtmlBackend, IncludeError, IncludeReader, MarkdownRenderer}; +/// use rw_renderer::{HtmlBackend, IncludeError, IncludeReader, IncludedFile, MarkdownRenderer}; /// /// struct Partials; /// /// impl IncludeReader for Partials { -/// fn read(&self, path: &str) -> Result { +/// fn read(&self, path: &str) -> Result { /// match path { -/// "docs/_partials/note.md" => Ok("Mind the gap.".to_owned()), +/// "docs/_partials/note.md" => Ok(IncludedFile { +/// text: "Mind the gap.".to_owned(), +/// canonical_path: path.to_owned(), +/// }), /// _ => Err(IncludeError::NotFound), /// } /// } @@ -38,13 +41,24 @@ use crate::file::{SEPARATORS, fold, is_external}; /// assert_eq!(result.warnings, ["include `missing.md` not found"]); /// ``` pub trait IncludeReader: Send + Sync { - /// The text of the markdown file at `path`, or why it can't be read. + /// The markdown file at `path`, or why it can't be read. /// /// `path` is relative to the project root, folded, inside the project as /// written, and ends in `.md` in any case. A symlink may still lead out /// of the project or to a file that is not markdown; the reader refuses /// those. - fn read(&self, path: &str) -> Result; + fn read(&self, path: &str) -> Result; +} + +/// A markdown file an [`IncludeReader`] read. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IncludedFile { + /// The file's text. + pub text: String, + /// The file's project path with symlinks resolved, so that every path to + /// one file gives the same; `path` itself for a reader without symlinks. + /// A file that includes itself through another path is caught by it. + pub canonical_path: String, } /// Why an [`IncludeReader`] returns no text. The page warns with the reason. @@ -132,8 +146,11 @@ mod tests { struct Reader(fn(&str) -> Result<&'static str, IncludeError>); impl IncludeReader for Reader { - fn read(&self, path: &str) -> Result { - (self.0)(path).map(ToOwned::to_owned) + fn read(&self, path: &str) -> Result { + (self.0)(path).map(|text| IncludedFile { + text: text.to_owned(), + canonical_path: path.to_owned(), + }) } } @@ -346,7 +363,8 @@ mod tests { } } - /// Project files by path, and the path of each read, in order. + /// Project files by path, symlinks to them, and the path of each read, in + /// order. struct Files { reader: FileReader, reads: Receiver, @@ -355,6 +373,7 @@ mod tests { #[derive(Clone)] struct FileReader { texts: BTreeMap<&'static str, String>, + links: BTreeMap<&'static str, &'static str>, reads: Sender, } @@ -364,11 +383,17 @@ mod tests { let (sender, reads) = mpsc::channel(); let reader = FileReader { texts: texts.collect(), + links: BTreeMap::new(), reads: sender, }; Self { reader, reads } } + fn with_link(mut self, link: &'static str, target: &'static str) -> Self { + self.reader.links.insert(link, target); + self + } + fn render(&self, markdown: &str) -> RenderResult { MarkdownRenderer::::new() .with_source_path("docs/guide.md") @@ -383,9 +408,15 @@ mod tests { } impl IncludeReader for FileReader { - fn read(&self, path: &str) -> Result { + fn read(&self, path: &str) -> Result { self.reads.send(path.to_owned()).unwrap(); - self.texts.get(path).cloned().ok_or(IncludeError::NotFound) + let canonical_path = self.links.get(path).copied().unwrap_or(path); + let text = self.texts.get(canonical_path).cloned(); + text.map(|text| IncludedFile { + text, + canonical_path: canonical_path.to_owned(), + }) + .ok_or(IncludeError::NotFound) } } @@ -453,6 +484,51 @@ mod tests { ); } + #[test] + fn a_file_that_includes_itself_through_a_symlink_is_caught_on_its_first_repeat() { + let files = Files::new([ + ("docs/guide.md", "Guide.\n"), + ("docs/a.md", "A.\n\n::include[b.md]\n"), + ("docs/b.md", "B.\n\n::include[link.md]\n"), + ("docs/c.md", "C.\n\n::include[c-link.md]\n"), + ]) + .with_link("docs/self.md", "docs/guide.md") + .with_link("docs/link.md", "docs/a.md") + .with_link("docs/c-link.md", "docs/c.md"); + for (markdown, html, warning, reads) in [ + ( + "::include[self.md]\n", + "

::include[self.md]

", + "include `docs/self.md` is `docs/guide.md`, which includes itself", + &["docs/self.md"][..], + ), + ( + "::include[c.md]\n", + "

C.

::include[c-link.md]

", + "docs/c.md: include `docs/c-link.md` is `docs/c.md`, which includes itself", + &["docs/c.md", "docs/c-link.md"], + ), + ( + "::include[a.md]\n", + "

A.

B.

::include[link.md]

", + "docs/b.md: include `docs/link.md` is `docs/a.md`, which includes itself \ + through `docs/b.md`", + &["docs/a.md", "docs/b.md", "docs/link.md"], + ), + ( + "::include[link.md]\n", + "

A.

B.

::include[link.md]

", + "docs/b.md: include `docs/link.md` includes itself through `docs/b.md`", + &["docs/link.md", "docs/b.md"], + ), + ] { + let result = files.render(markdown); + assert_eq!(result.html, html, "{markdown}"); + assert_eq!(result.warnings, [warning], "{markdown}"); + assert_eq!(files.reads(), reads, "{markdown}"); + } + } + #[test] fn a_file_included_twice_renders_twice_and_is_read_once() { let files = Files::new([ diff --git a/crates/rw-renderer/src/lib.rs b/crates/rw-renderer/src/lib.rs index 6dd71db5..5913fa76 100644 --- a/crates/rw-renderer/src/lib.rs +++ b/crates/rw-renderer/src/lib.rs @@ -135,7 +135,7 @@ pub use config::{FileRefusal, FileResolver, ImageSrc, LinkResolver, TitleResolve pub use diagram::{DiagramLink, DiagramView}; pub use file::{ProjectFiles, image_file, is_external}; pub use html::HtmlBackend; -pub use include::{IncludeError, IncludeReader}; +pub use include::{IncludeError, IncludeReader, IncludedFile}; pub use link::encode_url_path; pub use pass::RenderPass; pub use renderer::{MarkdownRenderer, RenderResult}; diff --git a/crates/rw-renderer/src/walker.rs b/crates/rw-renderer/src/walker.rs index 8555e714..c5a7e93e 100644 --- a/crates/rw-renderer/src/walker.rs +++ b/crates/rw-renderer/src/walker.rs @@ -14,7 +14,7 @@ use crate::backend::RenderBackend; use crate::config::RenderConfig; use crate::file; use crate::holes::Holes; -use crate::include; +use crate::include::{self, IncludedFile}; use crate::link; use crate::renderer::RenderResult; use crate::scope::{ImageAs, Scope}; @@ -52,6 +52,17 @@ enum Refusal { File(String), } +/// The page, or a file being included in it. +struct Source { + /// Its project path as written, which warnings name. + path: String, + /// What [`IncludedFile::canonical_path`] says, which cycles are found by. + /// The page's is its path. The scan skips symlinks, so a page has another + /// path only when the docs folder or the README is a symlink; a cycle + /// back through that path renders the page once more before it's caught. + canonical_path: String, +} + /// An open tab group. `bar` is the hole its tab bar fills at group close, /// once every label is known. struct TabGroup { @@ -136,7 +147,7 @@ pub(crate) struct Walker<'r, B: RenderBackend> { /// The page's source, then each file being included, innermost last. /// The top is the file relative images, `.md` links and includes resolve /// from. - sources: Vec, + sources: Vec, warnings: Vec, output: String, /// Where tab bars and diagrams go once known. @@ -170,7 +181,7 @@ pub(crate) struct Walker<'r, B: RenderBackend> { includes: BTreeSet, /// Each file an `::include` line asked for, with its text or the warning /// that refuses it, so a file included several times is read once. - include_reads: HashMap, String>>, + include_reads: HashMap, String>>, /// Lines rendered as a file so far, and the text they added. include_count: usize, include_bytes: usize, @@ -181,7 +192,10 @@ impl<'r, B: RenderBackend> Walker<'r, B> { pub(crate) fn new(cfg: &'r RenderConfig) -> Self { Self { cfg, - sources: Vec::from_iter(cfg.source_path.clone()), + sources: Vec::from_iter(cfg.source_path.iter().map(|path| Source { + path: path.clone(), + canonical_path: path.clone(), + })), warnings: Vec::new(), // Average page-sized documents fit without reallocating. output: String::with_capacity(4096), @@ -341,13 +355,13 @@ impl<'r, B: RenderBackend> Walker<'r, B> { /// The file relative paths resolve from. fn source_path(&self) -> Option<&str> { - self.sources.last().map(String::as_str) + self.sources.last().map(|source| source.path.as_str()) } /// The included file being rendered, which warnings name. fn included_file(&self) -> Option<&str> { match self.sources.split_first() { - Some((_, [.., file])) => Some(file), + Some((_, [.., file])) => Some(&file.path), _ => None, } } @@ -455,7 +469,7 @@ impl<'r, B: RenderBackend> Walker<'r, B> { /// Render the file an `::include` line names in its place, or the line /// as written, with a warning, when it can't. fn include(&mut self, directive: &BlockDirectivePayload) { - let (path, text) = match self.read_include(directive) { + let (path, file) = match self.read_include(directive) { Ok(read) => read, Err(refusal) => { match refusal { @@ -467,26 +481,29 @@ impl<'r, B: RenderBackend> Walker<'r, B> { } }; self.include_count += 1; - self.include_bytes += text.len(); + self.include_bytes += file.text.len(); // The file is a document of its own: a `:::tab` in it can't join // a group the includer opened. let tab_open = mem::take(&mut self.tab_open); - self.sources.push(path); - let mut parser = Parser::fragment(&text, self.cfg.wikilinks, B::TOKENIZE_DIRECTIVES); + self.sources.push(Source { + path, + canonical_path: file.canonical_path.clone(), + }); + let mut parser = Parser::fragment(&file.text, self.cfg.wikilinks, B::TOKENIZE_DIRECTIVES); while let Some(event) = parser.next() { self.handle(event); } - let path = self.sources.pop().expect("the included file is on top"); + let source = self.sources.pop().expect("the included file is on top"); self.tab_open = tab_open; - self.includes.insert(path); + self.includes.insert(source.path); } - /// The project path and text of the file an `::include` line names, or - /// why there is none. + /// The project path and file an `::include` line names, or why there is + /// none. fn read_include( &mut self, directive: &BlockDirectivePayload, - ) -> Result<(String, Rc), Refusal> { + ) -> Result<(String, Rc), Refusal> { let cfg = self.cfg; let (Some(reader), Some(source_path)) = (cfg.include_reader.as_deref(), self.source_path()) else { @@ -508,31 +525,36 @@ impl<'r, B: RenderBackend> Walker<'r, B> { } let path = include::project_path(source_path, src) .map_err(|e| Refusal::File(format!("include `{src}` {e}")))?; - if let Some(at) = self.sources.iter().position(|open| *open == path) { - let through = self.sources[at + 1..].join("`, `"); - return Err(Refusal::File(if through.is_empty() { - format!("include `{path}` includes itself") - } else { - format!("include `{path}` includes itself through `{through}`") - })); + if let Some(at) = self.sources.iter().position(|open| open.path == path) { + return Err(Refusal::File(self.cycle(at, &format!("include `{path}`")))); } if self.include_count >= MAX_INCLUDES { return Err(Refusal::File(format!( "include `{path}` is past the page's limit of {MAX_INCLUDES} includes" ))); } - let text = self + let file = self .include_reads .entry(path.clone()) .or_insert_with(|| { reader .read(&path) - .map(Rc::from) + .map(Rc::new) .map_err(|e| format!("include `{path}` {e}")) }) .clone() .map_err(Refusal::File)?; - if self.include_bytes + text.len() > MAX_INCLUDED_BYTES { + // Another path to an open file, which only the read reveals. + if let Some(at) = self + .sources + .iter() + .position(|open| open.canonical_path == file.canonical_path) + { + let open = &self.sources[at].path; + let subject = format!("include `{path}` is `{open}`, which"); + return Err(Refusal::File(self.cycle(at, &subject))); + } + if self.include_bytes + file.text.len() > MAX_INCLUDED_BYTES { let warning = format!( "include `{path}` is past the page's limit of {} MiB of included text", MAX_INCLUDED_BYTES >> 20 @@ -541,7 +563,24 @@ impl<'r, B: RenderBackend> Walker<'r, B> { self.include_reads.insert(path, Err(warning.clone())); return Err(Refusal::File(warning)); } - Ok((path, text)) + Ok((path, file)) + } + + /// The warning for an include of the file open at `at`, with the files + /// that lead back to it. + fn cycle(&self, at: usize, subject: &str) -> String { + let through: Vec<_> = self.sources[at + 1..] + .iter() + .map(|source| source.path.as_str()) + .collect(); + if through.is_empty() { + format!("{subject} includes itself") + } else { + format!( + "{subject} includes itself through `{}`", + through.join("`, `") + ) + } } /// Render a `ContainerDirectiveEnd` the parser paired: pop the outcome its @@ -1089,8 +1128,11 @@ mod tests { struct Big; impl IncludeReader for Big { - fn read(&self, _path: &str) -> Result { - Ok("a".repeat(MAX_INCLUDED_BYTES + 1)) + fn read(&self, path: &str) -> Result { + Ok(IncludedFile { + text: "a".repeat(MAX_INCLUDED_BYTES + 1), + canonical_path: path.to_owned(), + }) } } diff --git a/crates/rw-site/src/page.rs b/crates/rw-site/src/page.rs index 0c31fb70..d0545f08 100644 --- a/crates/rw-site/src/page.rs +++ b/crates/rw-site/src/page.rs @@ -22,13 +22,14 @@ use rw_kroki::KrokiProvider; use rw_parser::{Rewrite, rewrite}; use rw_plantuml::{DiagramFile, read_diagram_file, search_text}; use rw_renderer::{ - HtmlBackend, IncludeError, IncludeReader, LinkResolver, MAX_INCLUDED_BYTES, MarkdownRenderer, - RenderBackend, SearchDocumentBackend, TitleResolver, TocEntry, escape_html, image_file, + HtmlBackend, IncludeError, IncludeReader, IncludedFile, LinkResolver, MAX_INCLUDED_BYTES, + MarkdownRenderer, RenderBackend, SearchDocumentBackend, TitleResolver, TocEntry, escape_html, + image_file, }; use rw_sections::SectionAnchor; use crate::api::PageMeta; -use crate::path::PageIncludes; +use crate::path::{PageIncludes, included_file}; use crate::site_state::SiteState; use rw_meta::Meta; use rw_storage::{ @@ -695,7 +696,7 @@ struct StorageIncludes { } impl IncludeReader for StorageIncludes { - fn read(&self, path: &str) -> Result { + fn read(&self, path: &str) -> Result { let record = |file: &Path| { self.reads .stamps @@ -717,9 +718,10 @@ impl IncludeReader for StorageIncludes { pub struct ProjectIncludes(pub Arc); impl IncludeReader for ProjectIncludes { - fn read(&self, path: &str) -> Result { + fn read(&self, path: &str) -> Result { self.0 .read_include(path, None) + .map(included_file) .map_err(|error| include_error(&error, || {})) } } @@ -787,9 +789,12 @@ impl SearchIncludes { } impl IncludeReader for SearchIncludes { - fn read(&self, path: &str) -> Result { - let text = self.includes.read(path)?; - Ok(self.search_text(&text, Some(path)).into_owned()) + fn read(&self, path: &str) -> Result { + let file = self.includes.read(path)?; + Ok(IncludedFile { + text: self.search_text(&file.text, Some(path)).into_owned(), + ..file + }) } } diff --git a/crates/rw-site/src/path.rs b/crates/rw-site/src/path.rs index f88a3c8f..2e76a039 100644 --- a/crates/rw-site/src/path.rs +++ b/crates/rw-site/src/path.rs @@ -9,9 +9,10 @@ use parking_lot::Mutex; use rw_diagrams::diagram_language; use rw_parser::{Event, LinkKind, Parser, Tag, TagEnd}; use rw_renderer::{ - HtmlBackend, IncludeError, IncludeReader, MarkdownRenderer, ProjectFiles, image_file, + HtmlBackend, IncludeError, IncludeReader, IncludedFile, MarkdownRenderer, ProjectFiles, + image_file, }; -use rw_storage::{Storage, StorageError, download_content_type, image_content_type}; +use rw_storage::{IncludeText, Storage, StorageError, download_content_type, image_content_type}; /// Whether a page's image may name the project file at `path`: an image file, /// which is shown, or a diagram file, which is rendered. @@ -65,17 +66,28 @@ impl PageIncludes { &self, path: &str, record: Option<&dyn Fn(&Path)>, - ) -> Result { + ) -> Result { match &self.published { Some(published) => published .get(path) - .cloned() + .map(|text| IncludedFile { + text: text.clone(), + canonical_path: path.to_owned(), + }) .ok_or_else(|| StorageError::not_found(path)), - None => self.storage.read_include(path, record), + None => self.storage.read_include(path, record).map(included_file), } } } +/// What [`Storage::read_include`] read, as the renderer takes it. +pub(crate) fn included_file(include: IncludeText) -> IncludedFile { + IncludedFile { + text: include.text, + canonical_path: include.canonical_path, + } +} + /// The path and text of each markdown file that `markdown`, read from the /// project file `source_path`, includes when it renders. pub(crate) fn included_files( @@ -88,14 +100,18 @@ pub(crate) fn included_files( } let included = Arc::new(Mutex::new(Vec::new())); // Rendering places each include as the page's render does. - let _ = MarkdownRenderer::::new() + let rendered = MarkdownRenderer::::new() .with_source_path(source_path) .with_include_reader(IncludedFiles { page_includes, included: Arc::clone(&included), }) - .render(markdown); - mem::take(&mut *included.lock()) + .render(markdown) + .includes; + // The walk refuses some files it read, such as a symlink back to an open one. + let mut included = mem::take(&mut *included.lock()); + included.retain(|(path, _)| rendered.contains(path)); + included } struct IncludedFiles { @@ -104,13 +120,15 @@ struct IncludedFiles { } impl IncludeReader for IncludedFiles { - fn read(&self, path: &str) -> Result { - let text = self + fn read(&self, path: &str) -> Result { + let file = self .page_includes .read(path, None) .map_err(|_| IncludeError::NotFound)?; - self.included.lock().push((path.to_owned(), text.clone())); - Ok(text) + self.included + .lock() + .push((path.to_owned(), file.text.clone())); + Ok(file) } } diff --git a/crates/rw-site/tests/linked_files.rs b/crates/rw-site/tests/linked_files.rs index fbbebef3..9e7b3cb1 100644 --- a/crates/rw-site/tests/linked_files.rs +++ b/crates/rw-site/tests/linked_files.rs @@ -210,6 +210,26 @@ fn a_page_links_to_exactly_the_downloads_that_are_served() { } } +#[cfg(unix)] +#[test] +fn an_include_refused_as_a_cycle_links_nothing() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + write( + root, + &[ + ("docs/a.md", "[r](report.pdf)\n\n::include[sub/link.md]\n"), + ("docs/report.pdf", "pdf"), + ("docs/sub/report.pdf", "pdf"), + ], + ); + std::os::unix::fs::symlink("../a.md", root.join("docs/sub/link.md")).unwrap(); + let site = site(root); + + assert!(served(&site, "docs/report.pdf")); + assert!(!served(&site, "docs/sub/report.pdf")); +} + #[test] fn a_linked_file_that_cannot_be_served_warns_with_the_reason() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/rw-site/tests/markdown_includes.rs b/crates/rw-site/tests/markdown_includes.rs index 6e921030..8cb9a37e 100644 --- a/crates/rw-site/tests/markdown_includes.rs +++ b/crates/rw-site/tests/markdown_includes.rs @@ -94,6 +94,23 @@ fn a_failed_markdown_include_warns_and_renders_as_written() { ); } +#[cfg(unix)] +#[test] +fn a_page_that_includes_itself_through_a_symlink_renders_once() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path(); + write(root, &[("docs/a.md", "Once.\n\n::include[link.md]\n")]); + std::os::unix::fs::symlink("a.md", root.join("docs/link.md")).unwrap(); + + let page = site(root).render("a").unwrap(); + + assert_eq!( + page.warnings, + ["include `docs/link.md` is `docs/a.md`, which includes itself"] + ); + assert_eq!(page.html, "

Once.

::include[link.md]

"); +} + #[test] fn creating_editing_or_deleting_a_markdown_include_invalidates_the_cached_page() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/rw-storage-fs/src/lib.rs b/crates/rw-storage-fs/src/lib.rs index 7f77f4c3..13054e47 100644 --- a/crates/rw-storage-fs/src/lib.rs +++ b/crates/rw-storage-fs/src/lib.rs @@ -51,8 +51,8 @@ use debouncer::{DebouncedEvent, EventDebouncer, RawEventKind}; pub use dir_watch::watch_dirs; pub use event::{StorageEvent, StorageEventKind, StorageEventReceiver, WatchHandle}; use rw_storage::{ - Document, IncludeRefusal, MAX_FILE_SIZE, MissingFile, PageText, Storage, StorageError, - StorageErrorKind, file_content_type, image_content_type, is_markdown, + Document, IncludeRefusal, IncludeText, MAX_FILE_SIZE, MissingFile, PageText, Storage, + StorageError, StorageErrorKind, file_content_type, image_content_type, is_markdown, }; use scanner::Listing; use source::{ @@ -281,7 +281,7 @@ impl FsStorage { /// [`Storage::read_file`] may serve it. fn project_file(&self, path: &str) -> Result { file_content_type(path).ok_or(MissingFile::NotFound)?; - let file = self.confine(path, None).map_err(|error| match error { + let (file, _) = self.confine(path, None).map_err(|error| match error { Unconfined::Misspelled { on_disk } => MissingFile::Misspelled { on_disk }, Unconfined::Refused(_) => MissingFile::NotFound, })?; @@ -298,11 +298,16 @@ impl FsStorage { } } - /// The canonical path of the project file `path`, which must be plain and - /// relative, spelled as stored on disk and, since a symlink in the project - /// can point anywhere, must resolve inside the project. `record` gets the - /// file before it is resolved. - fn confine(&self, path: &str, record: Option<&dyn Fn(&Path)>) -> Result { + /// The canonical path of the project file `path`, and that path relative + /// to the project. `path` must be plain and relative, spelled as stored on + /// disk and, since a symlink in the project can point anywhere, must + /// resolve inside the project. `record` gets the file before it is + /// resolved. + fn confine( + &self, + path: &str, + record: Option<&dyn Fn(&Path)>, + ) -> Result<(PathBuf, String), Unconfined> { let relative = Path::new(path); if !relative .components() @@ -317,15 +322,14 @@ impl FsStorage { let file = joined .canonicalize() .map_err(|error| read_error(path, error))?; - let Some(project) = self - .resolver - .project() - .filter(|project| file.starts_with(project)) + let Some((project, in_project)) = (self.resolver.project()) + .and_then(|project| Some((project, file.strip_prefix(project).ok()?))) else { return Err(refused(path, IncludeRefusal::OutsideProject).into()); }; check_spelling(project, path, &file)?; - Ok(file) + let in_project = dir_url_path(in_project); + Ok((file, in_project)) } /// Fails once the source directory, confined to the project at config @@ -726,13 +730,13 @@ impl Storage for FsStorage { &self, path: &str, record: Option<&dyn Fn(&Path)>, - ) -> Result { + ) -> Result { let refuse = |refusal| refused(path, refusal); // Checked before `record`: no change on disk can make it markdown. if !is_markdown(path) { return Err(refuse(IncludeRefusal::NotMarkdown)); } - let file = self.confine(path, record).map_err(|error| match error { + let (file, canonical_path) = self.confine(path, record).map_err(|error| match error { Unconfined::Misspelled { on_disk } => refuse(IncludeRefusal::Misspelled { on_disk }), Unconfined::Refused(error) => error, })?; @@ -751,7 +755,11 @@ impl Storage for FsStorage { let bytes = read_bounded(&file) .map_err(|error| read_error(path, error))? .map_err(refuse)?; - String::from_utf8(bytes).map_err(|_| refuse(IncludeRefusal::NotUtf8)) + let text = String::from_utf8(bytes).map_err(|_| refuse(IncludeRefusal::NotUtf8))?; + Ok(IncludeText { + text, + canonical_path, + }) } fn mtime(&self, path: &str) -> Result { @@ -1925,7 +1933,11 @@ mod tests { ("docs/index.md", "# Home"), ("README.md", "# Readme"), ] { - assert_eq!(storage.read_include(path, None).unwrap(), text); + let include = storage.read_include(path, None).unwrap(); + assert_eq!( + (include.text.as_str(), include.canonical_path.as_str()), + (text, path) + ); } } @@ -2052,7 +2064,10 @@ mod tests { assert_eq!( storage.read_include("docs/alias.md", None).unwrap(), - "# Home" + IncludeText { + text: "# Home".to_owned(), + canonical_path: "docs/index.md".to_owned(), + } ); for (path, refusal) in [ ("docs/escape.md", IncludeRefusal::OutsideProject), @@ -2078,7 +2093,8 @@ mod tests { assert_eq!( storage .read_include("docs/_partials/note.md", None) - .unwrap(), + .unwrap() + .text, "Note." ); if !case_insensitive(temp.path()) { @@ -2109,7 +2125,10 @@ mod tests { assert_eq!( storage.read_include("docs/shared/note.md", None).unwrap(), - "Note." + IncludeText { + text: "Note.".to_owned(), + canonical_path: "partials/note.md".to_owned(), + } ); if case_insensitive(temp.path()) { let on_disk = "docs/shared/note.md".to_owned(); diff --git a/crates/rw-storage-s3/src/publisher.rs b/crates/rw-storage-s3/src/publisher.rs index 838b2ef7..4966da22 100644 --- a/crates/rw-storage-s3/src/publisher.rs +++ b/crates/rw-storage-s3/src/publisher.rs @@ -293,7 +293,7 @@ async fn build_bundles( let resolved_content = rewrite(&content, true, true, &mut bundling).into_owned(); let mut bundled_includes = BTreeMap::new(); for path in includes.get(&doc.path).into_iter().flatten() { - let text = storage.read_include(path, None)?; + let text = storage.read_include(path, None)?.text; // Its diagram files resolve from its own folder. bundling.source_path = Some(path.clone()); let file_warnings = bundling.warnings.len(); diff --git a/crates/rw-storage/src/lib.rs b/crates/rw-storage/src/lib.rs index 8e2780e5..5381e6a7 100644 --- a/crates/rw-storage/src/lib.rs +++ b/crates/rw-storage/src/lib.rs @@ -30,5 +30,6 @@ pub use file::{ #[cfg(feature = "mock")] pub use mock::MockStorage; pub use storage::{ - Document, IncludeRefusal, PageText, Storage, StorageError, StorageErrorKind, mtime_to_datetime, + Document, IncludeRefusal, IncludeText, PageText, Storage, StorageError, StorageErrorKind, + mtime_to_datetime, }; diff --git a/crates/rw-storage/src/mock.rs b/crates/rw-storage/src/mock.rs index be705d31..d2a4441a 100644 --- a/crates/rw-storage/src/mock.rs +++ b/crates/rw-storage/src/mock.rs @@ -11,7 +11,9 @@ use parking_lot::RwLock; use rw_meta::Meta; use crate::file::{MissingFile, file_content_type, is_markdown}; -use crate::storage::{Document, IncludeRefusal, PageText, Storage, StorageError, StorageErrorKind}; +use crate::storage::{ + Document, IncludeRefusal, IncludeText, PageText, Storage, StorageError, StorageErrorKind, +}; fn titled(title: impl Into) -> Arc { Arc::new(Meta { @@ -377,7 +379,7 @@ impl Storage for MockStorage { &self, path: &str, _record: Option<&dyn Fn(&Path)>, - ) -> Result { + ) -> Result { let refused = |refusal| { StorageError::from(refusal) .with_path(path) @@ -387,9 +389,13 @@ impl Storage for MockStorage { return Err(refused(IncludeRefusal::NotMarkdown)); } match self.files.get(path) { - Some(Ok(bytes)) => { - String::from_utf8(bytes.clone()).map_err(|_| refused(IncludeRefusal::NotUtf8)) - } + Some(Ok(bytes)) => match String::from_utf8(bytes.clone()) { + Ok(text) => Ok(IncludeText { + text, + canonical_path: path.to_owned(), + }), + Err(_) => Err(refused(IncludeRefusal::NotUtf8)), + }, Some(&Err(kind)) => Err(StorageError::new(kind).with_path(path).with_backend("Mock")), None => Err(StorageError::not_found(path).with_backend("Mock")), } @@ -474,7 +480,10 @@ mod tests { storage .read_include("docs/_partials/Note.MD", None) .unwrap(), - "Note." + IncludeText { + text: "Note.".to_owned(), + canonical_path: "docs/_partials/Note.MD".to_owned(), + } ); for (path, expected) in [ ("docs/missing.md", (NotFound, None)), diff --git a/crates/rw-storage/src/storage.rs b/crates/rw-storage/src/storage.rs index 8b37c98f..e8d3d72a 100644 --- a/crates/rw-storage/src/storage.rs +++ b/crates/rw-storage/src/storage.rs @@ -127,6 +127,16 @@ impl From for PageText { } } +/// A markdown file a page includes, as [`Storage::read_include`] returns it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct IncludeText { + /// The file's markdown. + pub text: String, + /// The file's project path with symlinks resolved, so that every path to + /// one file gives the same; `path` itself for a backend without symlinks. + pub canonical_path: String, +} + /// Semantic error categories. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[non_exhaustive] @@ -409,7 +419,7 @@ pub trait Storage: Send + Sync { &self, path: &str, _record: Option<&dyn Fn(&Path)>, - ) -> Result { + ) -> Result { Err(StorageError::not_found(path)) } diff --git a/docs/includes.md b/docs/includes.md index 834d882d..fe9f8923 100644 --- a/docs/includes.md +++ b/docs/includes.md @@ -74,7 +74,8 @@ with these differences: - It is a markdown document of its own: its link reference definitions don't reach the page, and a `:::` container it opens ends with it. - Its own `::include` lines are followed too. A file that includes itself, - directly or through other files, warns and that line shows as written. + directly, through other files or through a symlink to it, warns and that + line shows as written. Comments left on included text belong to the including page. A partial included on three pages has three separate sets of comments. @@ -90,6 +91,7 @@ include `notes.txt` is not a markdown file include `docs/_partials/missing.md` not found include `docs/_Partials/note.md` not found; the file on disk is `docs/_partials/note.md` include `docs/_partials/note.md` is past the page's limit of 64 includes +include `docs/link.md` is `docs/guide.md`, which includes itself docs/b.md: include `docs/a.md` includes itself through `docs/b.md` ```