From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <pve-devel-bounces@lists.proxmox.com> Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 2FDD71FF172 for <inbox@lore.proxmox.com>; Tue, 1 Apr 2025 16:03:03 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A7F24344CF; Tue, 1 Apr 2025 16:02:51 +0200 (CEST) Message-ID: <31921890-830e-446d-8ba9-1b473fae8fd8@proxmox.com> Date: Tue, 1 Apr 2025 16:02:17 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Christoph Heiss <c.heiss@proxmox.com> References: <20250401132106.309957-1-s.hanreich@proxmox.com> <20250401132106.309957-2-s.hanreich@proxmox.com> <D8VCQAP1N3UA.2K8R8DPL9BA28@proxmox.com> Content-Language: en-US From: Stefan Hanreich <s.hanreich@proxmox.com> In-Reply-To: <D8VCQAP1N3UA.2K8R8DPL9BA28@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.673 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, rust-lang.org, utils.rs, hostname.rs] Subject: Re: [pve-devel] [PATCH proxmox 2/2] network-types: add hostname type X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com> Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com> On 4/1/25 15:54, Christoph Heiss wrote: > On Tue Apr 1, 2025 at 3:21 PM CEST, Stefan Hanreich wrote: >> Add a type for representing Linux hostnames. These are the same >> constraints as the installer enforces [1]. > > Actually, the regex is not *fully* complete, see parse_fqdn() in the > same file in [1]. > > [..] >> [1] https://git.proxmox.com/?p=pve-installer.git;a=blob;f=Proxmox/Sys/Net.pm;h=81cb15f0042b195461324fffeca53d732133629e;hb=HEAD#l11 >> > [..] >> diff --git a/proxmox-network-types/src/hostname.rs b/proxmox-network-types/src/hostname.rs >> new file mode 100644 >> index 00000000..f183aecb >> --- /dev/null >> +++ b/proxmox-network-types/src/hostname.rs > [..] >> +/// Hostname of a Linux machine >> +/// >> +/// A hostname is at most 63 characters long and must only contain lowercase alphanumeric >> +/// characters as well as hyphens. It must not start or end with a hyphen. > > This should probably also reject purely numeric hostnames, as mentioned > in Bugzilla #1054 [0]. This isn't a requirement per any RFC, but seems > to can cause subtle bugs. It's enforced in the installer too. > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=1054 > >> +#[derive(Debug, Deserialize, Serialize, Clone, Eq, Hash, PartialOrd, Ord, PartialEq)] >> +pub struct Hostname(String); >> + > [..] >> + >> +impl Hostname { >> + /// Constructs a new hostname from a string >> + /// >> + /// This function accepts characters in any case, but the resulting hostname will be >> + /// lowercased. >> + pub fn new(name: impl AsRef<str>) -> Result<Self, HostnameError> { >> + let name_ref = name.as_ref(); >> + >> + if !(1..63).contains(&name_ref.len()) { > > This actually fails for hostnames exactly 63 characters long (which > should be accepted), as `..` is non-inclusive :^) > > E.g. the following tests, adapted from proxmox-installer-common/utils.rs > in pve-installer: > > #[cfg(test)] > mod tests { > use super::*; > > #[test] > fn too_long() { > assert!(Hostname::new(format!("{}.com", "a".repeat(63))).is_ok()); > assert!(Hostname::new(format!("{}.com", "a".repeat(64))).is_err()); > } > } > >> + return Err(HostnameError::InvalidLength); >> + } >> + >> + let host_name = name_ref.to_lowercase(); >> + >> + let mut characters = host_name.chars(); >> + >> + // first character must not be a hyphen >> + // SAFETY: ok because of length check >> + if !characters.next().unwrap().is_alphanumeric() { > > This should rather use .is_ascii_alphanumeric(). Otherwise any > alphanumeric Unicode character is accepted [1], but hostnames may be > ASCII only, as per e.g. RFC 952 ("ASSUMPTIONS") and hostname(7) [2]. > > [1] https://doc.rust-lang.org/std/primitive.char.html#method.is_alphabetic > [2] https://manpages.debian.org/bookworm/manpages/hostname.7.en.html > >> + return Err(HostnameError::InvalidSymbols); >> + } >> + >> + if !characters.all(|c| c.is_alphanumeric() || c == '-') { > > ^ same as above. > >> + return Err(HostnameError::InvalidSymbols); >> + } >> + >> + // last character must not be a hyphen >> + // SAFETY: ok because of length check >> + if !host_name.chars().next_back().unwrap().is_alphanumeric() { > > ^ same as above. > Thanks! Will send a v3 with the suggested improvements _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel