diff --git a/crates/mdbook-core/src/config.rs b/crates/mdbook-core/src/config.rs index b2c6862c30..8e79d53e51 100644 --- a/crates/mdbook-core/src/config.rs +++ b/crates/mdbook-core/src/config.rs @@ -44,7 +44,7 @@ //! ``` use crate::static_regex; -use crate::utils::{TomlExt, fs, log_backtrace}; +use crate::utils::{TomlExt, fs}; use anyhow::{Context, Error, Result, bail}; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, HashMap}; @@ -247,15 +247,8 @@ impl Config { /// This is for compatibility only. It will be removed completely once the /// HTML renderer is refactored to be less coupled to `mdbook` internals. #[doc(hidden)] - pub fn html_config(&self) -> Option { - match self.get("output.html") { - Ok(Some(config)) => Some(config), - Ok(None) => None, - Err(e) => { - log_backtrace(&e); - None - } - } + pub fn html_config(&self) -> Result> { + self.get("output.html") } /// Set a config key, clobbering any existing values along the way. @@ -816,7 +809,7 @@ mod tests { assert_eq!(got.book, book_should_be); assert_eq!(got.build, build_should_be); assert_eq!(got.rust, rust_should_be); - assert_eq!(got.html_config().unwrap(), html_should_be); + assert_eq!(got.html_config().unwrap().unwrap(), html_should_be); } #[test] @@ -832,7 +825,7 @@ mod tests { "#; let got = Config::from_str(src).unwrap(); - assert!(!got.html_config().unwrap().playground.runnable); + assert!(!got.html_config().unwrap().unwrap().playground.runnable); } #[test] @@ -995,7 +988,7 @@ mod tests { "#; let got = Config::from_str(src).unwrap(); - let html_config = got.html_config().unwrap(); + let html_config = got.html_config().unwrap().unwrap(); assert_eq!(html_config.input_404, None); assert_eq!(html_config.get_404_output_file(), "404.html"); } @@ -1008,7 +1001,7 @@ mod tests { "#; let got = Config::from_str(src).unwrap(); - let html_config = got.html_config().unwrap(); + let html_config = got.html_config().unwrap().unwrap(); assert_eq!(html_config.input_404, Some("missing.md".to_string())); assert_eq!(html_config.get_404_output_file(), "missing.html"); } @@ -1153,7 +1146,7 @@ mod tests { enable = false "#; let got = Config::from_str(src).unwrap(); - let html_config = got.html_config().unwrap(); + let html_config = got.html_config().unwrap().unwrap(); assert!(!html_config.print.enable); assert!(html_config.print.page_break); let src = r#" @@ -1161,7 +1154,7 @@ mod tests { page-break = false "#; let got = Config::from_str(src).unwrap(); - let html_config = got.html_config().unwrap(); + let html_config = got.html_config().unwrap().unwrap(); assert!(html_config.print.enable); assert!(!html_config.print.page_break); } @@ -1173,6 +1166,29 @@ mod tests { assert_eq!(json!(TextDirection::LeftToRight), json!("ltr")); } + #[test] + fn html_config_rejects_unknown_fields() { + let src = r#" + [book] + title = "Some Book" + + [output.html] + foo = 123 + "#; + + let got = Config::from_str(src).unwrap(); + let err = got.html_config().unwrap_err(); + let err_msg = format!("{err:#}"); + assert!( + err_msg.contains("Failed to deserialize `output.html`"), + "unexpected error: {err_msg}" + ); + assert!( + err_msg.contains("unknown field `foo`"), + "unexpected error: {err_msg}" + ); + } + #[test] fn get_deserialize_error() { let src = r#" diff --git a/crates/mdbook-driver/src/init.rs b/crates/mdbook-driver/src/init.rs index badd24c58b..bdfb6b9f53 100644 --- a/crates/mdbook-driver/src/init.rs +++ b/crates/mdbook-driver/src/init.rs @@ -108,7 +108,7 @@ impl BookBuilder { fn copy_across_theme(&self) -> Result<()> { debug!("Copying theme"); - let html_config = self.config.html_config().unwrap_or_default(); + let html_config = self.config.html_config()?.unwrap_or_default(); Theme::copy_theme(&html_config, &self.root)?; Ok(()) } diff --git a/crates/mdbook-driver/src/mdbook.rs b/crates/mdbook-driver/src/mdbook.rs index 45bae41816..c1e9bcecc5 100644 --- a/crates/mdbook-driver/src/mdbook.rs +++ b/crates/mdbook-driver/src/mdbook.rs @@ -384,11 +384,12 @@ impl MDBook { } /// Get the directory containing the theme resources for the book. - pub fn theme_dir(&self) -> PathBuf { - self.config - .html_config() + pub fn theme_dir(&self) -> Result { + Ok(self + .config + .html_config()? .unwrap_or_default() - .theme_dir(&self.root) + .theme_dir(&self.root)) } } diff --git a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs index 8edac3cace..9c3076c23a 100644 --- a/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs +++ b/crates/mdbook-html/src/html_handlebars/hbs_renderer.rs @@ -307,7 +307,7 @@ impl Renderer for HtmlHandlebars { fn render(&self, ctx: &RenderContext) -> Result<()> { let book_config = &ctx.config.book; - let html_config = ctx.config.html_config().unwrap_or_default(); + let html_config = ctx.config.html_config()?.unwrap_or_default(); let src_dir = ctx.root.join(&ctx.config.book.src); let destination = &ctx.destination; let book = &ctx.book; diff --git a/crates/mdbook-html/src/html_handlebars/search.rs b/crates/mdbook-html/src/html_handlebars/search.rs index 09920d76d2..75d693f046 100644 --- a/crates/mdbook-html/src/html_handlebars/search.rs +++ b/crates/mdbook-html/src/html_handlebars/search.rs @@ -337,7 +337,7 @@ fn chapter_settings_priority() { "foo" = {} # Just to make sure empty table is allowed. "#; let cfg: mdbook_core::config::Config = toml::from_str(cfg).unwrap(); - let html = cfg.html_config().unwrap(); + let html = cfg.html_config().unwrap().unwrap(); let chapter_configs = sort_search_config(&html.search.unwrap().chapter); for (path, enable) in [ ("foo.md", None), diff --git a/src/cmd/serve.rs b/src/cmd/serve.rs index 255c077d98..ec781b395d 100644 --- a/src/cmd/serve.rs +++ b/src/cmd/serve.rs @@ -74,7 +74,7 @@ pub fn execute(args: &ArgMatches) -> Result<()> { .next() .ok_or_else(|| anyhow::anyhow!("no address found for {}", address))?; let build_dir = book.build_dir_for("html"); - let html_config = book.config.html_config().unwrap_or_default(); + let html_config = book.config.html_config()?.unwrap_or_default(); let file_404 = html_config.get_404_output_file(); // A channel used to broadcast to any websockets to reload when a file changes. diff --git a/src/cmd/watch/native.rs b/src/cmd/watch/native.rs index b4cbc9a774..5c87193637 100644 --- a/src/cmd/watch/native.rs +++ b/src/cmd/watch/native.rs @@ -38,7 +38,11 @@ pub fn rebuild_on_change( std::process::exit(1); }; - let _ = watcher.watch(&book.theme_dir(), Recursive); + let theme_dir = book.theme_dir().unwrap_or_else(|e| { + error!("invalid book configuration: {e}"); + std::process::exit(1); + }); + let _ = watcher.watch(&theme_dir, Recursive); // Add the book.toml file to the watcher if it exists let _ = watcher.watch(&book.root.join("book.toml"), NonRecursive); diff --git a/src/cmd/watch/poller.rs b/src/cmd/watch/poller.rs index b834b67928..a3c48c71aa 100644 --- a/src/cmd/watch/poller.rs +++ b/src/cmd/watch/poller.rs @@ -4,6 +4,7 @@ //! lots of problems. Various operating systems and different filesystems have //! had problems correctly reporting changes. +use anyhow::Result; use ignore::gitignore::Gitignore; use mdbook_driver::MDBook; use pathdiff::diff_paths; @@ -29,7 +30,10 @@ pub fn rebuild_on_change( info!("Watching for changes..."); // Scan once to initialize the starting point. - watcher.set_roots(&book); + if let Err(e) = watcher.set_roots(&book) { + error!("invalid book configuration: {e}"); + std::process::exit(1); + } watcher.scan(); // Track average scan time, to help investigate if the poller is taking @@ -66,7 +70,9 @@ pub fn rebuild_on_change( post_build(); } book = b; - watcher.set_roots(&book); + if let Err(e) = watcher.set_roots(&book) { + error!("invalid book configuration: {e}"); + } } Err(e) => error!("failed to load book config: {e:?}"), } @@ -121,10 +127,10 @@ impl Watcher { } /// Sets the root directories where scanning will start. - fn set_roots(&mut self, book: &MDBook) { + fn set_roots(&mut self, book: &MDBook) -> Result<()> { let mut root_paths = vec![ book.source_dir(), - book.theme_dir(), + book.theme_dir()?, book.root.join("book.toml"), ]; root_paths.extend( @@ -134,7 +140,7 @@ impl Watcher { .iter() .map(|path| book.root.join(path)), ); - if let Some(html_config) = book.config.html_config() { + if let Some(html_config) = book.config.html_config()? { root_paths.extend( html_config .additional_css @@ -145,6 +151,7 @@ impl Watcher { } self.root_paths = root_paths; + Ok(()) } /// Scans for changes. @@ -276,7 +283,7 @@ mod tests { // Create a watcher and check its behavior. let book = MDBook::load(&book_root).unwrap(); let mut watcher = Watcher::new(&book_root); - watcher.set_roots(&book); + watcher.set_roots(&book).unwrap(); // Do an initial scan to initialize its state. watcher.scan(); // Verify the steady state is empty. diff --git a/tests/testsuite/config.rs b/tests/testsuite/config.rs index 641243c103..7608c14563 100644 --- a/tests/testsuite/config.rs +++ b/tests/testsuite/config.rs @@ -179,6 +179,33 @@ ERROR Invalid configuration file unknown field `foo`, expected one of `title`, `authors`, `description`, `src`, `language`, `text-direction` +"#]]); + }); +} + +// An invalid key in the HTML output table should fail the build. +#[test] +fn bad_config_in_html_table() { + BookTest::init(|_| {}) + .change_file( + "book.toml", + "[book]\n\ + title = \"bad-html-config\"\n\ + \n\ + [output.html]\n\ + foo = 123", + ) + .run("build", |cmd| { + cmd.expect_failure() + .expect_stdout(str![[""]]) + .expect_stderr(str![[r#" + INFO Book building has started + INFO Running the html backend +ERROR Rendering failed +[TAB]Caused by: Failed to deserialize `output.html` +[TAB]Caused by: unknown field `foo`, expected one of `theme`, `default-theme`, `preferred-dark-theme`, `smart-punctuation`, `definition-lists`, `admonitions`, `mathjax-support`, `additional-css`, `additional-js`, `fold`, `playground`, `playpen`, `code`, `print`, `no-section-label`, `search`, `git-repository-url`, `git-repository-icon`, `input-404`, `site-url`, `cname`, `edit-url-template`, `live-reload-endpoint`, `redirect`, `hash-files`, `sidebar-header-nav` + + "#]]); }); }