Re: [pve-devel] [PATCH pve-installer] answer: perform basic input validation for keyboard

[Stoiko Ivanov <s.ivanov@proxmox.com>]

On Wed, 24 Apr 2024 10:48:50 +0200
Christian Ebner <c.ebner@proxmox.com> wrote:

> Currently it is possible to validate and create an iso with an
> invalid keyboad layout, only failing later during installation.
> 
> Add a basic check for correct keyboard layout by defining an enum
> with allowed variants.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  proxmox-auto-installer/src/answer.rs | 39 +++++++++++++++++++++++++++-
>  proxmox-auto-installer/src/utils.rs  |  8 ++++--
>  2 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/proxmox-auto-installer/src/answer.rs b/proxmox-auto-installer/src/answer.rs
> index a6cf8b7..af7485a 100644
> --- a/proxmox-auto-installer/src/answer.rs
> +++ b/proxmox-auto-installer/src/answer.rs
> @@ -23,7 +23,7 @@ pub struct Answer {
>  pub struct Global {
>      pub country: String,
>      pub fqdn: Fqdn,
> -    pub keyboard: String,
> +    pub keyboard: KeyboardLayout,
>      pub mailto: String,
>      pub timezone: String,
>      pub root_password: String,
> @@ -270,3 +270,40 @@ pub struct BtrfsOptions {
>      pub hdsize: Option<f64>,
>      pub raid: Option<BtrfsRaidLevel>,
>  }
> +
> +#[derive(Clone, Deserialize, Serialize, Debug, PartialEq)]
> +#[serde(rename_all = "kebab-case", deny_unknown_fields)]
> +pub enum KeyboardLayout {
> +    De,
> +    DeCh,
> +    Dk,
> +    EnGb,
> +    EnUs,
> +    Es,
> +    Fi,
> +    Fr,
> +    FrBe,
> +    FrCa,
> +    FrCh,
> +    Hu,
> +    Is,
> +    It,
> +    Jp,
> +    Lt,
> +    Mk,
> +    Nl,
> +    No,
> +    Pl,
> +    Pt,
> +    PtBr,
> +    Se,
> +    Si,
> +    Tr,
> +}
> +
quickly looked at that as well yesterday - and I also ran into the issue
that we get the relevant data in the installer itself (where actual
validation takes place).

with the target to have the auto-install-assistant available as single
static binary - I think your approach works well enough - the one thing
that might be an improvment is to get the data from country.dat (a
build-artefact output from country.pl based on
/usr/share/iso-codes/json/iso_3166-1.json ) at build-time and embed it in the binary.
(but I did not get around to checking how this is done sensibly in rust)

additionally we could verify the country selection as well with that.


> +impl std::fmt::Display for KeyboardLayout {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
> +        let keyboard_layout = serde_json::to_value(self).unwrap().to_string();
> +        write!(f, "{}", keyboard_layout.trim_matches('\"'))
> +    }
> +}
> diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
> index 7e1366c..202ad41 100644
> --- a/proxmox-auto-installer/src/utils.rs
> +++ b/proxmox-auto-installer/src/utils.rs
> @@ -281,7 +281,11 @@ pub fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<(
>      {
>          bail!("country code '{}' is not valid", &answer.global.country);
>      }
> -    if !locales.kmap.keys().any(|i| i == &answer.global.keyboard) {
> +    if !locales
> +        .kmap
> +        .keys()
> +        .any(|i| i == &answer.global.keyboard.to_string())
> +    {
>          bail!("keyboard layout '{}' is not valid", &answer.global.keyboard);
>      }
>  
> @@ -328,7 +332,7 @@ pub fn parse_answer(
>  
>          country: answer.global.country.clone(),
>          timezone: answer.global.timezone.clone(),
> -        keymap: answer.global.keyboard.clone(),
> +        keymap: answer.global.keyboard.to_string(),
>  
>          password: answer.global.root_password.clone(),
>          mailto: answer.global.mailto.clone(),



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel