Fix and harden client/server configuration #48

Merged
sjg merged 13 commits from feat/config-improvements into main 2026-08-06 22:22:40 +02:00
12 changed files with 563 additions and 369 deletions
Showing only changes of commit 335922fecc - Show all commits
Generated
+12
View File
@@ -2412,6 +2412,16 @@ dependencies = [
"syn",
]
[[package]]
name = "serde_ignored"
version = "0.1.14"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "115dffd5f3853e06e746965a20dcbae6ee747ae30b543d91b0e089668bb07798"
dependencies = [
"serde",
"serde_core",
]
[[package]]
name = "serde_json"
version = "1.0.149"
@@ -3127,6 +3137,7 @@ version = "0.1.0"
dependencies = [
"dirs",
"serde",
"serde_ignored",
"thiserror 2.0.18",
"toml",
"tracing",
@@ -3143,6 +3154,7 @@ dependencies = [
"dialoguer",
"tempfile",
"tokio-serial",
"toml",
"toml_edit 0.22.27",
"trx-config",
]
+13 -6
View File
@@ -50,6 +50,9 @@ struct Cli {
/// Print example configuration and exit
#[arg(long = "print-config")]
print_config: bool,
/// Treat unknown configuration keys as a fatal error
#[arg(long = "strict-config")]
strict_config: bool,
/// Remote server URL (host:port)
#[arg(short = 'u', long = "url")]
url: Option<String>,
@@ -135,20 +138,24 @@ async fn async_init() -> DynResult<AppState> {
std::process::exit(0);
}
let (cfg, config_path) = if let Some(ref path) = cli.config {
let cfg = ClientConfig::load_from_file(path)?;
(cfg, Some(path.clone()))
let loaded = if let Some(ref path) = cli.config {
ClientConfig::load_from_file(path)?
} else {
ClientConfig::load_from_default_paths()?
};
cfg.validate()
.map_err(|e| format!("Invalid client configuration: {}", e))?;
let config_path = loaded.path.clone();
init_logging(cfg.general.log_level.as_deref());
// Logging comes up before any config complaint so the warnings are visible.
init_logging(loaded.config.general.log_level.as_deref());
if let Some(ref path) = config_path {
info!("Loaded configuration from {}", path.display());
}
loaded.report_unknown_keys(cli.strict_config)?;
let cfg = loaded.config;
cfg.validate()
.map_err(|e| format!("Invalid client configuration: {}", e))?;
frontend_runtime.http_auth.tokens = cfg
.frontends
+1
View File
@@ -17,3 +17,4 @@ thiserror = "2"
trx-core = { path = "../trx-core" }
trx-decode-log = { path = "../decoders/trx-decode-log" }
trx-reporting = { path = "../trx-reporting" }
serde_ignored = "0.1"
+20 -4
View File
@@ -13,11 +13,11 @@
use std::collections::HashMap;
use std::net::IpAddr;
use std::path::{Path, PathBuf};
use std::path::Path;
use std::time::Duration;
use serde::{Deserialize, Serialize};
use crate::file::{ConfigError, ConfigFile};
use crate::file::{ConfigError, ConfigFile, ConfigLoad};
use crate::shared::{validate_log_level, validate_tokens};
/// Top-level client configuration structure.
@@ -609,13 +609,13 @@ impl ClientConfig {
}
/// Load configuration from a specific file path.
pub fn load_from_file(path: &Path) -> Result<Self, ConfigError> {
pub fn load_from_file(path: &Path) -> Result<ConfigLoad<Self>, ConfigError> {
<Self as ConfigFile>::load_from_file(path)
}
/// Load configuration from the default search paths.
/// Returns default config if no config file is found.
pub fn load_from_default_paths() -> Result<(Self, Option<PathBuf>), ConfigError> {
pub fn load_from_default_paths() -> Result<ConfigLoad<Self>, ConfigError> {
<Self as ConfigFile>::load_from_default_paths()
}
@@ -734,6 +734,22 @@ impl ConfigFile for ClientConfig {
fn section_key() -> &'static str {
"trx-client"
}
/// Include one `[[remotes]]` entry so unknown keys nested inside a remote
/// entry still get a suggestion.
fn reference_value() -> toml::Value {
let cfg = ClientConfig {
remotes: vec![RemoteEntry {
name: "example".to_string(),
url: "127.0.0.1:4530".to_string(),
rig_id: Some("hf".to_string()),
auth: RemoteAuthConfig::default(),
poll_interval_ms: default_poll_interval_ms(),
}],
..Default::default()
};
toml::Value::try_from(cfg).unwrap_or_else(|_| toml::Value::Table(toml::Table::new()))
}
}
#[cfg(test)]
+79 -22
View File
@@ -18,12 +18,44 @@
//! falling back to defaults.
use serde::de::DeserializeOwned;
use serde::Serialize;
use std::path::{Path, PathBuf};
use thiserror::Error;
use crate::unknown::{describe, UnknownKey};
/// Every section key that may appear at the root of a combined config file.
pub const SECTION_KEYS: &[&str] = &["trx-server", "trx-client"];
/// A loaded config plus what the loader noticed on the way in.
#[derive(Debug, Clone)]
pub struct ConfigLoad<T> {
/// The deserialized configuration.
pub config: T,
/// File the config came from; `None` when nothing was found and defaults
/// were used.
pub path: Option<PathBuf>,
/// Keys present in the file that no config field claimed.
pub unknown_keys: Vec<UnknownKey>,
}
impl<T> ConfigLoad<T> {
/// Log every unknown key as a warning. With `strict`, also return an error
/// so the caller can refuse to start.
pub fn report_unknown_keys(&self, strict: bool) -> Result<(), String> {
for key in &self.unknown_keys {
tracing::warn!("{}", key);
}
if strict && !self.unknown_keys.is_empty() {
return Err(format!(
"{} unknown config key(s); refusing to start because --strict-config is set",
self.unknown_keys.len()
));
}
Ok(())
}
}
#[derive(Debug, Error)]
pub enum ConfigError {
#[error("Failed to read config file {0}: {1}")]
@@ -62,12 +94,13 @@ fn select_section(table: &toml::Table, key: &str) -> Option<toml::Value> {
/// Extract and deserialize a named section from a TOML file.
///
/// Returns `Ok(Some(cfg))` when the section is present and parses cleanly,
/// `Ok(None)` when the section is absent, or `Err` on I/O / parse failure.
fn load_section_from_file<T: DeserializeOwned>(
/// Returns `Ok(Some((cfg, unknown_keys)))` when the section is present and
/// parses cleanly, `Ok(None)` when the section is absent, or `Err` on I/O /
/// parse failure.
fn load_section_from_file<T: ConfigFile>(
path: &Path,
key: &str,
) -> Result<Option<T>, ConfigError> {
) -> Result<Option<(T, Vec<UnknownKey>)>, ConfigError> {
let content = std::fs::read_to_string(path)
.map_err(|e| ConfigError::ReadError(path.to_path_buf(), e.to_string()))?;
@@ -78,48 +111,72 @@ fn load_section_from_file<T: DeserializeOwned>(
return Ok(None);
};
// Re-serialize the section then parse as T so all serde defaults apply.
let section_toml = toml::to_string(&section)
// Deserialize straight from the TOML value so serde applies every default,
// recording any key no field claimed.
let mut ignored: Vec<String> = Vec::new();
let cfg: T = serde_ignored::deserialize(section, |path| ignored.push(path.to_string()))
.map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?;
let cfg = toml::from_str::<T>(&section_toml)
.map_err(|e| ConfigError::ParseError(path.to_path_buf(), e.to_string()))?;
Ok(Some(cfg))
Ok(Some((cfg, describe(&ignored, &T::reference_value()))))
}
/// Trait for loading configuration from a `trx-rs.toml` section.
pub trait ConfigFile: Sized + Default + DeserializeOwned {
pub trait ConfigFile: Sized + Default + DeserializeOwned + Serialize {
/// Section key in `trx-rs.toml` (e.g. `"trx-server"` or `"trx-client"`).
fn section_key() -> &'static str;
/// A TOML rendering of a populated config, used to suggest corrections for
/// unknown keys. Implementations should fill in list-valued sections such
/// as `[[rigs]]` so keys nested inside them can be suggested too.
fn reference_value() -> toml::Value {
toml::Value::try_from(Self::default())
.unwrap_or_else(|_| toml::Value::Table(toml::Table::new()))
}
/// Load the section from a specific file path.
///
/// Accepts both a sectioned file (`[<section_key>]` at the root) and a bare
/// file whose root is the section itself. Returns an error if the file
/// cannot be read, is not valid TOML, or is sectioned for some other
/// component only.
fn load_from_file(path: &Path) -> Result<Self, ConfigError> {
load_section_from_file::<Self>(path, Self::section_key())?.ok_or_else(|| {
ConfigError::ParseError(
path.to_path_buf(),
format!("missing [{}] section", Self::section_key()),
)
fn load_from_file(path: &Path) -> Result<ConfigLoad<Self>, ConfigError> {
let (config, unknown_keys) = load_section_from_file::<Self>(path, Self::section_key())?
.ok_or_else(|| {
ConfigError::ParseError(
path.to_path_buf(),
format!("missing [{}] section", Self::section_key()),
)
})?;
Ok(ConfigLoad {
config,
path: Some(path.to_path_buf()),
unknown_keys,
})
}
/// Search default paths (`trx-rs.toml` in CWD → XDG → /etc) and load
/// the first file that contains the expected section.
///
/// Returns `(config, path_where_found)` or `(Default::default(), None)`
/// when no config file is found.
fn load_from_default_paths() -> Result<(Self, Option<PathBuf>), ConfigError> {
/// Falls back to `Self::default()` with no path when nothing is found.
fn load_from_default_paths() -> Result<ConfigLoad<Self>, ConfigError> {
for path in config_search_paths() {
if path.exists() {
if let Some(cfg) = load_section_from_file::<Self>(&path, Self::section_key())? {
return Ok((cfg, Some(path)));
if let Some((config, unknown_keys)) =
load_section_from_file::<Self>(&path, Self::section_key())?
{
return Ok(ConfigLoad {
config,
path: Some(path),
unknown_keys,
});
}
}
}
Ok((Self::default(), None))
Ok(ConfigLoad {
config: Self::default(),
path: None,
unknown_keys: Vec::new(),
})
}
}
+3 -1
View File
@@ -13,10 +13,12 @@ pub mod client;
pub mod file;
pub mod server;
pub mod shared;
pub mod unknown;
pub mod url;
pub use client::ClientConfig;
pub use file::{ConfigError, ConfigFile};
pub use file::{ConfigError, ConfigFile, ConfigLoad};
pub use server::ServerConfig;
pub use shared::{validate_log_level, validate_tokens};
pub use unknown::UnknownKey;
pub use url::{parse_audio_url, parse_remote_url, RemoteEndpoint};
+14 -4
View File
@@ -12,10 +12,10 @@
//! 4. `/etc/trx-rs/trx-rs.toml`
use std::net::IpAddr;
use std::path::{Path, PathBuf};
use std::path::Path;
use serde::{Deserialize, Serialize};
use crate::file::{ConfigError, ConfigFile};
use crate::file::{ConfigError, ConfigFile, ConfigLoad};
use crate::shared::{validate_log_level, validate_tokens};
pub use trx_decode_log::DecodeLogsConfig;
@@ -706,13 +706,13 @@ impl ServerConfig {
}
/// Load configuration from a specific file path.
pub fn load_from_file(path: &Path) -> Result<Self, ConfigError> {
pub fn load_from_file(path: &Path) -> Result<ConfigLoad<Self>, ConfigError> {
<Self as ConfigFile>::load_from_file(path)
}
/// Load configuration from the default search paths.
/// Returns default config if no config file is found.
pub fn load_from_default_paths() -> Result<(Self, Option<PathBuf>), ConfigError> {
pub fn load_from_default_paths() -> Result<ConfigLoad<Self>, ConfigError> {
<Self as ConfigFile>::load_from_default_paths()
}
@@ -900,6 +900,16 @@ impl ConfigFile for ServerConfig {
fn section_key() -> &'static str {
"trx-server"
}
/// Include one `[[rigs]]` entry so unknown keys nested inside a rig entry
/// still get a suggestion.
fn reference_value() -> toml::Value {
let cfg = ServerConfig {
rigs: vec![RigInstanceConfig::default()],
..Default::default()
};
toml::Value::try_from(cfg).unwrap_or_else(|_| toml::Value::Table(toml::Table::new()))
}
}
#[cfg(test)]
+256
View File
@@ -0,0 +1,256 @@
// SPDX-FileCopyrightText: 2026 Stan Grams <sjg@haxx.space>
//
// SPDX-License-Identifier: GPL-2.0-or-later
//! Reporting for config keys the deserializer ignored.
//!
//! Every config struct is `#[serde(default)]`, so a misspelled key used to be
//! dropped without a word and the setting silently kept its default. The
//! loader now collects the ignored key paths and pairs each with the closest
//! known key at the same level, so `prot = 9999` reads as a typo instead of
//! looking like it worked.
use std::collections::BTreeSet;
use std::fmt;
/// A config key that the deserializer did not recognise.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct UnknownKey {
/// Dotted path of the key, e.g. `listen.prot` or `rigs.0.audio.prot`.
pub path: String,
/// Closest known key at the same level, when one is near enough to suggest.
pub suggestion: Option<String>,
}
impl fmt::Display for UnknownKey {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.suggestion {
Some(s) => write!(f, "unknown config key '{}' (did you mean '{}'?)", self.path, s),
None => write!(f, "unknown config key '{}'", self.path),
}
}
}
/// Flatten a TOML value into the set of dotted key paths it contains.
///
/// Array indices are normalised to `0` so a path inside `[[rigs]]` matches
/// whichever entry it came from.
pub fn flatten_paths(value: &toml::Value) -> BTreeSet<String> {
let mut paths = BTreeSet::new();
walk(value, "", &mut paths);
paths
}
fn walk(value: &toml::Value, prefix: &str, paths: &mut BTreeSet<String>) {
let join = |seg: &str| {
if prefix.is_empty() {
seg.to_string()
} else {
format!("{prefix}.{seg}")
}
};
match value {
toml::Value::Table(table) => {
for (key, child) in table {
let path = join(key);
paths.insert(path.clone());
walk(child, &path, paths);
}
}
toml::Value::Array(items) => {
// Every entry of an array of tables has the same shape, so collapse
// them onto index 0 and let one entry stand for all.
for item in items {
let path = join("0");
walk(item, &path, paths);
}
}
_ => {}
}
}
/// Replace numeric path segments with `0` so array entries compare equal.
fn normalize(path: &str) -> String {
path.split('.')
.map(|seg| {
if !seg.is_empty() && seg.bytes().all(|b| b.is_ascii_digit()) {
"0"
} else {
seg
}
})
.collect::<Vec<_>>()
.join(".")
}
/// Suggest the closest known key that sits at the same level as `path`.
///
/// Returns `None` when nothing is close enough to be worth printing.
pub fn suggest(path: &str, known: &BTreeSet<String>) -> Option<String> {
let normalized = normalize(path);
let (parent, leaf) = match normalized.rsplit_once('.') {
Some((parent, leaf)) => (parent, leaf),
None => ("", normalized.as_str()),
};
// Anything longer than this is a different word, not a typo.
let limit = (leaf.chars().count() / 3).clamp(1, 3);
let mut best: Option<(usize, &str)> = None;
for candidate in known {
let (cand_parent, cand_leaf) = match candidate.rsplit_once('.') {
Some((p, l)) => (p, l),
None => ("", candidate.as_str()),
};
if cand_parent != parent || cand_leaf == leaf {
continue;
}
let distance = edit_distance(leaf, cand_leaf);
if distance <= limit && best.is_none_or(|(best_d, _)| distance < best_d) {
best = Some((distance, cand_leaf));
}
}
best.map(|(_, leaf)| {
if parent.is_empty() {
leaf.to_string()
} else {
format!("{parent}.{leaf}")
}
})
}
/// Pair each ignored path with a suggestion drawn from `reference`.
pub fn describe(paths: &[String], reference: &toml::Value) -> Vec<UnknownKey> {
let known = flatten_paths(reference);
paths
.iter()
.map(|path| UnknownKey {
path: path.clone(),
suggestion: suggest(path, &known),
})
.collect()
}
/// Optimal string alignment distance: Levenshtein plus transpositions, so the
/// common `port` → `prot` slip counts as one mistake rather than two.
fn edit_distance(a: &str, b: &str) -> usize {
let a: Vec<char> = a.chars().collect();
let b: Vec<char> = b.chars().collect();
if a.is_empty() {
return b.len();
}
if b.is_empty() {
return a.len();
}
let mut rows = vec![vec![0usize; b.len() + 1]; a.len() + 1];
for (i, row) in rows.iter_mut().enumerate() {
row[0] = i;
}
for (j, cell) in rows[0].iter_mut().enumerate() {
*cell = j;
}
for i in 1..=a.len() {
for j in 1..=b.len() {
let cost = usize::from(a[i - 1] != b[j - 1]);
let mut best = (rows[i - 1][j] + 1)
.min(rows[i][j - 1] + 1)
.min(rows[i - 1][j - 1] + cost);
if i > 1 && j > 1 && a[i - 1] == b[j - 2] && a[i - 2] == b[j - 1] {
best = best.min(rows[i - 2][j - 2] + 1);
}
rows[i][j] = best;
}
}
rows[a.len()][b.len()]
}
#[cfg(test)]
mod tests {
use super::*;
fn reference() -> toml::Value {
toml::from_str(
r#"
[general]
callsign = "N0CALL"
log_level = "info"
[listen]
enabled = true
port = 4530
[[rigs]]
id = "hf"
[rigs.audio]
port = 4531
sample_rate = 48000
"#,
)
.unwrap()
}
#[test]
fn test_flatten_collects_nested_paths() {
let paths = flatten_paths(&reference());
assert!(paths.contains("general.callsign"));
assert!(paths.contains("listen.port"));
assert!(paths.contains("rigs.0.audio.sample_rate"));
}
#[test]
fn test_suggest_finds_close_sibling() {
let known = flatten_paths(&reference());
assert_eq!(suggest("listen.prot", &known).as_deref(), Some("listen.port"));
}
#[test]
fn test_suggest_inside_array_entry() {
let known = flatten_paths(&reference());
assert_eq!(
suggest("rigs.1.audio.prot", &known).as_deref(),
Some("rigs.0.audio.port")
);
}
#[test]
fn test_suggest_ignores_distant_names() {
let known = flatten_paths(&reference());
assert_eq!(suggest("listen.bananas", &known), None);
}
#[test]
fn test_suggest_does_not_cross_levels() {
let known = flatten_paths(&reference());
// `port` exists under [listen], but not under [general].
assert_eq!(suggest("general.port", &known), None);
}
#[test]
fn test_describe_formats_message() {
let described = describe(&["listen.prot".to_string()], &reference());
assert_eq!(
described[0].to_string(),
"unknown config key 'listen.prot' (did you mean 'listen.port'?)"
);
}
#[test]
fn test_describe_without_suggestion() {
let described = describe(&["listen.bananas".to_string()], &reference());
assert_eq!(
described[0].to_string(),
"unknown config key 'listen.bananas'"
);
}
#[test]
fn test_edit_distance_counts_transposition_once() {
assert_eq!(edit_distance("port", "prot"), 1);
assert_eq!(edit_distance("port", "port"), 0);
assert_eq!(edit_distance("", "port"), 4);
assert_eq!(edit_distance("sample_rat", "sample_rate"), 1);
}
}
+1
View File
@@ -15,6 +15,7 @@ path = "src/main.rs"
clap = { workspace = true, features = ["derive"] }
dialoguer = "0.11"
tokio-serial = { workspace = true }
toml = "0.8"
toml_edit = "0.22"
trx-config = { path = "../trx-config" }
+136 -319
View File
@@ -2,80 +2,37 @@
//
// SPDX-License-Identifier: GPL-2.0-or-later
//! `trx-configurator --check`: run a config file through the same loader and
//! validators the binaries use.
//!
//! This used to be a second, hand-maintained implementation — lists of known
//! keys and a handful of re-implemented range checks — which drifted out of
//! date as soon as a field was added. It now defers entirely to `trx-config`,
//! so a config that checks clean here is one the binaries will accept.
use std::fmt::Write as _;
use std::path::Path;
use toml_edit::DocumentMut;
use trx_config::{ClientConfig, ServerConfig};
/// Known top-level keys for a standalone server config.
const SERVER_KEYS: &[&str] = &[
"general",
/// Top-level keys that only appear in a server config. Used solely to guess
/// what a section-less file is meant to be; the real key checking is done by
/// the loader.
const SERVER_MARKERS: &[&str] = &[
"rig",
"rigs",
"behavior",
"listen",
"audio",
"behavior",
"sdr",
"pskreporter",
"aprsfi",
"decode_logs",
"timeouts",
"audio",
];
/// Known top-level keys for a standalone client config.
const CLIENT_KEYS: &[&str] = &["general", "remote", "remotes", "frontends"];
/// Known top-level keys for a combined trx-rs.toml.
const COMBINED_KEYS: &[&str] = &["trx-server", "trx-client"];
/// Known sub-keys within [general] (server).
const SERVER_GENERAL_KEYS: &[&str] = &["callsign", "log_level", "latitude", "longitude"];
/// Known sub-keys within [general] (client).
const CLIENT_GENERAL_KEYS: &[&str] = &[
"callsign",
"log_level",
"website_url",
"website_name",
"ais_vessel_url_base",
];
/// Known sub-keys within [rig].
const RIG_KEYS: &[&str] = &["model", "initial_freq_hz", "initial_mode", "access"];
/// Known sub-keys within [rig.access].
const ACCESS_KEYS: &[&str] = &["type", "port", "baud", "host", "tcp_port", "args"];
/// Known sub-keys within [listen].
const LISTEN_KEYS: &[&str] = &["enabled", "listen", "port", "auth"];
/// Known sub-keys within [audio] (server).
const AUDIO_KEYS: &[&str] = &[
"enabled",
"listen",
"port",
"rx_enabled",
"tx_enabled",
"device",
"sample_rate",
"channels",
"frame_duration_ms",
"bitrate_bps",
];
/// Known sub-keys within [behavior].
const BEHAVIOR_KEYS: &[&str] = &[
"poll_interval_ms",
"poll_interval_tx_ms",
"max_retries",
"retry_base_delay_ms",
"vfo_prime",
];
/// Known sub-keys within [remote].
const REMOTE_KEYS: &[&str] = &["url", "rig_id", "auth", "poll_interval_ms"];
/// Known sub-keys within [frontends].
const FRONTENDS_KEYS: &[&str] = &["http", "rigctl", "http_json", "audio"];
/// Top-level keys that only appear in a client config.
const CLIENT_MARKERS: &[&str] = &["remote", "remotes", "frontends"];
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum DetectedType {
@@ -100,49 +57,38 @@ pub fn check_file(path: &Path) -> Result<String, String> {
let content = std::fs::read_to_string(path)
.map_err(|e| format!("Failed to read {}: {}", path.display(), e))?;
// Step 1: TOML syntax check
let doc: DocumentMut = content
.parse()
let table: toml::Table = toml::from_str(&content)
.map_err(|e| format!("{}: TOML syntax error: {}", path.display(), e))?;
let detected = detect_type(&table);
let mut report = String::new();
let mut warnings: Vec<String> = Vec::new();
let mut errors: Vec<String> = Vec::new();
let table = doc.as_table();
// Step 2: Detect config type
let detected = detect_type(table);
writeln!(report, "{}: valid TOML", path.display()).unwrap();
writeln!(report, " Detected type: {}", detected).unwrap();
// Step 3: Structural validation
match detected {
DetectedType::Server => {
check_unknown_keys(table, SERVER_KEYS, "", &mut warnings);
check_server_sections(table, "", &mut warnings, &mut errors);
}
DetectedType::Client => {
check_unknown_keys(table, CLIENT_KEYS, "", &mut warnings);
check_client_sections(table, "", &mut warnings, &mut errors);
}
DetectedType::Server => check_server(path, &mut warnings, &mut errors),
DetectedType::Client => check_client(path, &mut warnings, &mut errors),
DetectedType::Combined => {
check_unknown_keys(table, COMBINED_KEYS, "", &mut warnings);
if let Some(server) = table.get("trx-server").and_then(|v| v.as_table()) {
check_unknown_keys(server, SERVER_KEYS, "[trx-server].", &mut warnings);
check_server_sections(server, "[trx-server].", &mut warnings, &mut errors);
if table.contains_key("trx-server") {
check_server(path, &mut warnings, &mut errors);
}
if let Some(client) = table.get("trx-client").and_then(|v| v.as_table()) {
check_unknown_keys(client, CLIENT_KEYS, "[trx-client].", &mut warnings);
check_client_sections(client, "[trx-client].", &mut warnings, &mut errors);
if table.contains_key("trx-client") {
check_client(path, &mut warnings, &mut errors);
}
}
DetectedType::Unknown => {
warnings.push("Could not detect config type. Expected server, client, or combined (trx-rs.toml) layout.".to_string());
warnings.push(
"Could not detect config type. Expected server, client, or combined \
(trx-rs.toml) layout."
.to_string(),
);
}
}
// Step 4: Format report
for w in &warnings {
writeln!(report, " warning: {}", w).unwrap();
}
@@ -169,23 +115,54 @@ pub fn check_file(path: &Path) -> Result<String, String> {
}
}
fn detect_type(table: &toml_edit::Table) -> DetectedType {
fn check_server(path: &Path, warnings: &mut Vec<String>, errors: &mut Vec<String>) {
match ServerConfig::load_from_file(path) {
Ok(loaded) => {
warnings.extend(loaded.unknown_keys.iter().map(|k| k.to_string()));
if let Err(e) = loaded.config.validate() {
errors.push(format!("[trx-server] {}", e));
}
errors.extend(
loaded
.config
.validate_sdr()
.into_iter()
.map(|e| format!("[trx-server] {}", e)),
);
}
Err(e) => errors.push(e.to_string()),
}
}
fn check_client(path: &Path, warnings: &mut Vec<String>, errors: &mut Vec<String>) {
match ClientConfig::load_from_file(path) {
Ok(loaded) => {
warnings.extend(loaded.unknown_keys.iter().map(|k| k.to_string()));
if let Err(e) = loaded.config.validate() {
errors.push(format!("[trx-client] {}", e));
}
}
Err(e) => errors.push(e.to_string()),
}
}
fn detect_type(table: &toml::Table) -> DetectedType {
if table.contains_key("trx-server") || table.contains_key("trx-client") {
return DetectedType::Combined;
}
let keys: Vec<&str> = table.iter().map(|(k, _)| k).collect();
let keys: Vec<&str> = table.keys().map(|k| k.as_str()).collect();
let server_score = keys.iter().filter(|k| SERVER_KEYS.contains(k)).count();
let client_score = keys.iter().filter(|k| CLIENT_KEYS.contains(k)).count();
// Use distinguishing keys to break ties
if keys.contains(&"rig") || keys.contains(&"rigs") || keys.contains(&"listen") {
// Distinguishing keys first, then a simple majority.
if keys.iter().any(|k| ["rig", "rigs", "listen"].contains(k)) {
return DetectedType::Server;
}
if keys.contains(&"remote") || keys.contains(&"remotes") || keys.contains(&"frontends") {
if keys.iter().any(|k| CLIENT_MARKERS.contains(k)) {
return DetectedType::Client;
}
let server_score = keys.iter().filter(|k| SERVER_MARKERS.contains(k)).count();
let client_score = keys.iter().filter(|k| CLIENT_MARKERS.contains(k)).count();
if server_score > client_score {
DetectedType::Server
} else if client_score > server_score {
@@ -197,194 +174,6 @@ fn detect_type(table: &toml_edit::Table) -> DetectedType {
}
}
fn check_unknown_keys(
table: &toml_edit::Table,
known: &[&str],
prefix: &str,
warnings: &mut Vec<String>,
) {
for (key, _) in table.iter() {
if !known.contains(&key) {
warnings.push(format!("{}unknown key '{}'", prefix, key));
}
}
}
fn check_server_sections(
table: &toml_edit::Table,
prefix: &str,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) {
if let Some(general) = table.get("general").and_then(|v| v.as_table()) {
check_unknown_keys(
general,
SERVER_GENERAL_KEYS,
&format!("{}[general].", prefix),
warnings,
);
validate_log_level(general, &format!("{}[general]", prefix), errors);
validate_coordinates(general, &format!("{}[general]", prefix), errors);
}
if let Some(rig) = table.get("rig").and_then(|v| v.as_table()) {
check_unknown_keys(rig, RIG_KEYS, &format!("{}[rig].", prefix), warnings);
if let Some(access) = rig.get("access").and_then(|v| v.as_table()) {
check_unknown_keys(
access,
ACCESS_KEYS,
&format!("{}[rig.access].", prefix),
warnings,
);
validate_access(access, &format!("{}[rig.access]", prefix), errors);
}
}
if let Some(listen) = table.get("listen").and_then(|v| v.as_table()) {
check_unknown_keys(
listen,
LISTEN_KEYS,
&format!("{}[listen].", prefix),
warnings,
);
validate_port(listen, "port", &format!("{}[listen]", prefix), errors);
}
if let Some(audio) = table.get("audio").and_then(|v| v.as_table()) {
check_unknown_keys(audio, AUDIO_KEYS, &format!("{}[audio].", prefix), warnings);
validate_port(audio, "port", &format!("{}[audio]", prefix), errors);
}
if let Some(behavior) = table.get("behavior").and_then(|v| v.as_table()) {
check_unknown_keys(
behavior,
BEHAVIOR_KEYS,
&format!("{}[behavior].", prefix),
warnings,
);
}
}
fn check_client_sections(
table: &toml_edit::Table,
prefix: &str,
warnings: &mut Vec<String>,
errors: &mut Vec<String>,
) {
if let Some(general) = table.get("general").and_then(|v| v.as_table()) {
check_unknown_keys(
general,
CLIENT_GENERAL_KEYS,
&format!("{}[general].", prefix),
warnings,
);
validate_log_level(general, &format!("{}[general]", prefix), errors);
}
if let Some(remote) = table.get("remote").and_then(|v| v.as_table()) {
check_unknown_keys(
remote,
REMOTE_KEYS,
&format!("{}[remote].", prefix),
warnings,
);
}
if let Some(frontends) = table.get("frontends").and_then(|v| v.as_table()) {
check_unknown_keys(
frontends,
FRONTENDS_KEYS,
&format!("{}[frontends].", prefix),
warnings,
);
if let Some(http) = frontends.get("http").and_then(|v| v.as_table()) {
validate_port(http, "port", &format!("{}[frontends.http]", prefix), errors);
}
if let Some(rigctl) = frontends.get("rigctl").and_then(|v| v.as_table()) {
validate_port(
rigctl,
"port",
&format!("{}[frontends.rigctl]", prefix),
errors,
);
}
}
}
// ── Value validators ────────────────────────────────────────────────────
fn validate_log_level(table: &toml_edit::Table, context: &str, errors: &mut Vec<String>) {
if let Some(level) = table.get("log_level").and_then(|v| v.as_str()) {
if !["trace", "debug", "info", "warn", "error"].contains(&level) {
errors.push(format!(
"{}.log_level '{}' is invalid (expected: trace, debug, info, warn, error)",
context, level
));
}
}
}
fn validate_coordinates(table: &toml_edit::Table, context: &str, errors: &mut Vec<String>) {
if let Some(lat) = table
.get("latitude")
.and_then(|v| v.as_float().or_else(|| v.as_integer().map(|i| i as f64)))
{
if !(-90.0..=90.0).contains(&lat) {
errors.push(format!(
"{}.latitude {} is out of range (-90..90)",
context, lat
));
}
}
if let Some(lon) = table
.get("longitude")
.and_then(|v| v.as_float().or_else(|| v.as_integer().map(|i| i as f64)))
{
if !(-180.0..=180.0).contains(&lon) {
errors.push(format!(
"{}.longitude {} is out of range (-180..180)",
context, lon
));
}
}
let has_lat = table.contains_key("latitude");
let has_lon = table.contains_key("longitude");
if has_lat != has_lon {
errors.push(format!(
"{}: latitude and longitude must be set together or both omitted",
context
));
}
}
fn validate_port(table: &toml_edit::Table, key: &str, context: &str, errors: &mut Vec<String>) {
if let Some(port) = table.get(key).and_then(|v| v.as_integer()) {
if let Some(enabled) = table.get("enabled").and_then(|v| v.as_bool()) {
if enabled && port <= 0 {
errors.push(format!("{}.{} must be > 0 when enabled", context, key));
}
}
if !(0..=65535).contains(&port) {
errors.push(format!(
"{}.{} {} is out of range (0..65535)",
context, key, port
));
}
}
}
fn validate_access(table: &toml_edit::Table, context: &str, errors: &mut Vec<String>) {
if let Some(access_type) = table.get("type").and_then(|v| v.as_str()) {
if !["serial", "tcp", "sdr"].contains(&access_type) {
errors.push(format!(
"{}.type '{}' is invalid (expected: serial, tcp, sdr)",
context, access_type
));
}
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -418,8 +207,7 @@ enabled = true
port = 4530
"#,
);
assert!(result.is_ok());
let report = result.unwrap();
let report = result.expect("expected a clean report");
assert!(report.contains("Detected type: server"));
assert!(report.contains("No issues found"));
}
@@ -432,41 +220,40 @@ port = 4530
callsign = "W1AW"
[remote]
url = "localhost:4530"
url = "192.168.1.10:4530"
[frontends.http]
enabled = true
port = 8080
"#,
);
assert!(result.is_ok());
let report = result.unwrap();
let report = result.expect("expected a clean report");
assert!(report.contains("Detected type: client"));
assert!(report.contains("No issues found"));
}
#[test]
fn test_valid_combined_config() {
let result = check_toml(
r#"
[trx-server.general]
callsign = "W1AW"
[trx-client.general]
callsign = "W1AW"
[trx-server.rig]
model = "ft817"
[trx-server.rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
[trx-client.remote]
url = "localhost:4530"
url = "127.0.0.1:4530"
"#,
);
assert!(result.is_ok());
let report = result.unwrap();
let report = result.expect("expected a clean report");
assert!(report.contains("Detected type: combined"));
}
#[test]
fn test_invalid_toml_syntax() {
let result = check_toml("this is not [valid toml");
assert!(result.is_err());
let result = check_toml("[general\ncallsign = \"W1AW\"\n");
assert!(result.unwrap_err().contains("TOML syntax error"));
}
@@ -474,19 +261,22 @@ url = "localhost:4530"
fn test_unknown_key_warning() {
let result = check_toml(
r#"
[general]
callsign = "W1AW"
[rig]
model = "ft817"
[rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
[bogus_section]
foo = "bar"
[listen]
prot = 4530
"#,
);
assert!(result.is_ok());
let report = result.unwrap();
assert!(report.contains("unknown key 'bogus_section'"));
let report = result.expect("unknown keys are warnings, not errors");
assert!(
report.contains("unknown config key 'listen.prot' (did you mean 'listen.port'?)"),
"unexpected report: {report}"
);
}
#[test]
@@ -498,11 +288,13 @@ log_level = "verbose"
[rig]
model = "ft817"
[rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
"#,
);
assert!(result.is_err());
let report = result.unwrap_err();
assert!(report.contains("log_level 'verbose' is invalid"));
assert!(result.unwrap_err().contains("log_level"));
}
#[test]
@@ -510,15 +302,17 @@ model = "ft817"
let result = check_toml(
r#"
[general]
latitude = 45.0
latitude = 52.0
[rig]
model = "ft817"
[rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
"#,
);
assert!(result.is_err());
let report = result.unwrap_err();
assert!(report.contains("latitude and longitude must be set together"));
assert!(result.unwrap_err().contains("longitude"));
}
#[test]
@@ -526,16 +320,18 @@ model = "ft817"
let result = check_toml(
r#"
[general]
latitude = 95.0
latitude = 120.0
longitude = 10.0
[rig]
model = "ft817"
[rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
"#,
);
assert!(result.is_err());
let report = result.unwrap_err();
assert!(report.contains("latitude 95 is out of range"));
assert!(result.unwrap_err().contains("latitude"));
}
#[test]
@@ -547,10 +343,31 @@ model = "ft817"
[rig.access]
type = "usb"
port = "/dev/ttyUSB0"
baud = 9600
"#,
);
assert!(result.is_err());
let report = result.unwrap_err();
assert!(report.contains("type 'usb' is invalid"));
assert!(result.unwrap_err().contains("access"));
}
/// The old checker only knew a fixed list of top-level keys and a few range
/// rules, so it passed configs the server rejects at startup.
#[test]
fn test_catches_errors_the_key_list_checker_missed() {
let result = check_toml(
r#"
[rig]
model = "ft817"
[rig.access]
type = "serial"
port = "/dev/ttyUSB0"
baud = 9600
[audio]
enabled = true
frame_duration_ms = 7
"#,
);
assert!(result.unwrap_err().contains("frame_duration_ms"));
}
}
+12 -4
View File
@@ -474,7 +474,9 @@ mod tests {
#[test]
fn test_generated_server_config_loads_and_validates() {
let file = write_temp(&build_default(ConfigType::Server));
let cfg = ServerConfig::load_from_file(file.path()).expect("generated config must load");
let cfg = ServerConfig::load_from_file(file.path())
.expect("generated config must load")
.config;
cfg.validate().expect("generated config must validate");
assert_eq!(cfg.rig.model.as_deref(), Some("ft817"));
assert_eq!(cfg.listen.port, 4530);
@@ -483,7 +485,9 @@ mod tests {
#[test]
fn test_generated_client_config_loads_and_validates() {
let file = write_temp(&build_default(ConfigType::Client));
let cfg = ClientConfig::load_from_file(file.path()).expect("generated config must load");
let cfg = ClientConfig::load_from_file(file.path())
.expect("generated config must load")
.config;
cfg.validate().expect("generated config must validate");
assert_eq!(cfg.remote.url.as_deref(), Some("localhost:4530"));
}
@@ -491,9 +495,13 @@ mod tests {
#[test]
fn test_generated_combined_config_loads_both_sections() {
let file = write_temp(&build_default(ConfigType::Combined));
let server = ServerConfig::load_from_file(file.path()).expect("server section must load");
let server = ServerConfig::load_from_file(file.path())
.expect("server section must load")
.config;
server.validate().expect("server section must validate");
let client = ClientConfig::load_from_file(file.path()).expect("client section must load");
let client = ClientConfig::load_from_file(file.path())
.expect("client section must load")
.config;
client.validate().expect("client section must validate");
}
+16 -9
View File
@@ -56,6 +56,9 @@ struct Cli {
/// Print example configuration and exit
#[arg(long = "print-config")]
print_config: bool,
/// Treat unknown configuration keys as a fatal error
#[arg(long = "strict-config")]
strict_config: bool,
/// Rig backend to use (e.g. ft817, ft450d)
#[arg(short = 'r', long = "rig")]
rig: Option<String>,
@@ -894,12 +897,22 @@ async fn main() -> DynResult<()> {
return Ok(());
}
let (cfg, config_path) = if let Some(ref path) = cli.config {
let cfg = ServerConfig::load_from_file(path)?;
(cfg, Some(path.clone()))
let loaded = if let Some(ref path) = cli.config {
ServerConfig::load_from_file(path)?
} else {
ServerConfig::load_from_default_paths()?
};
let config_path = loaded.path.clone();
// Logging comes up before any config complaint so the warnings are visible.
init_logging(loaded.config.general.log_level.as_deref());
if let Some(ref path) = config_path {
info!("Loaded configuration from {}", path.display());
}
loaded.report_unknown_keys(cli.strict_config)?;
let cfg = loaded.config;
cfg.validate()
.map_err(|e| format!("Invalid server configuration: {}", e))?;
@@ -912,12 +925,6 @@ async fn main() -> DynResult<()> {
std::process::exit(1);
}
init_logging(cfg.general.log_level.as_deref());
if let Some(ref path) = config_path {
info!("Loaded configuration from {}", path.display());
}
let registry = Arc::new(bootstrap_ctx);
// --- Resolve the effective rig list ---