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 0FB181FF13F for ; Thu, 12 Mar 2026 14:25:58 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 548BC15C75; Thu, 12 Mar 2026 14:25:54 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 12 Mar 2026 14:25:20 +0100 Message-Id: From: "Daniel Kral" To: "Wolfgang Bumiller" Subject: Re: [RFC PATCH common] section config: correctly handle required options in write_config X-Mailer: aerc 0.21.0-38-g7088c3642f2c-dirty References: <20260311122659.250421-1-d.kral@proxmox.com> <5au47ehnodyxkvpthlc7kxsh6ttnbzosexabjsihf64bka3ama@y2jskncbucy7> In-Reply-To: <5au47ehnodyxkvpthlc7kxsh6ttnbzosexabjsihf64bka3ama@y2jskncbucy7> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1773321885095 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_MSPIKE_H2 0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: CDBKMMLP6IXCE2K6FTAIMC4OHF2ZRTJG X-Message-ID-Hash: CDBKMMLP6IXCE2K6FTAIMC4OHF2ZRTJG X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu Mar 12, 2026 at 2:06 PM CET, Wolfgang Bumiller wrote: > On Wed, Mar 11, 2026 at 01:26:18PM +0100, Daniel Kral wrote: >> diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm >> index 84ff81a..d0332f9 100644 >> --- a/src/PVE/SectionConfig.pm >> +++ b/src/PVE/SectionConfig.pm >> @@ -1572,11 +1572,12 @@ sub write_config { >> next if $opts->{$k}->{optional}; >> $done_hash->{$k} =3D 1; >> my $v =3D $scfg->{$k}; >> - die "section '$sectionId' - missing value for required opti= on '$k'\n" >> - if !defined($v); >> $v =3D $class->encode_value($type, $k, $v); > > Note that this also changes the public section config API by requiring > `encode_value()` to deal with the `$value` parameter being `undef` - > previously this would not happen here. Right, good catch! I had a bad gut feeling about moving it farther here and only did so that the die is coupled to whether format_config_line(...) does add a line to the config $data string. I'd go for having both the definedness check above and the 'is the data actually written to the config file?' check below with a more telling error message in a v2, if there are no objections. > > Its description would need to be updated, and its example as well, since > it only checks the `$key` before then dereferencing `$value` as a hash. > > A quick grep shows a bunch of unprotected uses: > - PVE::Storage::Plugin for `nodes` or `content{,-dirs}` > - PVE::SDN::Zones::Plugin > - PVE::ACME::Challenge > - PVE::HA::Rules > - PVE::HA::Groups > - plugins for PVE::Job::Registry > >> my $prop =3D $class->get_property_schema($type, $k); >> - $data .=3D format_config_line($prop, $k, $v); >> + my $cfg_line =3D format_config_line($prop, $k, $v); >> + die "section '$sectionId' - missing value for required opti= on '$k'\n" >> + if !$cfg_line; >> + $data .=3D $cfg_line; >> } >> =20 >> for my $k (@option_keys) { >> --=20 >> 2.47.3