From 3ecbd85d5da373e2b99c6341ba15f0dd4379c8ee Mon Sep 17 00:00:00 2001 From: ABuljko Date: Tue, 11 Aug 2026 12:03:05 +0200 Subject: [PATCH 1/2] Add support for string config fields - Add FieldType::String and ConfigValueMut::String, backed by a StringView so any String can be used - Put both after U8 so the existing type ids stay the same - Take the max length from the config struct; too long gives DataTooLong - Check the length before clear() so a rejected value keeps the old one - Reject control characters, line and paragraph separators, and bidirectional embeddings, overrides and isolates with InvalidValue, since the value is printed by host tools; the directional marks are still allowed - Enable the serde feature of heapless, needed for StringView's Serialize impl --- Cargo.toml | 2 +- src/config.rs | 180 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 180 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 736c2b8..d3d5f69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,7 @@ apdu-app = "0.2" cbor-smol = { version = "0.5.0", features = ["heapless-v0-9", "heapless-bytes-v0-5"] } ctaphid-app = "0.2" delog = "0.1" -heapless = "0.9" +heapless = { version = "0.9", features = ["serde"] } heapless-bytes = { version = "0.5", features = ["heapless-0.9"] } iso7816 = "0.2" littlefs2 = { version = "0.8", optional = true } diff --git a/src/config.rs b/src/config.rs index eafa330..8b38f23 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,7 +5,7 @@ use core::{ }; use cbor_smol::{cbor_deserialize, cbor_serialize_to}; -use heapless::VecView; +use heapless::{string::StringView, VecView}; use littlefs2_core::{path, Path}; use serde::{de::DeserializeOwned, Serialize}; use strum_macros::FromRepr; @@ -175,11 +175,17 @@ pub trait Config: Default + PartialEq + DeserializeOwned + Serialize { } // No need to rename, cbor-smol already packs enum using ids +// +// As the variants are serialized as their index, new variants may only be appended #[derive(Serialize)] #[non_exhaustive] pub enum FieldType { Bool, U8, + /// A UTF-8 string + /// + /// The maximum length is defined by the config struct holding the value, not by this type + String, } #[derive(Serialize)] @@ -226,6 +232,21 @@ impl Config for () { pub enum ConfigValueMut<'a> { Bool(&'a mut bool), U8(&'a mut u8), + /// A string of any capacity, obtained from a `heapless::String` with `as_mut_view` + String(&'a mut StringView), +} + +// The line and paragraph separators and the bidi scope characters are not in the `Cc` category, +// so `char::is_control` does not cover them. A separator is rendered as a line break and an +// unterminated bidi override changes the rendering of the text that follows it, so a stored +// value could split up or reorder the line a host prints. The directional marks (U+061C, +// U+200E, U+200F) only affect a single position and are still allowed +fn is_forbidden(c: char) -> bool { + c.is_control() + || matches!( + c, + '\u{2028}' | '\u{2029}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}' + ) } impl<'a> ConfigValueMut<'a> { @@ -238,6 +259,20 @@ impl<'a> ConfigValueMut<'a> { match self { Self::Bool(r) => set_value(*r, value), Self::U8(r) => set_value(*r, value), + Self::String(r) => { + // The value is echoed back by GetConfig and printed by host tools, so escape + // sequences must not be storable + if value.chars().any(is_forbidden) { + return Err(ConfigError::InvalidValue); + } + // Check the capacity before clearing so that a rejected value leaves the stored + // one intact + if value.len() > r.capacity() { + return Err(ConfigError::DataTooLong); + } + r.clear(); + r.push_str(value).map_err(|_| ConfigError::DataTooLong) + } } } } @@ -247,6 +282,7 @@ impl<'a> Display for ConfigValueMut<'a> { match self { Self::Bool(value) => write!(f, "{value}"), Self::U8(value) => write!(f, "{value}"), + Self::String(value) => f.write_str(value), } } } @@ -380,4 +416,146 @@ mod tests { &hex!("81A5616E69746573745F6E616D656163F56172F46164F5617400") ); } + + // The field types are parsed as integers by the hosts, so their values may never change + #[test] + fn field_type_ids() { + for (ty, id) in [ + (FieldType::Bool, hex!("00").as_slice()), + (FieldType::U8, hex!("01").as_slice()), + (FieldType::String, hex!("02").as_slice()), + ] { + let mut bytes: heapless::Vec = Default::default(); + cbor_smol::cbor_serialize_to(&ty, &mut bytes).unwrap(); + assert_eq!(bytes.as_slice(), id); + } + } + + #[derive(Default, PartialEq, serde::Deserialize, serde::Serialize)] + struct TestConfig { + label: heapless::String<8>, + } + + impl Config for TestConfig { + fn field(&mut self, key: &str) -> Option> { + match key { + "label" => Some(ConfigValueMut::String(self.label.as_mut_view())), + _ => None, + } + } + + fn migration_version(&self) -> Option { + None + } + + fn set_migration_version(&mut self, _version: u32) -> bool { + false + } + + fn list_available_fields(&self) -> &'static [ConfigField] { + &[] + } + } + + fn get_field(config: &mut TestConfig, key: &str) -> Result, ConfigError> { + let mut response: heapless::Vec = Default::default(); + get(config, key, response.as_mut_view())?; + Ok(core::str::from_utf8(&response).unwrap().try_into().unwrap()) + } + + #[test] + fn string_field() { + let mut config = TestConfig::default(); + assert_eq!(get_field(&mut config, "label").unwrap(), ""); + + set(&mut config, "label", "Backup").unwrap(); + assert_eq!(config.label, "Backup"); + assert_eq!(get_field(&mut config, "label").unwrap(), "Backup"); + + set(&mut config, "label", "12345678").unwrap(); + assert_eq!(config.label, "12345678"); + set(&mut config, "label", "").unwrap(); + assert_eq!(config.label, ""); + } + + #[test] + fn string_field_too_long() { + let mut config = TestConfig::default(); + set(&mut config, "label", "old").unwrap(); + + let error = set(&mut config, "label", "123456789").unwrap_err(); + assert!(matches!(error, ConfigError::DataTooLong), "{error:?}"); + // A rejected value must not destroy the stored one + assert_eq!(config.label, "old"); + } + + #[test] + fn string_field_control_characters() { + let mut config = TestConfig::default(); + set(&mut config, "label", "old").unwrap(); + + for value in [ + "a\nb", "a\tb", "a\rb", "\x1b[2J", "\0", "\x7f", // C0 and DEL + "\u{85}", "\u{9b}", // C1, including the single-character CSI + ] { + let error = set(&mut config, "label", value).unwrap_err(); + assert!( + matches!(error, ConfigError::InvalidValue), + "{value:?}: {error:?}" + ); + assert_eq!(config.label, "old"); + } + } + + // Separators and bidi scope characters are not in the `Cc` category, so they need their + // own check + #[test] + fn string_field_bidi() { + let mut config = TestConfig::default(); + set(&mut config, "label", "old").unwrap(); + + for value in [ + "\u{202a}", + "\u{202b}", + "\u{202c}", + "\u{202d}", + "\u{202e}", // embedding/override + "\u{2066}", + "\u{2067}", + "\u{2068}", + "\u{2069}", // isolates + "a\u{202e}b", // embedded mid-value + "\u{2028}", + "\u{2029}", // line and paragraph separator + "a\u{2028}b", // embedded mid-value + ] { + let error = set(&mut config, "label", value).unwrap_err(); + assert!( + matches!(error, ConfigError::InvalidValue), + "{value:?}: {error:?}" + ); + assert_eq!(config.label, "old"); + } + + // Directional marks and ordinary right-to-left text stay usable + for value in ["\u{200e}", "\u{200f}", "\u{61c}", "עבר"] { + set(&mut config, "label", value).unwrap_or_else(|e| panic!("{value:?}: {e:?}")); + assert_eq!(config.label, value); + } + } + + // Values are bounded by their length in bytes, not in characters + #[test] + fn string_field_multibyte() { + let mut config = TestConfig::default(); + + // The capacity is 8 bytes: two 4-byte characters fit, three 3-byte ones do not + set(&mut config, "label", "🔑🔑").unwrap(); + assert_eq!(config.label, "🔑🔑"); + assert_eq!(config.label.len(), 8); + + let error = set(&mut config, "label", "中中中").unwrap_err(); + assert!(matches!(error, ConfigError::DataTooLong), "{error:?}"); + assert_eq!(config.label, "🔑🔑"); + } } From ea446cce23bf46186e47b3a54be60b7f756bf090 Mon Sep 17 00:00:00 2001 From: ABuljko Date: Tue, 11 Aug 2026 19:10:43 +0200 Subject: [PATCH 2/2] Accept arbitrary UTF-8 in string config fields --- src/config.rs | 63 +++------------------------------------------------ 1 file changed, 3 insertions(+), 60 deletions(-) diff --git a/src/config.rs b/src/config.rs index 8b38f23..de0744f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -241,14 +241,6 @@ pub enum ConfigValueMut<'a> { // unterminated bidi override changes the rendering of the text that follows it, so a stored // value could split up or reorder the line a host prints. The directional marks (U+061C, // U+200E, U+200F) only affect a single position and are still allowed -fn is_forbidden(c: char) -> bool { - c.is_control() - || matches!( - c, - '\u{2028}' | '\u{2029}' | '\u{202a}'..='\u{202e}' | '\u{2066}'..='\u{2069}' - ) -} - impl<'a> ConfigValueMut<'a> { fn set(&mut self, value: &str) -> Result<(), ConfigError> { fn set_value(target: &mut T, s: &str) -> Result<(), ConfigError> { @@ -260,11 +252,6 @@ impl<'a> ConfigValueMut<'a> { Self::Bool(r) => set_value(*r, value), Self::U8(r) => set_value(*r, value), Self::String(r) => { - // The value is echoed back by GetConfig and printed by host tools, so escape - // sequences must not be storable - if value.chars().any(is_forbidden) { - return Err(ConfigError::InvalidValue); - } // Check the capacity before clearing so that a rejected value leaves the stored // one intact if value.len() > r.capacity() { @@ -489,56 +476,12 @@ mod tests { assert_eq!(config.label, "old"); } + // The firmware stores arbitrary UTF-8, clients sanitize the value before displaying it #[test] - fn string_field_control_characters() { + fn string_field_arbitrary_utf8() { let mut config = TestConfig::default(); - set(&mut config, "label", "old").unwrap(); - - for value in [ - "a\nb", "a\tb", "a\rb", "\x1b[2J", "\0", "\x7f", // C0 and DEL - "\u{85}", "\u{9b}", // C1, including the single-character CSI - ] { - let error = set(&mut config, "label", value).unwrap_err(); - assert!( - matches!(error, ConfigError::InvalidValue), - "{value:?}: {error:?}" - ); - assert_eq!(config.label, "old"); - } - } - - // Separators and bidi scope characters are not in the `Cc` category, so they need their - // own check - #[test] - fn string_field_bidi() { - let mut config = TestConfig::default(); - set(&mut config, "label", "old").unwrap(); - - for value in [ - "\u{202a}", - "\u{202b}", - "\u{202c}", - "\u{202d}", - "\u{202e}", // embedding/override - "\u{2066}", - "\u{2067}", - "\u{2068}", - "\u{2069}", // isolates - "a\u{202e}b", // embedded mid-value - "\u{2028}", - "\u{2029}", // line and paragraph separator - "a\u{2028}b", // embedded mid-value - ] { - let error = set(&mut config, "label", value).unwrap_err(); - assert!( - matches!(error, ConfigError::InvalidValue), - "{value:?}: {error:?}" - ); - assert_eq!(config.label, "old"); - } - // Directional marks and ordinary right-to-left text stay usable - for value in ["\u{200e}", "\u{200f}", "\u{61c}", "עבר"] { + for value in ["a\nb", "\x1b[2J", "\u{202e}", "\u{2028}", "עבר"] { set(&mut config, "label", value).unwrap_or_else(|e| panic!("{value:?}: {e:?}")); assert_eq!(config.label, value); }