From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id D3BDC1FF17E for ; Thu, 16 Oct 2025 08:12:59 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 85EFDA39B; Thu, 16 Oct 2025 08:13:19 +0200 (CEST) Message-ID: <6d7c3a09-abdf-4b62-ba0d-9af02c732bc4@proxmox.com> Date: Thu, 16 Oct 2025 08:13:14 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Wolfgang Bumiller References: <20251001154717.405839-1-h.laimer@proxmox.com> <20251001154717.405839-2-h.laimer@proxmox.com> Content-Language: en-US From: Hannes Laimer In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760595191677 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.042 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, schema2rust.pm] Subject: Re: [pdm-devel] [PATCH 1/2] schema2rust: extract pattern validation from nested API schema fields X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: pdm-devel@lists.proxmox.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On 10/15/25 14:32, Wolfgang Bumiller wrote: > The patch itself works, but there are 2 issues: > > The currently generated regexes (luckily?) all produce regexes which are > 1. anchored > 2. compatible with the regex crate > > With your patch, a lot of them will not satisfy both. > > For 1: > The PVE schema code generally anchors the regexes at *use* time, which > is less than ideal and we could probably move towards including the > anchors in the regexes itself (unless we specifically need them > unanchored for some reason?) > In the rust code we could not do this. The regex crate does not expose > this and requires them to be anchored explicitly within the regex. > > We could also include `^` and `$` in the generator, but it'll be a bit > redundant (but won't hurt). > > But for 2: > I'm afraid what we need to do is find a way to "whitelist" which regexes > get generated. We could produce TODO comments for the rest. > > I went ahead and pushed a commit which produces tests for the regexes. > If you go ahead and run `cargo` test after regenerating with this patch, > you'll see the issue. > Thanks for taking a look! Hmm, I don't think we have patterns that rely on PCRE specific features, i.e. we could maybe convert those without losing semantic meaning for validation. I'm not super familiar with PCRE and its intricacies, but I don't think we have pattern that couldn't be represented otherwise. Some things conversion would have to do: - {,N} -> {0,N} - in rust we shouldn't need flag-resets, (I think) - (?^i:...) -> (?i:...) - ... Not sure if it would be feasible to try to integrate some kind of "conversion" into the generator itself. Of the patterns used in the PVE API spec, I think this would mostly be syntactic things. BUT I did probably miss some things :P also, is there a reason we would not want this assuming converting patterns "on-the-fly" in the generator is feasible? > On Wed, Oct 01, 2025 at 05:47:16PM +0200, Hannes Laimer wrote: >> Previously, pattern validation was only extracted from top-level string fields, >> leaving nested pattern fields and array item patterns unprocessed. This change >> enables pattern extraction from nested schema structures, ensuring regex >> validation is generated for all pattern fields regardless of their depth. >> The generated Rust code now includes const_regex! definitions and >> ApiStringFormat::Pattern references for previously ignored pattern fields. >> >> Signed-off-by: Hannes Laimer >> --- >> pve-api-types/generator-lib/Schema2Rust.pm | 27 +++++++++++++++++++--- >> 1 file changed, 24 insertions(+), 3 deletions(-) >> >> diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm >> index 0f3efc0a..794c7abe 100644 >> --- a/pve-api-types/generator-lib/Schema2Rust.pm >> +++ b/pve-api-types/generator-lib/Schema2Rust.pm >> @@ -206,6 +206,25 @@ my sub print_struct : prototype($$$) { >> >> my @arrays; >> >> + # Collect array fields >> + for my $field (sort keys $def->{fields}->%*) { >> + my $field_def = $def->{fields}->{$field}; >> + if ($field_def->{kind} eq 'array-field') { >> + push @arrays, $field_def; >> + } >> + } >> + >> + # Add regex constants from array fields to the main struct's API object >> + for my $array (@arrays) { >> + my $type_name = $array->{array_type_name}; >> + next if $done_array_types->{$type_name}; >> + if ($API && $array->{api}->{-regexes}) { >> + for my $name (keys %{$array->{api}->{-regexes}}) { >> + $def->{api}->{-regexes}->{$name} = $array->{api}->{-regexes}->{$name}; >> + } >> + } >> + } >> + >> print_api_string($out, $def->{api}, 'struct', $def->{name}); >> >> if (length($def->{description})) { >> @@ -217,9 +236,6 @@ my sub print_struct : prototype($$$) { >> print {$out} "pub struct $def->{name} {\n"; >> for my $field (sort keys $def->{fields}->%*) { >> my $field_def = $def->{fields}->{$field}; >> - if ($field_def->{kind} eq 'array-field') { >> - push @arrays, $field_def; >> - } >> >> my $name = $field_def->{name}; >> my $rust_name = $field_def->{rust_name}; >> @@ -1097,6 +1113,11 @@ my sub string_type : prototype($$$$) { >> # if (my $kind = $fmt->{kind}) { >> # $api_props->{format_fixme} = '"LIST TYPE"'; >> # } >> + } elsif (defined(my $pattern = delete $schema->{pattern})) { >> + # Handle pattern field directly from schema if no format was specified >> + my $re_name = namify_const(${name_hint}, 're'); >> + $api_props->{-regexes}->{$re_name} = $pattern; >> + $api_props->{format} = "&ApiStringFormat::Pattern(&$re_name)"; >> } >> >> return 'String'; >> -- >> 2.47.3 >> >> >> >> _______________________________________________ >> pdm-devel mailing list >> pdm-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel >> >> _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel