Re: [pdm-devel] [PATCH 1/2] schema2rust: extract pattern validation from nested API schema fields

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Thu, Oct 16, 2025 at 08:13:14AM +0200, Hannes Laimer wrote:
> 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}

Weirdly it accepts {N,}, but not {,N} ...

> - in rust we shouldn't need flag-resets, (I think)
> - (?^i:...) -> (?i:...)

Missing a `-` here - (?^i:...) would be (?-i:...) AFAICT ;-)

We'd only not need the *outer* ones, but anything interpolated into the
middle of a regex may well need it.

`qr/foo${other_re}bar/i` <- there's an `/i` at the end, if `$other_re`
uses `qr//` as well, *its* `(?^i:...)` is still required.

Also - we still do need to anchor the un-anchored regexes with `^…$`.

> - ...
> 
> 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?

If the conversion is sound, we can do this. But if that entails
fully and properly parsing regular expressions, then I'd rather manually
vet the regexes and require them to be predefined in generate.pl (with a
from->to mapping to verify that the perl-side hasn't changed)...


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