fix(protocol): reject one-line KDL blocks with multiple keys
In KDL, a `{ }` block's children are newline- or semicolon-separated,
so a block like `restart { policy "always" backoff-initial "10ms" }`
written on one line parses as a SINGLE node named `policy` whose
remaining words become extra arguments, not sibling keys. The parser
only ever read the first argument of each child node, so every key
after the first was silently discarded with no error.
Add `single_arg`, applied to every restart/stop/wait-for key that
takes exactly one value (all except `wait-for`'s `args`, which
legitimately takes zero or more), so a one-line block with 2+ keys
now fails to parse instead of silently keeping defaults.
`optional_string_map` (the `env { }` block) has the identical trap
but is out of scope here: it returns a bare BTreeMap with no error
path and would need a signature change to report a config error.
This commit is contained in:
@@ -154,6 +154,8 @@ fn parse_restart(doc: &KdlDocument, path: &Path) -> Result<RestartConfig, Config
|
||||
for child in children.nodes() {
|
||||
match child.name().value() {
|
||||
"policy" => {
|
||||
single_arg(child, "restart", path)?;
|
||||
|
||||
let s = string_arg(child, "policy", path)?;
|
||||
|
||||
out.policy = match s.as_str() {
|
||||
@@ -170,12 +172,18 @@ fn parse_restart(doc: &KdlDocument, path: &Path) -> Result<RestartConfig, Config
|
||||
};
|
||||
}
|
||||
"backoff-initial" => {
|
||||
single_arg(child, "restart", path)?;
|
||||
|
||||
out.backoff_initial = parse_duration_arg(child, "restart.backoff-initial", path)?;
|
||||
}
|
||||
"backoff-max" => {
|
||||
single_arg(child, "restart", path)?;
|
||||
|
||||
out.backoff_max = parse_duration_arg(child, "restart.backoff-max", path)?;
|
||||
}
|
||||
"max-retries-per-minute" => {
|
||||
single_arg(child, "restart", path)?;
|
||||
|
||||
let v = child
|
||||
.entries()
|
||||
.first()
|
||||
@@ -220,6 +228,8 @@ fn parse_stop(doc: &KdlDocument, path: &Path) -> Result<StopConfig, ConfigError>
|
||||
for child in children.nodes() {
|
||||
match child.name().value() {
|
||||
"grace" => {
|
||||
single_arg(child, "stop", path)?;
|
||||
|
||||
out.grace = parse_duration_arg(child, "stop.grace", path)?;
|
||||
}
|
||||
other => {
|
||||
@@ -274,16 +284,22 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
|
||||
for child in children.nodes() {
|
||||
match child.name().value() {
|
||||
"path" => {
|
||||
single_arg(child, "wait-for", path)?;
|
||||
|
||||
let raw = string_arg(child, "wait-for", path)?;
|
||||
|
||||
conditions.push(WaitCondition::Path(expand_tilde(&raw, home.as_deref())));
|
||||
}
|
||||
"tcp" => {
|
||||
single_arg(child, "wait-for", path)?;
|
||||
|
||||
let addr = string_arg(child, "wait-for", path)?;
|
||||
|
||||
conditions.push(WaitCondition::Tcp(addr));
|
||||
}
|
||||
"command" => {
|
||||
single_arg(child, "wait-for", path)?;
|
||||
|
||||
if command.is_some() {
|
||||
return Err(invalid("duplicate key `command`".into()));
|
||||
}
|
||||
@@ -304,9 +320,13 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
|
||||
);
|
||||
}
|
||||
"timeout" => {
|
||||
single_arg(child, "wait-for", path)?;
|
||||
|
||||
timeout = parse_duration_arg(child, "wait-for", path)?;
|
||||
}
|
||||
"interval" => {
|
||||
single_arg(child, "wait-for", path)?;
|
||||
|
||||
interval = parse_duration_arg(child, "wait-for", path)?;
|
||||
}
|
||||
other => {
|
||||
@@ -339,6 +359,21 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
|
||||
}
|
||||
}
|
||||
|
||||
fn single_arg(node: &KdlNode, field: &'static str, path: &Path) -> Result<(), ConfigError> {
|
||||
if node.entries().len() == 1 {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
Err(ConfigError::InvalidValue {
|
||||
path: path.to_path_buf(),
|
||||
field,
|
||||
message: format!(
|
||||
"unexpected extra arguments for key `{}`; put each key on its own line",
|
||||
node.name().value()
|
||||
),
|
||||
})
|
||||
}
|
||||
|
||||
fn string_arg(node: &KdlNode, field: &'static str, path: &Path) -> Result<String, ConfigError> {
|
||||
node.entries()
|
||||
.first()
|
||||
@@ -727,4 +762,52 @@ stop {
|
||||
|
||||
assert_eq!(expand_tilde("", Some(&home)), PathBuf::from(""));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_line_restart_block_with_two_keys_fails() {
|
||||
let text =
|
||||
"command \"/bin/x\"\nport 1\nrestart { policy \"always\" backoff-initial \"10ms\" }";
|
||||
let err = parse_server_config("foo", text, p()).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
err,
|
||||
ConfigError::InvalidValue {
|
||||
field: "restart",
|
||||
..
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_line_wait_for_block_with_two_keys_fails() {
|
||||
let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/x\" timeout \"5s\" }";
|
||||
let err = parse_server_config("foo", text, p()).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
err,
|
||||
ConfigError::InvalidValue {
|
||||
field: "wait-for",
|
||||
..
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_line_single_key_stop_block_still_parses() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstop { grace \"30s\" }";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(cfg.stop.grace, Duration::from_secs(30));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn one_line_single_key_wait_for_block_still_parses() {
|
||||
let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/x\" }";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
cfg.wait_for.unwrap().condition,
|
||||
WaitCondition::Path(PathBuf::from("/x"))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user