rust-lang / mdBook

Create book from markdown files. Like Gitbook but implemented in Rust
https://rust-lang.github.io/mdBook/
Mozilla Public License 2.0
18.49k stars 1.66k forks source link

Crash when SUMMARY contains URLs #1766

Open kleinesfilmroellchen opened 2 years ago

kleinesfilmroellchen commented 2 years ago

I converted some old Markdown documentation to use mdbook. My SUMMARY-like file (that I renamed) was previously pointing at online links (not that that's a good idea, but bear with me), so it looked something like this:

- [Section 1](https://www.example.com/markdown/file/path)
- [Section 2](https://www.example.com/markdown/file/path2)

When putting this file into the correct directory the following happens when init-ing on these existing files:

$ mdbook init
Do you want a .gitignore to be created? (y/n)
y
What title would you like to give the book?
<doesn't matter>
2022-03-11 19:28:34 [INFO] (mdbook::book::init): Creating a new book with stub content
2022-03-11 19:28:34 [ERROR] (mdbook::book::init): Unable to create missing chapters
thread 'main' panicked at 'The BookBuilder should always create a valid book. If you are seeing this it is a bug and should be reported.', path\to\.cargo\registry\src\github.com-1ecc6299db9ec823\mdbook-0.4.15\src\book\init.rs:89:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

A follow-on bug is that there is still a book.toml created. However, mdbook build is not any better off:

2022-03-11 19:26:42 [ERROR] (mdbook::utils): Error: Unable to create missing chapters
2022-03-11 19:26:42 [ERROR] (mdbook::utils):    Caused By: Die Syntax für den Dateinamen, Verzeichnisnamen oder die Datenträgerbezeichnung ist falsch. (os error 123)

(OS error says: Syntax for file name, directory name or storage medium identifier is incorrect)

Both of these lead me to believe that it's trying to interpret the URLs as file system paths without verifying them. Replacing the URLs by the respective local files makes things work. While URLs should of course not be supported, it would be good to have a descriptive user-facing error like "Chapter XYZ links to a URL. Please point it to a valid file instead." Also, no half-baked book.toml should be created.

kg4zow commented 2 years ago

I know it's not exactly what you're looking for, but here's a suggestion. If you only need a single link "out" of the book, you can add something like this to your books' theme/index.hbs files ...

        <nav id="sidebar" class="sidebar" aria-label="Table of contents">
            <div class="sidebar-scrollbox">

<!-- Start extra link above the ToC -->
                <a class='part-title' href='https://github.com/'>All The Things!</a>
                <hr/>
<!-- End extra link above the ToC -->

                {{#toc}}{{/toc}}

I'm using this idea at work, where in my spare time (hah!) I'm writing a collection of "books" about different topics under a common field. (Can't be more specific, sorry.) My plan is for each of the books to have a link "up" to a static index page which contains a list of links to each of the books. Each book's content is tracked in its own git repo, and will be hosted on GitHub Pages, with each book having a random hostname because the repos are in a private organization. The index page will be the only thing with an easy-to-remember hostname, because it'll be on an internal web server.

kleinesfilmroellchen commented 2 years ago

@kg4zow I don't actually need external links in the TOC, I wanted to replace them anyways after setting up the mdbook. This issue is just about better error behavior, if you would like this to be a feature then it's a separate question I believe.

ISSOtm commented 2 years ago

I guess the question is whether external URLs should be allowed in the summary file. (I'd say no?)

kleinesfilmroellchen commented 2 years ago

@ISSOtm Agreed, no allowing external URLs. Again, this issue is about improving the error, not making it go away.

ISSOtm commented 2 years ago

I suppose URLs that contain a scheme should be rejected? Other than that, I'm not sure exactly what to filter on, actually.

kleinesfilmroellchen commented 2 years ago

@ISSOtm sounds like a good solution; no need to accept explicit file:// URLs.

ISSOtm commented 2 years ago

Given #1640, should this be an error, or a warning? The latter makes more sense if we're going for path sanitization.

kleinesfilmroellchen commented 2 years ago

Both sounds fine to me; you're more familiar with the error/warning split in mdbook than me so please choose whatever you think is better.

ISSOtm commented 2 years ago

I'm not really, but I'd expect errors to fail the build while warnings not.