Deny all unknown config fields

This changes it so that it is an error if there is ever an unknown
configuration field. This is intended to help avoid things like typos,
or using an outdated version of mdbook. Although it is possible that new
fields could potentially safely be ignored, setting up a warning system
is a bit more of a hassle. I don't think mdbook needs to have the same
kind of multi-version support as something like cargo does. However, if
this ends up being too much of a pain point, we can try to add a warning
system instead.

There are a variety of changes here:

- The top-level config namespace is now closed so that it only accepts
  the keys defined in `Config`.
- All config tables now reject unknown fields.
- Added `Config::outputs` and `Config::preprocessors` for convenience
  to access the entire `output` and `preprocessor` tables.
- Moved the unit-tests that were setting environment variables to the
  testsuite where it launches a process instead.

Closes #1595

Author: ehuss

crates/mdbook-core/src/config.rs

@@ -14,7 +14,6 @@ use mdbook_preprocessor::{Preprocessor, PreprocessorContext};
 use mdbook_renderer::{RenderContext, Renderer};
 use mdbook_summary::Summary;
 use serde::Deserialize;
-use std::collections::HashMap;
 use std::ffi::OsString;
 use std::io::{IsTerminal, Write};
 use std::path::{Path, PathBuf};
@@ -423,22 +422,17 @@ struct OutputConfig {
 fn determine_renderers(config: &Config) -> Result<Vec<Box<dyn Renderer>>> {
     let mut renderers = Vec::new();
 
-    match config.get::<HashMap<String, OutputConfig>>("output") {
-        Ok(Some(output_table)) => {
-            renderers.extend(output_table.into_iter().map(|(key, table)| {
-                if key == "html" {
-                    Box::new(HtmlHandlebars::new()) as Box<dyn Renderer>
-                } else if key == "markdown" {
-                    Box::new(MarkdownRenderer::new()) as Box<dyn Renderer>
-                } else {
-                    let command = table.command.unwrap_or_else(|| format!("mdbook-{key}"));
-                    Box::new(CmdRenderer::new(key, command))
-                }
-            }));
+    let outputs = config.outputs::<OutputConfig>()?;
+    renderers.extend(outputs.into_iter().map(|(key, table)| {
+        if key == "html" {
+            Box::new(HtmlHandlebars::new()) as Box<dyn Renderer>
+        } else if key == "markdown" {
+            Box::new(MarkdownRenderer::new()) as Box<dyn Renderer>
+        } else {
+            let command = table.command.unwrap_or_else(|| format!("mdbook-{key}"));
+            Box::new(CmdRenderer::new(key, command))
         }
-        Ok(None) => {}
-        Err(e) => bail!("failed to get output table config: {e}"),
-    }
+    }));
 
     // if we couldn't find anything, add the HTML renderer as a default
     if renderers.is_empty() {
@@ -477,12 +471,7 @@ fn determine_preprocessors(config: &Config) -> Result<Vec<Box<dyn Preprocessor>>
         }
     }
 
-    let preprocessor_table = match config.get::<HashMap<String, PreprocessorConfig>>("preprocessor")
-    {
-        Ok(Some(preprocessor_table)) => preprocessor_table,
-        Ok(None) => HashMap::new(),
-        Err(e) => bail!("failed to get preprocessor table config: {e}"),
-    };
+    let preprocessor_table = config.preprocessors::<PreprocessorConfig>()?;
 
     for (name, table) in preprocessor_table.iter() {
         preprocessor_names.insert(name.to_string());

crates/mdbook-driver/src/mdbook.rs

@@ -7,7 +7,7 @@ fn config_defaults_to_html_renderer_if_empty() {
     let cfg = Config::default();
 
     // make sure we haven't got anything in the `output` table
-    assert!(cfg.get::<Value>("output").unwrap().is_none());
+    assert!(cfg.outputs::<toml::Value>().unwrap().is_empty());
 
     let got = determine_renderers(&cfg).unwrap();
 
@@ -45,7 +45,7 @@ fn config_defaults_to_link_and_index_preprocessor_if_not_set() {
     let cfg = Config::default();
 
     // make sure we haven't got anything in the `preprocessor` table
-    assert!(cfg.get::<Value>("preprocessor").unwrap().is_none());
+    assert!(cfg.preprocessors::<toml::Value>().unwrap().is_empty());
 
     let got = determine_preprocessors(&cfg);
 

crates/mdbook-driver/src/mdbook/tests.rs

@@ -2,7 +2,7 @@ use super::command_prelude::*;
 #[cfg(feature = "watch")]
 use super::watch;
 use crate::{get_book_dir, open};
-use anyhow::{Result, bail};
+use anyhow::Result;
 use axum::Router;
 use axum::extract::ws::{Message, WebSocket, WebSocketUpgrade};
 use axum::routing::get;
@@ -76,10 +76,8 @@ 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 input_404 = match book.config.get::<String>("output.html.input-404") {
-        Ok(v) => v,
-        Err(e) => bail!("expected string for output.html.input-404: {e}"),
-    };
+    let html_config = book.config.html_config();
+    let input_404 = html_config.and_then(|c| c.input_404);
     let file_404 = get_404_output_file(&input_404);
 
     // A channel used to broadcast to any websockets to reload when a file changes.

src/cmd/serve.rs

@@ -0,0 +1,159 @@
+//! Tests for book configuration loading.
+
+use crate::prelude::*;
+
+// Test that config can load from environment variable.
+#[test]
+fn config_from_env() {
+    BookTest::from_dir("config/empty")
+        .run("build", |cmd| {
+            cmd.env("MDBOOK_BOOK__TITLE", "Custom env title");
+        })
+        .check_file_contains(
+            "book/index.html",
+            "<title>Chapter 1 - Custom env title</title>",
+        );
+
+    // json for some subtable
+    //
+}
+
+// Test environment config with JSON.
+#[test]
+fn config_json_from_env() {
+    // build table
+    BookTest::from_dir("config/empty")
+        .run("build", |cmd| {
+            cmd.env(
+                "MDBOOK_BOOK",
+                r#"{"title": "My Awesome Book", "authors": ["Michael-F-Bryan"]}"#,
+            );
+        })
+        .check_file_contains(
+            "book/index.html",
+            "<title>Chapter 1 - My Awesome Book</title>",
+        );
+
+    // book table
+    BookTest::from_dir("config/empty")
+        .run("build", |cmd| {
+            cmd.env("MDBOOK_BUILD", r#"{"build-dir": "alt"}"#);
+        })
+        .check_file_contains("alt/index.html", "<title>Chapter 1</title>");
+}
+
+// Test that a preprocessor receives config set in the environment.
+#[test]
+fn preprocessor_cfg_from_env() {
+    let mut test = BookTest::from_dir("config/empty");
+    test.rust_program(
+        "cat-to-file",
+        r#"
+        fn main() {
+            use std::io::Read;
+            let mut s = String::new();
+            std::io::stdin().read_to_string(&mut s).unwrap();
+            std::fs::write("out.txt", s).unwrap();
+            println!("{{\"sections\": []}}");
+        }
+        "#,
+    )
+    .run("build", |cmd| {
+        cmd.env(
+            "MDBOOK_PREPROCESSOR__CAT_TO_FILE",
+            r#"{"command":"./cat-to-file", "array": [1,2,3], "number": 123}"#,
+        );
+    });
+    let out = read_to_string(test.dir.join("out.txt"));
+    let (ctx, _book) = mdbook_preprocessor::parse_input(out.as_bytes()).unwrap();
+    let cfg: serde_json::Value = ctx.config.get("preprocessor.cat-to-file").unwrap().unwrap();
+    assert_eq!(
+        cfg,
+        serde_json::json!({
+            "command": "./cat-to-file",
+            "array": [1,2,3],
+            "number": 123,
+        })
+    );
+}
+
+// Test that a renderer receives config set in the environment.
+#[test]
+fn output_cfg_from_env() {
+    let mut test = BookTest::from_dir("config/empty");
+    test.rust_program(
+        "cat-to-file",
+        r#"
+        fn main() {
+            use std::io::Read;
+            let mut s = String::new();
+            std::io::stdin().read_to_string(&mut s).unwrap();
+            std::fs::write("out.txt", s).unwrap();
+        }
+        "#,
+    )
+    .run("build", |cmd| {
+        cmd.env(
+            "MDBOOK_OUTPUT__CAT_TO_FILE",
+            r#"{"command":"./cat-to-file", "array": [1,2,3], "number": 123}"#,
+        );
+    });
+    let out = read_to_string(test.dir.join("book/out.txt"));
+    let ctx = mdbook_renderer::RenderContext::from_json(out.as_bytes()).unwrap();
+    let cfg: serde_json::Value = ctx.config.get("output.cat-to-file").unwrap().unwrap();
+    assert_eq!(
+        cfg,
+        serde_json::json!({
+            "command": "./cat-to-file",
+            "array": [1,2,3],
+            "number": 123,
+        })
+    );
+}
+
+// An invalid key at the top level.
+#[test]
+fn bad_config_top_level() {
+    BookTest::init(|_| {})
+        .change_file("book.toml", "foo = 123")
+        .run("build", |cmd| {
+            cmd.expect_failure()
+                .expect_stdout(str![[""]])
+                .expect_stderr(str![[r#"
+[TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Invalid configuration file
+[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: TOML parse error at line 1, column 1
+  |
+1 | foo = 123
+  | ^^^
+unknown field `foo`, expected one of `book`, `build`, `rust`, `output`, `preprocessor`
+
+
+"#]]);
+        });
+}
+
+// An invalid key in the main book table.
+#[test]
+fn bad_config_in_book_table() {
+    BookTest::init(|_| {})
+        .change_file(
+            "book.toml",
+            "[book]\n\
+             title = \"bad-config\"\n\
+             foo = 123"
+        )
+        .run("build", |cmd| {
+            cmd.expect_failure()
+                .expect_stdout(str![[""]])
+                .expect_stderr(str![[r#"
+[TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Invalid configuration file
+[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: TOML parse error at line 3, column 1
+  |
+3 | foo = 123
+  | ^^^
+unknown field `foo`, expected one of `title`, `authors`, `description`, `src`, `language`, `text-direction`
+
+
+"#]]);
+        });
+}

tests/testsuite/config.rs

@@ -0,0 +1,3 @@
+# Summary
+
+- [Chapter 1](./chapter_1.md)

tests/testsuite/config/empty/book.toml

@@ -0,0 +1 @@
+# Chapter 1

tests/testsuite/config/empty/src/SUMMARY.md

@@ -28,8 +28,8 @@ All done, no errors...
         str![[r#"
 [book]
 authors = []
-language = "en"
 src = "src"
+language = "en"
 
 "#]],
     )
@@ -94,8 +94,8 @@ All done, no errors...
         str![[r#"
 [book]
 authors = []
-language = "en"
 src = "src"
+language = "en"
 
 "#]],
     );
@@ -126,10 +126,10 @@ All done, no errors...
         "book.toml",
         str![[r#"
 [book]
+title = "Example title"
 authors = []
-language = "en"
 src = "src"
-title = "Example title"
+language = "en"
 
 "#]],
     );
@@ -179,14 +179,14 @@ fn init_with_custom_book_and_src_locations() {
         str![[r#"
 [book]
 authors = []
-language = "en"
 src = "in"
+language = "en"
 
 [build]
 build-dir = "out"
 create-missing = true
-extra-watch-dirs = []
 use-default-preprocessors = true
+extra-watch-dirs = []
 
 "#]],
     )

tests/testsuite/config/empty/src/chapter_1.md

@@ -7,6 +7,7 @@
 mod book_test;
 mod build;
 mod cli;
+mod config;
 mod includes;
 mod index;
 mod init;