Skip to content

Commit 79e9ae4

Browse files
authored
Merge pull request #2787 from ehuss/deny-unknown-fields
Deny all unknown config fields
2 parents e284eb1 + d290727 commit 79e9ae4

File tree

11 files changed

+286
-226
lines changed

11 files changed

+286
-226
lines changed

crates/mdbook-core/src/config.rs

Lines changed: 96 additions & 190 deletions
Large diffs are not rendered by default.

crates/mdbook-driver/src/mdbook.rs

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use mdbook_preprocessor::{Preprocessor, PreprocessorContext};
1414
use mdbook_renderer::{RenderContext, Renderer};
1515
use mdbook_summary::Summary;
1616
use serde::Deserialize;
17-
use std::collections::HashMap;
1817
use std::ffi::OsString;
1918
use std::io::{IsTerminal, Write};
2019
use std::path::{Path, PathBuf};
@@ -423,22 +422,17 @@ struct OutputConfig {
423422
fn determine_renderers(config: &Config) -> Result<Vec<Box<dyn Renderer>>> {
424423
let mut renderers = Vec::new();
425424

426-
match config.get::<HashMap<String, OutputConfig>>("output") {
427-
Ok(Some(output_table)) => {
428-
renderers.extend(output_table.into_iter().map(|(key, table)| {
429-
if key == "html" {
430-
Box::new(HtmlHandlebars::new()) as Box<dyn Renderer>
431-
} else if key == "markdown" {
432-
Box::new(MarkdownRenderer::new()) as Box<dyn Renderer>
433-
} else {
434-
let command = table.command.unwrap_or_else(|| format!("mdbook-{key}"));
435-
Box::new(CmdRenderer::new(key, command))
436-
}
437-
}));
425+
let outputs = config.outputs::<OutputConfig>()?;
426+
renderers.extend(outputs.into_iter().map(|(key, table)| {
427+
if key == "html" {
428+
Box::new(HtmlHandlebars::new()) as Box<dyn Renderer>
429+
} else if key == "markdown" {
430+
Box::new(MarkdownRenderer::new()) as Box<dyn Renderer>
431+
} else {
432+
let command = table.command.unwrap_or_else(|| format!("mdbook-{key}"));
433+
Box::new(CmdRenderer::new(key, command))
438434
}
439-
Ok(None) => {}
440-
Err(e) => bail!("failed to get output table config: {e}"),
441-
}
435+
}));
442436

443437
// if we couldn't find anything, add the HTML renderer as a default
444438
if renderers.is_empty() {
@@ -477,12 +471,7 @@ fn determine_preprocessors(config: &Config) -> Result<Vec<Box<dyn Preprocessor>>
477471
}
478472
}
479473

480-
let preprocessor_table = match config.get::<HashMap<String, PreprocessorConfig>>("preprocessor")
481-
{
482-
Ok(Some(preprocessor_table)) => preprocessor_table,
483-
Ok(None) => HashMap::new(),
484-
Err(e) => bail!("failed to get preprocessor table config: {e}"),
485-
};
474+
let preprocessor_table = config.preprocessors::<PreprocessorConfig>()?;
486475

487476
for (name, table) in preprocessor_table.iter() {
488477
preprocessor_names.insert(name.to_string());

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ fn config_defaults_to_html_renderer_if_empty() {
77
let cfg = Config::default();
88

99
// make sure we haven't got anything in the `output` table
10-
assert!(cfg.get::<Value>("output").unwrap().is_none());
10+
assert!(cfg.outputs::<toml::Value>().unwrap().is_empty());
1111

1212
let got = determine_renderers(&cfg).unwrap();
1313

@@ -45,7 +45,7 @@ fn config_defaults_to_link_and_index_preprocessor_if_not_set() {
4545
let cfg = Config::default();
4646

4747
// make sure we haven't got anything in the `preprocessor` table
48-
assert!(cfg.get::<Value>("preprocessor").unwrap().is_none());
48+
assert!(cfg.preprocessors::<toml::Value>().unwrap().is_empty());
4949

5050
let got = determine_preprocessors(&cfg);
5151

src/cmd/serve.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::command_prelude::*;
22
#[cfg(feature = "watch")]
33
use super::watch;
44
use crate::{get_book_dir, open};
5-
use anyhow::{Result, bail};
5+
use anyhow::Result;
66
use axum::Router;
77
use axum::extract::ws::{Message, WebSocket, WebSocketUpgrade};
88
use axum::routing::get;
@@ -76,10 +76,8 @@ pub fn execute(args: &ArgMatches) -> Result<()> {
7676
.next()
7777
.ok_or_else(|| anyhow::anyhow!("no address found for {}", address))?;
7878
let build_dir = book.build_dir_for("html");
79-
let input_404 = match book.config.get::<String>("output.html.input-404") {
80-
Ok(v) => v,
81-
Err(e) => bail!("expected string for output.html.input-404: {e}"),
82-
};
79+
let html_config = book.config.html_config();
80+
let input_404 = html_config.and_then(|c| c.input_404);
8381
let file_404 = get_404_output_file(&input_404);
8482

8583
// A channel used to broadcast to any websockets to reload when a file changes.

tests/testsuite/config.rs

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
//! Tests for book configuration loading.
2+
3+
use crate::prelude::*;
4+
5+
// Test that config can load from environment variable.
6+
#[test]
7+
fn config_from_env() {
8+
BookTest::from_dir("config/empty")
9+
.run("build", |cmd| {
10+
cmd.env("MDBOOK_BOOK__TITLE", "Custom env title");
11+
})
12+
.check_file_contains(
13+
"book/index.html",
14+
"<title>Chapter 1 - Custom env title</title>",
15+
);
16+
17+
// json for some subtable
18+
//
19+
}
20+
21+
// Test environment config with JSON.
22+
#[test]
23+
fn config_json_from_env() {
24+
// build table
25+
BookTest::from_dir("config/empty")
26+
.run("build", |cmd| {
27+
cmd.env(
28+
"MDBOOK_BOOK",
29+
r#"{"title": "My Awesome Book", "authors": ["Michael-F-Bryan"]}"#,
30+
);
31+
})
32+
.check_file_contains(
33+
"book/index.html",
34+
"<title>Chapter 1 - My Awesome Book</title>",
35+
);
36+
37+
// book table
38+
BookTest::from_dir("config/empty")
39+
.run("build", |cmd| {
40+
cmd.env("MDBOOK_BUILD", r#"{"build-dir": "alt"}"#);
41+
})
42+
.check_file_contains("alt/index.html", "<title>Chapter 1</title>");
43+
}
44+
45+
// Test that a preprocessor receives config set in the environment.
46+
#[test]
47+
fn preprocessor_cfg_from_env() {
48+
let mut test = BookTest::from_dir("config/empty");
49+
test.rust_program(
50+
"cat-to-file",
51+
r#"
52+
fn main() {
53+
use std::io::Read;
54+
let mut s = String::new();
55+
std::io::stdin().read_to_string(&mut s).unwrap();
56+
std::fs::write("out.txt", s).unwrap();
57+
println!("{{\"sections\": []}}");
58+
}
59+
"#,
60+
)
61+
.run("build", |cmd| {
62+
cmd.env(
63+
"MDBOOK_PREPROCESSOR__CAT_TO_FILE",
64+
r#"{"command":"./cat-to-file", "array": [1,2,3], "number": 123}"#,
65+
);
66+
});
67+
let out = read_to_string(test.dir.join("out.txt"));
68+
let (ctx, _book) = mdbook_preprocessor::parse_input(out.as_bytes()).unwrap();
69+
let cfg: serde_json::Value = ctx.config.get("preprocessor.cat-to-file").unwrap().unwrap();
70+
assert_eq!(
71+
cfg,
72+
serde_json::json!({
73+
"command": "./cat-to-file",
74+
"array": [1,2,3],
75+
"number": 123,
76+
})
77+
);
78+
}
79+
80+
// Test that a renderer receives config set in the environment.
81+
#[test]
82+
fn output_cfg_from_env() {
83+
let mut test = BookTest::from_dir("config/empty");
84+
test.rust_program(
85+
"cat-to-file",
86+
r#"
87+
fn main() {
88+
use std::io::Read;
89+
let mut s = String::new();
90+
std::io::stdin().read_to_string(&mut s).unwrap();
91+
std::fs::write("out.txt", s).unwrap();
92+
}
93+
"#,
94+
)
95+
.run("build", |cmd| {
96+
cmd.env(
97+
"MDBOOK_OUTPUT__CAT_TO_FILE",
98+
r#"{"command":"./cat-to-file", "array": [1,2,3], "number": 123}"#,
99+
);
100+
});
101+
let out = read_to_string(test.dir.join("book/out.txt"));
102+
let ctx = mdbook_renderer::RenderContext::from_json(out.as_bytes()).unwrap();
103+
let cfg: serde_json::Value = ctx.config.get("output.cat-to-file").unwrap().unwrap();
104+
assert_eq!(
105+
cfg,
106+
serde_json::json!({
107+
"command": "./cat-to-file",
108+
"array": [1,2,3],
109+
"number": 123,
110+
})
111+
);
112+
}
113+
114+
// An invalid key at the top level.
115+
#[test]
116+
fn bad_config_top_level() {
117+
BookTest::init(|_| {})
118+
.change_file("book.toml", "foo = 123")
119+
.run("build", |cmd| {
120+
cmd.expect_failure()
121+
.expect_stdout(str![[""]])
122+
.expect_stderr(str![[r#"
123+
[TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Invalid configuration file
124+
[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: TOML parse error at line 1, column 1
125+
|
126+
1 | foo = 123
127+
| ^^^
128+
unknown field `foo`, expected one of `book`, `build`, `rust`, `output`, `preprocessor`
129+
130+
131+
"#]]);
132+
});
133+
}
134+
135+
// An invalid key in the main book table.
136+
#[test]
137+
fn bad_config_in_book_table() {
138+
BookTest::init(|_| {})
139+
.change_file(
140+
"book.toml",
141+
"[book]\n\
142+
title = \"bad-config\"\n\
143+
foo = 123"
144+
)
145+
.run("build", |cmd| {
146+
cmd.expect_failure()
147+
.expect_stdout(str![[""]])
148+
.expect_stderr(str![[r#"
149+
[TIMESTAMP] [ERROR] (mdbook_core::utils): Error: Invalid configuration file
150+
[TIMESTAMP] [ERROR] (mdbook_core::utils): [TAB]Caused By: TOML parse error at line 3, column 1
151+
|
152+
3 | foo = 123
153+
| ^^^
154+
unknown field `foo`, expected one of `title`, `authors`, `description`, `src`, `language`, `text-direction`
155+
156+
157+
"#]]);
158+
});
159+
}

tests/testsuite/config/empty/book.toml

Whitespace-only changes.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
# Summary
2+
3+
- [Chapter 1](./chapter_1.md)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Chapter 1

tests/testsuite/init.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ All done, no errors...
2828
str![[r#"
2929
[book]
3030
authors = []
31-
language = "en"
3231
src = "src"
32+
language = "en"
3333
3434
"#]],
3535
)
@@ -94,8 +94,8 @@ All done, no errors...
9494
str![[r#"
9595
[book]
9696
authors = []
97-
language = "en"
9897
src = "src"
98+
language = "en"
9999
100100
"#]],
101101
);
@@ -126,10 +126,10 @@ All done, no errors...
126126
"book.toml",
127127
str![[r#"
128128
[book]
129+
title = "Example title"
129130
authors = []
130-
language = "en"
131131
src = "src"
132-
title = "Example title"
132+
language = "en"
133133
134134
"#]],
135135
);
@@ -179,14 +179,14 @@ fn init_with_custom_book_and_src_locations() {
179179
str![[r#"
180180
[book]
181181
authors = []
182-
language = "en"
183182
src = "in"
183+
language = "en"
184184
185185
[build]
186186
build-dir = "out"
187187
create-missing = true
188-
extra-watch-dirs = []
189188
use-default-preprocessors = true
189+
extra-watch-dirs = []
190190
191191
"#]],
192192
)

tests/testsuite/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
mod book_test;
88
mod build;
99
mod cli;
10+
mod config;
1011
mod includes;
1112
mod index;
1213
mod init;

0 commit comments

Comments
 (0)