public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation
@ 2024-05-28  8:13 Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency Christoph Heiss
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

This uses the regex for <input type="email" /> elements as defined in
the HTML spec to validate emails in the installer. That regex should be
a lot more robust than our current on and cover basically all cases of
email addresses that might be encountered in the while.

Also, patch #6 adds validation for the auto-installer `global.mailto`
option too, while at it.

Patches #1 through #4 are just small cleanups/fixes and may be applied
independently.

[0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address

Christoph Heiss (7):
  cargo: convert `anyhow` to workspace dependency
  tui: fix new clippy lint
  auto-installer: drop some unneeded `pub` modifiers
  auto-installer: print full anyhow message on failure
  tui: use email regex from HTML specification for validation
  auto-installer: validate `global.mailto` answer option
  proxinstall: use email regex from HTML specification for validation

 Cargo.toml                                    |  2 ++
 Proxmox/Makefile                              |  1 +
 Proxmox/Sys.pm                                |  9 ++++++
 proxinstall                                   |  3 +-
 proxmox-auto-install-assistant/Cargo.toml     |  2 +-
 proxmox-auto-installer/Cargo.toml             |  2 +-
 .../src/bin/proxmox-auto-installer.rs         |  2 +-
 proxmox-auto-installer/src/utils.rs           | 17 ++++++-----
 proxmox-chroot/Cargo.toml                     |  2 +-
 proxmox-fetch-answer/Cargo.toml               |  2 +-
 proxmox-installer-common/Cargo.toml           |  1 +
 proxmox-installer-common/src/options.rs       | 29 ++++++++++++++++++-
 proxmox-tui-installer/Cargo.toml              |  1 -
 proxmox-tui-installer/src/main.rs             | 22 +++++---------
 14 files changed, 65 insertions(+), 30 deletions(-)
 create mode 100644 Proxmox/Sys.pm

-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 2/7] tui: fix new clippy lint Christoph Heiss
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Cargo.toml                                | 2 ++
 proxmox-auto-install-assistant/Cargo.toml | 2 +-
 proxmox-auto-installer/Cargo.toml         | 2 +-
 proxmox-chroot/Cargo.toml                 | 2 +-
 proxmox-fetch-answer/Cargo.toml           | 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 1e730ce..ec1deaf 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -9,3 +9,5 @@ members = [
     "proxmox-tui-installer",
 ]
 
+[workspace.dependencies]
+anyhow = "1.0"
diff --git a/proxmox-auto-install-assistant/Cargo.toml b/proxmox-auto-install-assistant/Cargo.toml
index eaca7f8..b30a157 100644
--- a/proxmox-auto-install-assistant/Cargo.toml
+++ b/proxmox-auto-install-assistant/Cargo.toml
@@ -11,7 +11,7 @@ exclude = [ "build", "debian" ]
 homepage = "https://www.proxmox.com"
 
 [dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
 clap = { version = "4.0", features = ["derive"] }
 glob = "0.3"
 log = "0.4.20"
diff --git a/proxmox-auto-installer/Cargo.toml b/proxmox-auto-installer/Cargo.toml
index 4f54439..efb0fe3 100644
--- a/proxmox-auto-installer/Cargo.toml
+++ b/proxmox-auto-installer/Cargo.toml
@@ -11,7 +11,7 @@ exclude = [ "build", "debian" ]
 homepage = "https://www.proxmox.com"
 
 [dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
 clap = { version = "4.0", features = ["derive"] }
 glob = "0.3"
 log = "0.4.20"
diff --git a/proxmox-chroot/Cargo.toml b/proxmox-chroot/Cargo.toml
index 43b96ff..6247baa 100644
--- a/proxmox-chroot/Cargo.toml
+++ b/proxmox-chroot/Cargo.toml
@@ -8,7 +8,7 @@ exclude = [ "build", "debian" ]
 homepage = "https://www.proxmox.com"
 
 [dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
 clap = { version = "4.0", features = ["derive"] }
 nix = "0.26.1"
 proxmox-installer-common = { path = "../proxmox-installer-common" }
diff --git a/proxmox-fetch-answer/Cargo.toml b/proxmox-fetch-answer/Cargo.toml
index 964682a..881979b 100644
--- a/proxmox-fetch-answer/Cargo.toml
+++ b/proxmox-fetch-answer/Cargo.toml
@@ -13,7 +13,7 @@ homepage = "https://www.proxmox.com"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-anyhow = "1.0"
+anyhow.workspace = true
 hex = "0.4"
 log = "0.4.20"
 native-tls = "0.2"
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 2/7] tui: fix new clippy lint
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 3/7] auto-installer: drop some unneeded `pub` modifiers Christoph Heiss
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

warning: unnecessary hashes around raw string literal
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_string_hashes
   = note: `#[warn(clippy::needless_raw_string_hashes)]` on by default

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-tui-installer/src/main.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 4fb7afd..a24fb0b 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -35,12 +35,12 @@ use views::{
 };
 
 // TextView::center() seems to garble the first two lines, so fix it manually here.
-const PROXMOX_LOGO: &str = r#"
+const PROXMOX_LOGO: &str = r"
  ____
 |  _ \ _ __ _____  ___ __ ___   _____  __
 | |_) | '__/ _ \ \/ / '_ ` _ \ / _ \ \/ /
 |  __/| | | (_) >  <| | | | | | (_) >  <
-|_|   |_|  \___/_/\_\_| |_| |_|\___/_/\_\ "#;
+|_|   |_|  \___/_/\_\_| |_| |_|\___/_/\_\ ";
 
 struct InstallerView {
     view: ResizedView<Dialog>,
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 3/7] auto-installer: drop some unneeded `pub` modifiers
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 2/7] tui: fix new clippy lint Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 4/7] auto-installer: print full anyhow message on failure Christoph Heiss
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-installer/src/utils.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 202ad41..f752924 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -14,7 +14,7 @@ use proxmox_installer_common::{
 };
 use serde::{Deserialize, Serialize};
 
-pub fn get_network_settings(
+fn get_network_settings(
     answer: &Answer,
     udev_info: &UdevInfo,
     runtime_info: &RuntimeInfo,
@@ -143,7 +143,7 @@ pub fn get_matched_udev_indexes(
     Ok(matches)
 }
 
-pub fn set_disks(
+fn set_disks(
     answer: &Answer,
     udev_info: &UdevInfo,
     runtime_info: &RuntimeInfo,
@@ -261,7 +261,7 @@ fn set_selected_disks(
     Ok(())
 }
 
-pub fn get_first_selected_disk(config: &InstallConfig) -> usize {
+fn get_first_selected_disk(config: &InstallConfig) -> usize {
     config
         .disk_selection
         .iter()
@@ -272,7 +272,7 @@ pub fn get_first_selected_disk(config: &InstallConfig) -> usize {
         .expect("could not parse key to usize")
 }
 
-pub fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<()> {
+fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<()> {
     info!("Verifying locale settings");
     if !locales
         .countries
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 4/7] auto-installer: print full anyhow message on failure
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (2 preceding siblings ...)
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 3/7] auto-installer: drop some unneeded `pub` modifiers Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 5/7] tui: use email regex from HTML specification for validation Christoph Heiss
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

Otherwise, context info is simply lost -- which might contain valuable
information for the user.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-installer/src/bin/proxmox-auto-installer.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index 9fcec1e..ef42dc6 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -86,7 +86,7 @@ fn main() -> ExitCode {
     match run_installation(&answer, &locales, &runtime_info, &udevadm_info, &setup_info) {
         Ok(_) => info!("Installation done."),
         Err(err) => {
-            error!("Installation failed: {err}");
+            error!("Installation failed: {err:#}");
             return exit_failure(answer.global.reboot_on_error);
         }
     }
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 5/7] tui: use email regex from HTML specification for validation
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (3 preceding siblings ...)
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 4/7] auto-installer: print full anyhow message on failure Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 6/7] auto-installer: validate `global.mailto` answer option Christoph Heiss
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

That regex should be a lot more accurate in what it allows - if it's
good enough for the HTML spec, it should be for us too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-installer-common/Cargo.toml     |  1 +
 proxmox-installer-common/src/options.rs | 29 ++++++++++++++++++++++++-
 proxmox-tui-installer/Cargo.toml        |  1 -
 proxmox-tui-installer/src/main.rs       | 18 +++++----------
 4 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/proxmox-installer-common/Cargo.toml b/proxmox-installer-common/Cargo.toml
index 70f828a..e151b0e 100644
--- a/proxmox-installer-common/Cargo.toml
+++ b/proxmox-installer-common/Cargo.toml
@@ -8,6 +8,7 @@ exclude = [ "build", "debian" ]
 homepage = "https://www.proxmox.com"
 
 [dependencies]
+anyhow.workspace = true
 regex = "1.7"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index e77914b..1962f87 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -1,5 +1,8 @@
+use anyhow::{bail, Result};
+use regex::Regex;
 use serde::Deserialize;
 use std::net::{IpAddr, Ipv4Addr};
+use std::sync::OnceLock;
 use std::{cmp, fmt};
 
 use crate::setup::{
@@ -327,6 +330,8 @@ impl TimezoneOptions {
     }
 }
 
+const EMAIL_DEFAULT_PLACEHOLDER: &str = "mail@example.invalid";
+
 #[derive(Clone, Debug)]
 pub struct PasswordOptions {
     pub email: String,
@@ -336,7 +341,7 @@ pub struct PasswordOptions {
 impl Default for PasswordOptions {
     fn default() -> Self {
         Self {
-            email: "mail@example.invalid".to_string(),
+            email: EMAIL_DEFAULT_PLACEHOLDER.to_owned(),
             root_password: String::new(),
         }
     }
@@ -418,6 +423,28 @@ impl NetworkOptions {
     }
 }
 
+/// Validates an email address using the regex for <input type="email" /> elements
+/// as defined in the HTML specification [0].
+/// Using that /should/ cover all possible cases that are encountered in the wild.
+///
+/// It additionally checks whether the email our default email placeholder value.
+///
+/// [0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
+pub fn email_validate(email: &str) -> Result<()> {
+    static RE: OnceLock<Regex> = OnceLock::new();
+    let re = RE.get_or_init(|| {
+        Regex::new(r"^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$").unwrap()
+    });
+
+    if !re.is_match(email) {
+        bail!("Email does not look like a valid address (user@domain.tld)")
+    } else if email == EMAIL_DEFAULT_PLACEHOLDER {
+        bail!("Invalid (default) email address")
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/proxmox-tui-installer/Cargo.toml b/proxmox-tui-installer/Cargo.toml
index fc653f0..d3087d8 100644
--- a/proxmox-tui-installer/Cargo.toml
+++ b/proxmox-tui-installer/Cargo.toml
@@ -11,5 +11,4 @@ homepage = "https://www.proxmox.com"
 cursive = { version = "0.20.0", default-features = false, features = ["termion-backend"] }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
-regex = "1.7"
 proxmox-installer-common = { path = "../proxmox-installer-common" }
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index a24fb0b..67dd479 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -13,13 +13,11 @@ use cursive::{
     Cursive, CursiveRunnable, ScreenId, View, XY,
 };
 
-use regex::Regex;
-
 mod options;
 use options::InstallerOptions;
 
 use proxmox_installer_common::{
-    options::{BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions},
+    options::{email_validate, BootdiskOptions, NetworkOptions, PasswordOptions, TimezoneOptions},
     setup::{installer_setup, LocaleInfo, ProxmoxProduct, RuntimeInfo, SetupInfo},
     utils::Fqdn,
 };
@@ -448,18 +446,12 @@ fn password_dialog(siv: &mut Cursive) -> InstallerView {
                     .get_value::<EditView, _>(2)
                     .ok_or("failed to retrieve email")?;
 
-                let email_regex =
-                    Regex::new(r"^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$")
-                        .unwrap();
-
                 if root_password.len() < 5 {
-                    Err("password too short, must be at least 5 characters long")
+                    Err("password too short, must be at least 5 characters long".to_owned())
                 } else if root_password != confirm_password {
-                    Err("passwords do not match")
-                } else if email == "mail@example.invalid" {
-                    Err("invalid email address")
-                } else if !email_regex.is_match(&email) {
-                    Err("Email does not look like a valid address (user@domain.tld)")
+                    Err("passwords do not match".to_owned())
+                } else if let Err(err) = email_validate(&email) {
+                    Err(err.to_string())
                 } else {
                     Ok(PasswordOptions {
                         root_password,
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 6/7] auto-installer: validate `global.mailto` answer option
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (4 preceding siblings ...)
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 5/7] tui: use email regex from HTML specification for validation Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 7/7] proxinstall: use email regex from HTML specification for validation Christoph Heiss
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 proxmox-auto-installer/src/utils.rs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index f752924..7241eec 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, Context as _, Result};
+use anyhow::{bail, Context, Result};
 use clap::ValueEnum;
 use glob::Pattern;
 use log::info;
@@ -9,7 +9,7 @@ use crate::{
     udevinfo::UdevInfo,
 };
 use proxmox_installer_common::{
-    options::{FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption},
+    options::{email_validate, FsType, NetworkOptions, ZfsChecksumOption, ZfsCompressOption},
     setup::{InstallConfig, InstallZfsOption, LocaleInfo, RuntimeInfo, SetupInfo},
 };
 use serde::{Deserialize, Serialize};
@@ -272,7 +272,10 @@ fn get_first_selected_disk(config: &InstallConfig) -> usize {
         .expect("could not parse key to usize")
 }
 
-fn verify_locale_settings(answer: &Answer, locales: &LocaleInfo) -> Result<()> {
+fn verify_global_settings(answer: &Answer, locales: &LocaleInfo) -> Result<()> {
+    info!("Verifying global settings");
+    email_validate(&answer.global.mailto).with_context(|| answer.global.mailto.clone())?;
+
     info!("Verifying locale settings");
     if !locales
         .countries
@@ -315,7 +318,7 @@ pub fn parse_answer(
 
     let network_settings = get_network_settings(answer, udev_info, runtime_info, setup_info)?;
 
-    verify_locale_settings(answer, locales)?;
+    verify_global_settings(answer, locales)?;
 
     let mut config = InstallConfig {
         autoreboot: 1_usize,
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [pve-devel] [PATCH installer 7/7] proxinstall: use email regex from HTML specification for validation
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (5 preceding siblings ...)
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 6/7] auto-installer: validate `global.mailto` answer option Christoph Heiss
@ 2024-05-28  8:13 ` Christoph Heiss
  2024-06-03  8:20 ` [pve-devel] [PATCH installer 0/7] use email regex from HTML spec " Max Carrara
  2024-06-28  8:16 ` Christoph Heiss
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-05-28  8:13 UTC (permalink / raw)
  To: pve-devel

That regex should be a lot more accurate in what it allows - if it's
good enough for the HTML spec, it should be for us too.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
 Proxmox/Makefile | 1 +
 Proxmox/Sys.pm   | 9 +++++++++
 proxinstall      | 3 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)
 create mode 100644 Proxmox/Sys.pm

diff --git a/Proxmox/Makefile b/Proxmox/Makefile
index 9561d9b..6f9eea8 100644
--- a/Proxmox/Makefile
+++ b/Proxmox/Makefile
@@ -12,6 +12,7 @@ PERL_MODULES=\
     Install/RunEnv.pm \
     Install/StorageConfig.pm \
     Log.pm \
+    Sys.pm \
     Sys/Block.pm \
     Sys/Command.pm \
     Sys/File.pm \
diff --git a/Proxmox/Sys.pm b/Proxmox/Sys.pm
new file mode 100644
index 0000000..afc6780
--- /dev/null
+++ b/Proxmox/Sys.pm
@@ -0,0 +1,9 @@
+package Proxmox::Sys;
+
+use strict;
+use warnings;
+
+# The HTML specification actually gives a "blessed" regex for email addresses:
+# https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
+# Using that /should/ cover all possible cases that are encountered in the wild.
+our $EMAIL_RE = '^[a-zA-Z0-9.!#$%&\'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$';
diff --git a/proxinstall b/proxinstall
index a6a4cfb..79b0c76 100755
--- a/proxinstall
+++ b/proxinstall
@@ -33,6 +33,7 @@ my $iso_env = Proxmox::Install::ISOEnv::get();
 use Proxmox::Install;
 use Proxmox::Install::Config;
 
