pve-firewall lowercases the names of aliases when reading from the configuration as well as when comparing source / destination entries with the entries in the parsed aliases. In order to stay backwards-compatible we also need to lowercase any parsed alias name. I decided to this in the constructor and switch all call sites to the new constructor, so there's only one place where we have to handle lowercasing the string.
Signed-off-by: Stefan Hanreich <[email protected]> --- Changes from v1 to v2: * fix typo and improve description in documentation of Alias * add bugzilla issue # to commit msg Also leaving the thoughts I posted under v1 here: Leo supplied a patch already [1] that changes the handling of names in the firewall. Might make sense to revisit this patch series in the future to unify how cases are handled in the firewall. Nevertheless, this patch acts as a quick fix for users running into issues with the firewall due to the difference in handling cases in pve-firewall and proxmox-firewall. [1] https://lists.proxmox.com/pipermail/pve-devel/2023-January/055596.html proxmox-ve-config/src/firewall/types/alias.rs | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/proxmox-ve-config/src/firewall/types/alias.rs b/proxmox-ve-config/src/firewall/types/alias.rs index 5dfaa41..553931e 100644 --- a/proxmox-ve-config/src/firewall/types/alias.rs +++ b/proxmox-ve-config/src/firewall/types/alias.rs @@ -53,10 +53,7 @@ impl FromStr for AliasName { fn from_str(s: &str) -> Result<Self, Self::Err> { match s.split_once('/') { - Some((prefix, name)) if !name.is_empty() => Ok(Self { - scope: prefix.parse()?, - name: name.to_string(), - }), + Some((prefix, name)) if !name.is_empty() => Ok(Self::new(prefix.parse()?, name)), _ => { bail!("Invalid Alias name!") } @@ -65,10 +62,18 @@ impl FromStr for AliasName { } impl AliasName { + /// Creates a new [`AliasName`]. + /// + /// It will convert any ASCII characters contained in the name into lowercase. This is for + /// maintaining backwards-compatiblity with pve-firewall, where all aliases are lowercased when + /// reading from the config. pub fn new(scope: AliasScope, name: impl Into<String>) -> Self { + let mut lowercase_name = name.into(); + lowercase_name.make_ascii_lowercase(); + Self { scope, - name: name.into(), + name: lowercase_name, } } @@ -90,13 +95,21 @@ pub struct Alias { } impl Alias { + /// Creates a new [`Alias`]. + /// + /// It will convert any ASCII characters contained in the name into lowercase. This is for + /// maintaining backwards-compatiblity with pve-firewall, where all aliases are lowercased when + /// reading from the config. pub fn new( name: impl Into<String>, address: impl Into<Cidr>, comment: impl Into<Option<String>>, ) -> Self { + let mut lowercase_name = name.into(); + lowercase_name.make_ascii_lowercase(); + Self { - name: name.into(), + name: lowercase_name, address: address.into(), comment: comment.into(), } @@ -135,11 +148,7 @@ impl FromStr for Alias { None => None, }; - Ok(Alias { - name: name.to_string(), - address, - comment, - }) + Ok(Alias::new(name, address, comment)) } } @@ -159,6 +168,16 @@ mod tests { for alias in ["-- 10.0.0.1/32", "0asd 10.0.0.1/32", "__test 10.0.0.0/32"] { alias.parse::<Alias>().expect_err("invalid alias"); } + + let alias = "pRoxMox 10.0.0.0/32 # a comment" + .parse::<Alias>() + .expect("valid alias"); + assert_eq!(alias.name(), "proxmox"); + assert_eq!( + alias.address(), + &Cidr::new_v4([10, 0, 0, 0], 32).expect("valid CIDR") + ); + assert_eq!(alias.comment(), Some("a comment")); } #[test] @@ -171,4 +190,12 @@ mod tests { name.parse::<AliasName>().expect_err("invalid alias name"); } } + + #[test] + fn test_parse_alias_case() { + for name in ["dc/PROxMoX", "guest/PROXMOX"] { + let alias_name = name.parse::<AliasName>().expect("valid alias name"); + assert_eq!(alias_name.name(), "proxmox"); + } + } } -- 2.39.5 _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
