Re: [PATCH proxmox v5 19/42] notify: matcher: ensure that field names match SAFE_ID_REGEX

["Lukas Wagner" <l.wagner@proxmox.com>]

On Thu Oct 22, 2026 at 4:30 PM CEST, Michael Köppl wrote:
> Thanks for adding these. Considering the tests here all pass, I just
> quickly ran through some manual tests with pvesh with invalid field
> names in various expressions and everything seems to work as expected.
>
> 1 comment inline that can easily be a follow-up.
>
> Tested-by: Michael Köppl <m.koeppl@proxmox.com>
> Reviewed-by: Michael Köppl <m.koeppl@proxmox.com>
>
> On Thu Oct 22, 2026 at 2:15 PM CEST, Lukas Wagner wrote:
>
> [snip]
>
>> +        match self {
>> +            FieldMatcher::Exact { field, values } => {
>> +                if values.is_empty() {
>> +                    return Err(Error::Generic(
>> +                        "field matcher must contain values to match".to_string(),
>> +                    ));
>> +                }
>> +                if !SAFE_ID_REGEX.is_match(field) {
>> +                    return Err(Error::Generic(
>> +                        "field name does not match regex pattern".to_string(),
>
> nit: would benefit from mentioning the field as well. Something like
> "invalid field name {field}" or "field name {field} does not match regex
> pattern".

I'll adapt the string in a followup patch in another series, I'm changing the error type
where I have to adapt this line anywhere.

Thanks!

>
>> +                    ));
>> +                }
>> +            }
>> +            FieldMatcher::Regex { field, .. } => {
>> +                if !SAFE_ID_REGEX.is_match(field) {
>> +                    return Err(Error::Generic(
>> +                        "field name does not match regex pattern".to_string(),
>
> nit: same as above
>
>> +                    ));
>> +                }
>>              }
>>          }
>>
>
> [snip]