From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D8794CB1C for ; Wed, 16 Aug 2023 11:09:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B9154132DE for ; Wed, 16 Aug 2023 11:09:13 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 16 Aug 2023 11:09:12 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 98D3947925 for ; Wed, 16 Aug 2023 11:09:12 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Wed, 16 Aug 2023 11:09:11 +0200 Message-Id: <20230816090911.3741277-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.011 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 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. [sectionconfig.pm, proxmox.com] Subject: [pve-devel] [PATCH common v2] SectionConfig: fix handling unknown sections X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Aug 2023 09:09:13 -0000 if we're parsing an unknown section, we cannot check the schema with `is_array` to check if it's an array type or not, thus we have to handle that separately. fix this by handling data in unknown sections like an array similar to "cb2646c7b4974e33f4148752deec71f0d589b0f3" in proxmox-section-config. This way we can write unknown section out again like we parsed it. Add a regression test for an unknown field not in the schema. This fixes an issue, where calling `qm destroy ID --purge` removed much of the configs ob backup jobs (since there we parse an 'unknown' section and run into the `is_array` error) (Reported in the forum: https://forum.proxmox.com/threads/132091) Suggested-by: Fiona Ebner Signed-off-by: Dominik Csapak --- changes from v1: * only convert to array on the second occurance of a key (more future proof and does not need a change in the testcase) * add test case that would have triggered the original issue * improved commit message src/PVE/SectionConfig.pm | 24 +++++++++++++++++------- test/section_config_test.pl | 2 ++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm index 2b75e6a..5690476 100644 --- a/src/PVE/SectionConfig.pm +++ b/src/PVE/SectionConfig.pm @@ -369,17 +369,22 @@ sub parse_config { my ($k, $v) = ($1, $3); eval { - if ($is_array->($type, $k)) { - if (!$unknown) { - $v = $plugin->check_value($type, $k, $v, $sectionId); + if ($unknown) { + if (!defined($config->{$k})) { + $config->{$k} = $v; + } else { + if (!ref($config->{$k})) { + $config->{$k} = [$config->{$k}]; + } + push $config->{$k}->@*, $v; } + } elsif ($is_array->($type, $k)) { + $v = $plugin->check_value($type, $k, $v, $sectionId); $config->{$k} = [] if !defined($config->{$k}); push $config->{$k}->@*, $v; } else { die "duplicate attribute\n" if defined($config->{$k}); - if (!$unknown) { - $v = $plugin->check_value($type, $k, $v, $sectionId); - } + $v = $plugin->check_value($type, $k, $v, $sectionId); $config->{$k} = $v; } }; @@ -525,7 +530,12 @@ sub write_config { next if defined($done_hash->{$k}); $done_hash->{$k} = 1; my $v = $scfg->{$k}; - $data .= "\t$k $v\n"; + my $ref = ref($v); + if (defined($ref) && $ref eq 'ARRAY') { + $data .= "\t$k $_\n" for $v->@*; + } else { + $data .= "\t$k $v\n"; + } } $out .= "$data\n"; next; diff --git a/test/section_config_test.pl b/test/section_config_test.pl index 02242bc..ccb364d 100755 --- a/test/section_config_test.pl +++ b/test/section_config_test.pl @@ -218,6 +218,7 @@ my $with_unknown_data = { invalid => { type => 'bad', common => 'omg', + unknownfield => 'shouldnotbehere', }, }, order => enum(qw(t1 t2 invalid t3)), @@ -234,6 +235,7 @@ one: t2 bad: invalid common omg + unknownfield shouldnotbehere two: t3 field2 5 -- 2.30.2