Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when SUMMARY contains URLs #1766

Open
kleinesfilmroellchen opened this issue Mar 11, 2022 · 9 comments
Open

Crash when SUMMARY contains URLs #1766

kleinesfilmroellchen opened this issue Mar 11, 2022 · 9 comments
Labels
A-Summary Area: The summary page, organization of pages. C-panic Category: A bug that causes a panic or crash

Comments

@kleinesfilmroellchen
Copy link

kleinesfilmroellchen commented Mar 11, 2022

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.lhy31512.workers.dev-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.

@ehuss ehuss added C-panic Category: A bug that causes a panic or crash A-Summary Area: The summary page, organization of pages. labels Jun 1, 2022
@kg4zow
Copy link

kg4zow commented Jun 8, 2022

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
Copy link
Author

@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
Copy link
Contributor

ISSOtm commented Jun 18, 2022

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

@kleinesfilmroellchen
Copy link
Author

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

@ISSOtm
Copy link
Contributor

ISSOtm commented Jun 18, 2022

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

@kleinesfilmroellchen
Copy link
Author

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

@ISSOtm
Copy link
Contributor

ISSOtm commented Jun 18, 2022

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

@kleinesfilmroellchen
Copy link
Author

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
Copy link
Contributor

ISSOtm commented Jun 18, 2022

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

arnetheduck added a commit to status-im/nimbus-eth2 that referenced this issue Oct 29, 2024
* use same versions as style guide
* remove obsoletion notice from summary - this is not supported by
mdbook: rust-lang/mdBook#1766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Summary Area: The summary page, organization of pages. C-panic Category: A bug that causes a panic or crash
Projects
None yet
Development

No branches or pull requests

4 participants