From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 786FA1FF144 for ; Tue, 10 Mar 2026 12:01:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C38AA19007; Tue, 10 Mar 2026 12:01:46 +0100 (CET) Message-ID: <41e62095-021e-4b17-93e3-d8541d6574c1@proxmox.com> Date: Tue, 10 Mar 2026 12:01:09 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file To: Hannes Laimer , pve-devel@lists.proxmox.com References: <20260310101631.32870-1-f.ebner@proxmox.com> <20260310101631.32870-6-f.ebner@proxmox.com> <2045463e-f1ef-464f-b74d-b4e3ce45d561@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <2045463e-f1ef-464f-b74d-b4e3ce45d561@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1773140435916 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.067 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.408 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.819 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.903 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 Message-ID-Hash: X6QK6B366RP62ZTNCOQB7MW46U7QWOLD X-Message-ID-Hash: X6QK6B366RP62ZTNCOQB7MW46U7QWOLD X-MailFrom: f.ebner@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 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 10.03.26 um 11:39 AM schrieb Hannes Laimer: > On 2026-03-10 11:16, Fiona Ebner wrote: >> A configuration file registered as UTF-8 will be automatically decoded >> from UTF-8 to Perl's internal string format after reading and encoded >> in the other direction before writing. >> >> Signed-off-by: Fiona Ebner >> --- >> >> Versioned dependency bump pve-cluster -> pve-common needed! >> >> Changes in v4: >> * tell the parser() directly whether the file was registered as UTF-8 >> >> src/PVE/Cluster.pm | 18 ++++++++++++++---- >> 1 file changed, 14 insertions(+), 4 deletions(-) >> >> diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm >> index cd5d6b5..c1e7d4c 100644 >> --- a/src/PVE/Cluster.pm >> +++ b/src/PVE/Cluster.pm >> @@ -519,7 +519,7 @@ sub verify_token { >> my $file_info = {}; >> >> sub cfs_register_file { >> - my ($filename, $parser, $writer) = @_; >> + my ($filename, $parser, $writer, $options) = @_; >> >> $observed->{$filename} || die "unknown file '$filename'"; >> >> @@ -529,12 +529,13 @@ sub cfs_register_file { >> parser => $parser, >> writer => $writer, >> }; >> + $file_info->{$filename}->{utf8} = 1 if $options && $options->{utf8}; >> >> return; >> } >> >> my $ccache_read = sub { >> - my ($filename, $parser, $version) = @_; >> + my ($filename, $parser, $version, $utf8) = @_; >> >> $ccache->{$filename} = {} if !$ccache->{$filename}; >> >> @@ -544,7 +545,14 @@ my $ccache_read = sub { >> # we always call the parser, even when the file does not exist >> # (in that case $data is undef) >> my $data = get_config($filename); >> - $ci->{data} = &$parser("/etc/pve/$filename", $data); >> + my $options = {}; >> + >> + if ($utf8) { >> + $data = decode('UTF-8', $data); > > The docs[1] mention calling decode with undef is harmless, but will > produce a warning, we should probably check for undef before decode Good catch! get_config() returns undef if the file does not exist. I cannot see a warning though: [I] root@pve9a1 ~# cat asdf.pl #!/usr/bin/perl use warnings; use strict; use Data::Dumper; use Encode; my $res = decode('UTF-8', undef); print Dumper($res); [I] root@pve9a1 ~# perl asdf.pl $VAR1 = undef; I can add a check in v5, but will wait for further reviews first. > not sure if we need one, but are we missing a > ``` > +use Encode qw(decode); > ``` > in Cluster.pm? There already is use Encode; which auto-imports 'decode' and 'encode' (among others) If we change it, we also need to import 'encode' explicitly (and check if any others are used) to not break the existing calls to 'encode'. > > > [1] > https://perldoc.perl.org/5.8.2/Encode#$string-=-decode(ENCODING,-$octets-%5B,-CHECK%5D)