diff --git a/src/book/book.rs b/src/book/book.rs index c531a891..53a964ca 100644 --- a/src/book/book.rs +++ b/src/book/book.rs @@ -61,11 +61,11 @@ fn create_missing(src_dir: &Path, summary: &Summary) -> Result<()> { /// A dumb tree structure representing a book. /// -/// For the moment a book is just a collection of `BookItems` which are +/// For the moment a book is just a collection of `BookItems` which are /// accessible by either iterating (immutably) over the book with [`iter()`], or /// recursively applying a closure to each section to mutate the chapters, using /// [`for_each_mut()`]. -/// +/// /// [`iter()`]: #method.iter /// [`for_each_mut()`]: #method.for_each_mut #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] @@ -199,7 +199,8 @@ fn load_chapter>(link: &Link, src_dir: P) -> Result { .chain_err(|| format!("Chapter file not found, {}", link.location.display()))?; let mut content = String::new(); - f.read_to_string(&mut content)?; + f.read_to_string(&mut content) + .chain_err(|| format!("Unable to read \"{}\" ({})", link.name, location.display()))?; let stripped = location .strip_prefix(&src_dir) @@ -476,4 +477,43 @@ And here is some \ assert_eq!(visited, num_items); } + + #[test] + fn cant_load_chapters_with_an_empty_path() { + let (_, temp) = dummy_link(); + let summary = Summary { + numbered_chapters: vec![ + SummaryItem::Link(Link { + name: String::from("Empty"), + location: PathBuf::from(""), + ..Default::default() + }), + ], + ..Default::default() + }; + + let got = load_book_from_disk(&summary, temp.path()); + assert!(got.is_err()); + } + + #[test] + fn cant_load_chapters_when_the_link_is_a_directory() { + let (_, temp) = dummy_link(); + let dir = temp.path().join("nested"); + fs::create_dir(&dir).unwrap(); + + let summary = Summary { + numbered_chapters: vec![ + SummaryItem::Link(Link { + name: String::from("nested"), + location: dir, + ..Default::default() + }), + ], + ..Default::default() + }; + + let got = load_book_from_disk(&summary, temp.path()); + assert!(got.is_err()); + } } diff --git a/src/book/summary.rs b/src/book/summary.rs index 16b67a7c..7db1df1e 100644 --- a/src/book/summary.rs +++ b/src/book/summary.rs @@ -6,7 +6,6 @@ use memchr::{self, Memchr}; use pulldown_cmark::{self, Alignment, Event, Tag}; use errors::*; - /// Parse the text from a `SUMMARY.md` file into a sort of "recipe" to be /// used when loading a book from disk. /// @@ -204,7 +203,7 @@ impl<'a> SummaryParser<'a> { } } - /// Get the current line and column to give the user more useful error + /// Get the current line and column to give the user more useful error /// messages. fn current_location(&self) -> (usize, usize) { let byte_offset = self.stream.get_offset(); @@ -273,12 +272,16 @@ impl<'a> SummaryParser<'a> { let link_content = collect_events!(self.stream, end Tag::Link(..)); let name = stringify_events(link_content); - Ok(Link { - name: name, - location: PathBuf::from(href.to_string()), - number: None, - nested_items: Vec::new(), - }) + if href.is_empty() { + Err(self.parse_error("You can't have an empty link.")) + } else { + Ok(Link { + name: name, + location: PathBuf::from(href.to_string()), + number: None, + nested_items: Vec::new(), + }) + } } /// Parse the numbered chapters. This assumes the opening list tag has @@ -325,7 +328,7 @@ impl<'a> SummaryParser<'a> { // break; // } if let Event::End(tag) = event { - if tag_eq(&tag, &other_tag) { + if tag_eq(&tag, &other_tag) { break; } } @@ -484,23 +487,25 @@ fn stringify_events(events: Vec) -> String { // FIXME: Remove this when google/pulldown_cmark#120 lands (new patch release) fn tag_eq(left: &Tag, right: &Tag) -> bool { match (left, right) { - (&Tag::Paragraph, &Tag::Paragraph) => true, - (&Tag::Rule, &Tag::Rule) => true, - (&Tag::Header(a), &Tag::Header(b)) => a == b, - (&Tag::BlockQuote, &Tag::BlockQuote) => true, - (&Tag::CodeBlock(ref a), &Tag::CodeBlock(ref b)) => a == b, - (&Tag::List(ref a), &Tag::List(ref b)) => a == b, - (&Tag::Item, &Tag::Item) => true, - (&Tag::FootnoteDefinition(ref a), &Tag::FootnoteDefinition(ref b)) => a == b, - (&Tag::Table(ref a), &Tag::Table(ref b)) => a.iter().zip(b.iter()).all(|(l, r)| alignment_eq(*l, *r)), - (&Tag::TableHead, &Tag::TableHead) => true, - (&Tag::TableRow, &Tag::TableRow) => true, - (&Tag::TableCell, &Tag::TableCell) => true, - (&Tag::Emphasis, &Tag::Emphasis) => true, - (&Tag::Strong, &Tag::Strong) => true, - (&Tag::Code, &Tag::Code) => true, - (&Tag::Link(ref a_1, ref a_2), &Tag::Link(ref b_1, ref b_2)) => a_1 == b_1 && a_2 == b_2, - (&Tag::Image(ref a_1, ref a_2), &Tag::Image(ref b_1, ref b_2)) => a_1 == b_1 && a_2 == b_2, + (&Tag::Paragraph, &Tag::Paragraph) => true, + (&Tag::Rule, &Tag::Rule) => true, + (&Tag::Header(a), &Tag::Header(b)) => a == b, + (&Tag::BlockQuote, &Tag::BlockQuote) => true, + (&Tag::CodeBlock(ref a), &Tag::CodeBlock(ref b)) => a == b, + (&Tag::List(ref a), &Tag::List(ref b)) => a == b, + (&Tag::Item, &Tag::Item) => true, + (&Tag::FootnoteDefinition(ref a), &Tag::FootnoteDefinition(ref b)) => a == b, + (&Tag::Table(ref a), &Tag::Table(ref b)) => { + a.iter().zip(b.iter()).all(|(l, r)| alignment_eq(*l, *r)) + } + (&Tag::TableHead, &Tag::TableHead) => true, + (&Tag::TableRow, &Tag::TableRow) => true, + (&Tag::TableCell, &Tag::TableCell) => true, + (&Tag::Emphasis, &Tag::Emphasis) => true, + (&Tag::Strong, &Tag::Strong) => true, + (&Tag::Code, &Tag::Code) => true, + (&Tag::Link(ref a_1, ref a_2), &Tag::Link(ref b_1, ref b_2)) => a_1 == b_1 && a_2 == b_2, + (&Tag::Image(ref a_1, ref a_2), &Tag::Image(ref b_1, ref b_2)) => a_1 == b_1 && a_2 == b_2, _ => false, } } @@ -512,7 +517,7 @@ fn alignment_eq(left: Alignment, right: Alignment) -> bool { (Alignment::Left, Alignment::Left) => true, (Alignment::Center, Alignment::Center) => true, (Alignment::Right, Alignment::Right) => true, - _ => false + _ => false, } } @@ -754,4 +759,14 @@ mod tests { assert_eq!(got, should_be); } -} \ No newline at end of file + + #[test] + fn an_empty_link_location_is_an_error() { + let src = "- [Empty]()\n"; + let mut parser = SummaryParser::new(src); + parser.stream.next(); + + let got = parser.parse_numbered(); + assert!(got.is_err()); + } +}