+use Proxmox::Sys;
 use Proxmox::Sys::Block qw(get_cached_disks);
 use Proxmox::Sys::Command qw(syscmd);
 use Proxmox::Sys::File qw(file_read_all file_write_all);
@@ -733,7 +734,7 @@ sub create_password_view {
 	}
 
 	my $t3 = $eme->get_text;
-	if ($t3 !~ m/^[\w\+\-\~]+(\.[\w\+\-\~]+)*@[a-zA-Z0-9\-]+(\.[a-zA-Z0-9\-]+)*$/) {
+	if ($t3 !~ m/$Proxmox::Sys::EMAIL_RE/) {
 	    Proxmox::UI::message("Email does not look like a valid address (user\@domain.tld)");
 	    $eme->grab_focus();
 	    return;
-- 
2.44.0



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (6 preceding siblings ...)
  2024-05-28  8:13 ` [pve-devel] [PATCH installer 7/7] proxinstall: use email regex from HTML specification for validation Christoph Heiss
@ 2024-06-03  8:20 ` Max Carrara
  2024-06-28  8:16 ` Christoph Heiss
  8 siblings, 0 replies; 10+ messages in thread
From: Max Carrara @ 2024-06-03  8:20 UTC (permalink / raw)
  To: Proxmox VE development discussion

On Tue May 28, 2024 at 10:13 AM CEST, Christoph Heiss wrote:
> This uses the regex for <input type="email" /> elements as defined in
> the HTML spec to validate emails in the installer. That regex should be
> a lot more robust than our current on and cover basically all cases of
> email addresses that might be encountered in the while.
>
> Also, patch #6 adds validation for the auto-installer `global.mailto`
> option too, while at it.
>
> Patches #1 through #4 are just small cleanups/fixes and may be applied
> independently.
>
> [0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address

The changes in this series are rather straightforward; there's nothing
extraordinary going on. LGTM.

Reviewed-by: Max Carrara <m.carrara@proxmox.com>

>
> Christoph Heiss (7):
>   cargo: convert `anyhow` to workspace dependency
>   tui: fix new clippy lint
>   auto-installer: drop some unneeded `pub` modifiers
>   auto-installer: print full anyhow message on failure
>   tui: use email regex from HTML specification for validation
>   auto-installer: validate `global.mailto` answer option
>   proxinstall: use email regex from HTML specification for validation
>
>  Cargo.toml                                    |  2 ++
>  Proxmox/Makefile                              |  1 +
>  Proxmox/Sys.pm                                |  9 ++++++
>  proxinstall                                   |  3 +-
>  proxmox-auto-install-assistant/Cargo.toml     |  2 +-
>  proxmox-auto-installer/Cargo.toml             |  2 +-
>  .../src/bin/proxmox-auto-installer.rs         |  2 +-
>  proxmox-auto-installer/src/utils.rs           | 17 ++++++-----
>  proxmox-chroot/Cargo.toml                     |  2 +-
>  proxmox-fetch-answer/Cargo.toml               |  2 +-
>  proxmox-installer-common/Cargo.toml           |  1 +
>  proxmox-installer-common/src/options.rs       | 29 ++++++++++++++++++-
>  proxmox-tui-installer/Cargo.toml              |  1 -
>  proxmox-tui-installer/src/main.rs             | 22 +++++---------
>  14 files changed, 65 insertions(+), 30 deletions(-)
>  create mode 100644 Proxmox/Sys.pm



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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation
  2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
                   ` (7 preceding siblings ...)
  2024-06-03  8:20 ` [pve-devel] [PATCH installer 0/7] use email regex from HTML spec " Max Carrara
@ 2024-06-28  8:16 ` Christoph Heiss
  8 siblings, 0 replies; 10+ messages in thread
From: Christoph Heiss @ 2024-06-28  8:16 UTC (permalink / raw)
  To: Proxmox VE development discussion

Ping, still applies.

On Tue, May 28, 2024 at 10:13:41AM +0200, Christoph Heiss wrote:
> This uses the regex for <input type="email" /> elements as defined in
> the HTML spec to validate emails in the installer. That regex should be
> a lot more robust than our current on and cover basically all cases of
> email addresses that might be encountered in the while.
>
> Also, patch #6 adds validation for the auto-installer `global.mailto`
> option too, while at it.
>
> Patches #1 through #4 are just small cleanups/fixes and may be applied
> independently.
>
> [0] https://html.spec.whatwg.org/multipage/input.html#valid-e-mail-address
>
> Christoph Heiss (7):
>   cargo: convert `anyhow` to workspace dependency
>   tui: fix new clippy lint
>   auto-installer: drop some unneeded `pub` modifiers
>   auto-installer: print full anyhow message on failure
>   tui: use email regex from HTML specification for validation
>   auto-installer: validate `global.mailto` answer option
>   proxinstall: use email regex from HTML specification for validation
>
>  Cargo.toml                                    |  2 ++
>  Proxmox/Makefile                              |  1 +
>  Proxmox/Sys.pm                                |  9 ++++++
>  proxinstall                                   |  3 +-
>  proxmox-auto-install-assistant/Cargo.toml     |  2 +-
>  proxmox-auto-installer/Cargo.toml             |  2 +-
>  .../src/bin/proxmox-auto-installer.rs         |  2 +-
>  proxmox-auto-installer/src/utils.rs           | 17 ++++++-----
>  proxmox-chroot/Cargo.toml                     |  2 +-
>  proxmox-fetch-answer/Cargo.toml               |  2 +-
>  proxmox-installer-common/Cargo.toml           |  1 +
>  proxmox-installer-common/src/options.rs       | 29 ++++++++++++++++++-
>  proxmox-tui-installer/Cargo.toml              |  1 -
>  proxmox-tui-installer/src/main.rs             | 22 +++++---------
>  14 files changed, 65 insertions(+), 30 deletions(-)
>  create mode 100644 Proxmox/Sys.pm
>
> --
> 2.44.0
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>


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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-06-28  8:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-28  8:13 [pve-devel] [PATCH installer 0/7] use email regex from HTML spec for validation Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 1/7] cargo: convert `anyhow` to workspace dependency Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 2/7] tui: fix new clippy lint Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 3/7] auto-installer: drop some unneeded `pub` modifiers Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 4/7] auto-installer: print full anyhow message on failure Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 5/7] tui: use email regex from HTML specification for validation Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 6/7] auto-installer: validate `global.mailto` answer option Christoph Heiss
2024-05-28  8:13 ` [pve-devel] [PATCH installer 7/7] proxinstall: use email regex from HTML specification for validation Christoph Heiss
2024-06-03  8:20 ` [pve-devel] [PATCH installer 0/7] use email regex from HTML spec " Max Carrara
2024-06-28  8:16 ` Christoph Heiss

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal