[RFC common/cluster v4 0/5] cluster files: support registering UTF-8 configuration file

[RFC common/cluster v4 0/5] cluster files: support registering UTF-8 configuration file

[Fiona Ebner]

Changes in v4:
* rather than searching for wide characters, tell parse_config()
  directly whether the file was registered as UTF-8

Changes in v3:
* rebase on current master.
* add patch to use strict UTF-8 encoding when writing.
* use strict UTF-8 for {de,en}coding in the rest of the series.
* let cfs_register_file() take an $options parameter rather than
  an $utf8 parameter, so future call sites are more readable.

Changes in v2:
* rebase on current master.
* use qw(encode) when importing Encode module.

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.

Patch 1/5 also makes sense without the rest of the RFC.

Note that patch 4/5 (required by 3/5) is a versioned breaks for
libpve-access-control which still needs to be bumped! Could be avoided
by keeping the implicit return of cfs_register_file(), but that's bad
for encapsulation.

Versioned dependency bump pve-cluster -> pve-common needed!

common:

Fiona Ebner (2):
  file: set contents: use strict UTF-8 encoding with $force_utf8
  section config: prepare for supporting UTF-8 encoded configurations

 src/PVE/File.pm          |  2 +-
 src/PVE/SectionConfig.pm | 29 ++++++++++++++++++++++++-----
 2 files changed, 25 insertions(+), 6 deletions(-)


cluster:

Fiona Ebner (3):
  cfs register file: avoid implicit return
  d/control: add versioned breaks for libpve-access-control
  cluster files: support registering UTF-8 configuration file

 debian/control     |  2 +-
 src/PVE/Cluster.pm | 20 ++++++++++++++++----
 2 files changed, 17 insertions(+), 5 deletions(-)


Summary over all repositories:
  4 files changed, 42 insertions(+), 11 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH common v4 1/5] file: set contents: use strict UTF-8 encoding with $force_utf8

[Fiona Ebner]

'perldoc Encode' states:

> "UTF-8" means UTF-8 in its current sense, which is conservative and
> strict and security-conscious, whereas "utf8" means UTF-8 in its
> former sense, which was liberal and loose and lax.

Currently, the following callers use the $force_utf8 flag:

1. Notification config
2. The SDN fabrics config
3. The PBS password file

The first two pass the contents to Rust, and the latter already uses
the strict 'UTF-8' encoding when decoding. So in all cases, after this
patch, any potential issues are just caught earlier, before writing.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/File.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/File.pm b/src/PVE/File.pm
index 7d0f7ab..89884a8 100644
--- a/src/PVE/File.pm
+++ b/src/PVE/File.pm
@@ -60,7 +60,7 @@ sub file_set_contents($filename, $data, $perm = 0644, $force_utf8 = 0) {
         die "unable to open file '$tmpname' - $!\n" if !$fh;
 
         if ($force_utf8) {
-            $data = encode("utf8", $data);
+            $data = encode("UTF-8", $data);
         } else {
             # Encode wide characters with print before passing them to syswrite
             my $unencoded_data = $data;
-- 
2.47.3

[PATCH common v4 2/5] section config: prepare for supporting UTF-8 encoded configurations

[Fiona Ebner]

Configurations registered as UTF-8 will be decoded after reading to
Perl's internal string format and thus may contain wide characters.
The Digest::SHA::sha1_hex() function croaks on wide characters, so
encode again before calling the function.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v4:
* rather than searching for wide characters, tell parse_config()
  directly whether the file was registered as UTF-8

 src/PVE/SectionConfig.pm | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/PVE/SectionConfig.pm b/src/PVE/SectionConfig.pm
index 84ff81a..9ff91ee 100644
--- a/src/PVE/SectionConfig.pm
+++ b/src/PVE/SectionConfig.pm
@@ -103,6 +103,7 @@ use warnings;
 
 use Carp;
 use Digest::SHA;
+use Encode qw(encode);
 
 use PVE::Exception qw(raise_param_exc);
 use PVE::JSONSchema qw(get_standard_option);
@@ -1103,10 +1104,10 @@ sub get_property_schema {
 
 =head3 $base->parse_config(...)
 
-=head3 $base->parse_config($filename, $raw [, $allow_unknown ])
+=head3 $base->parse_config($filename, $raw [, $allow_unknown [, $options ]])
 
-    $config = PVE::Example::BasePlugin->parse_config($filename, $raw, $allow_unknown)
-    $config = $class->parse_config($filename, $raw, $allow_unknown)
+    $config = PVE::Example::BasePlugin->parse_config($filename, $raw, $allow_unknown, $options)
+    $config = $class->parse_config($filename, $raw, $allow_unknown, $options)
 
 Parses the contents of a file as C<L<PVE::SectionConfig>>, returning the parsed
 data annotated with additional information (see below).
@@ -1127,6 +1128,18 @@ The raw content of C<$filename>.
 
 Whether to allow parsing unknown I<types>.
 
+=item C<$options> (optional)
+
+Further options:
+
+=over
+
+=item C<$utf8>
+
+The config file was registered as UTF-8 encoded.
+
+=back
+
 =back
 
 The returned hash is structured as follows:
@@ -1205,7 +1218,7 @@ The error.
 =cut
 
 sub parse_config {
-    my ($class, $filename, $raw, $allow_unknown) = @_;
+    my ($class, $filename, $raw, $allow_unknown, $options) = @_;
 
     my $pdata = $class->private();
 
@@ -1214,7 +1227,13 @@ sub parse_config {
 
     $raw = '' if !defined($raw);
 
-    my $digest = Digest::SHA::sha1_hex($raw);
+    my $bytes = $raw;
+    if ($options && $options->{utf8}) {
+        # Digest::SHA croaks on wide characters
+        $bytes = encode('UTF-8', $raw);
+    }
+
+    my $digest = Digest::SHA::sha1_hex($bytes);
 
     my $pri = 1;
 
-- 
2.47.3

[PATCH cluster v4 3/5] cfs register file: avoid implicit return

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Requires the versioned breaks done in the next patch.

 src/PVE/Cluster.pm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/PVE/Cluster.pm b/src/PVE/Cluster.pm
index bdb465f..cd5d6b5 100644
--- a/src/PVE/Cluster.pm
+++ b/src/PVE/Cluster.pm
@@ -529,6 +529,8 @@ sub cfs_register_file {
         parser => $parser,
         writer => $writer,
     };
+
+    return;
 }
 
 my $ccache_read = sub {
-- 
2.47.3

[PATCH cluster v4 4/5] d/control: add versioned breaks for libpve-access-control

[Fiona Ebner]

Before commit a484970 ("token config: have module return a true
value"), libpve-access-control implicitly relied on the implicit
return value from cfs_register_file() as the return value of the
module.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Note that libpve-access-control needs to be bumped first!

 debian/control | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/debian/control b/debian/control
index 594d29c..4265c93 100644
--- a/debian/control
+++ b/debian/control
@@ -40,7 +40,7 @@ Depends: corosync (>= 2.3.4-1),
          ${misc:Depends},
          ${perl:Depends},
          ${shlibs:Depends},
-Breaks: libpve-access-control (<= 6.0-3),
+Breaks: libpve-access-control (<= 9.0.5),
         libpve-guest-common-perl (<= 3.0-2),
         libpve-storage-perl (<= 6.0-9),
         pve-container (<= 3.0-10),
-- 
2.47.3

[PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file

[Fiona Ebner]

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 <f.ebner@proxmox.com>
---

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);
+            $options->{utf8} = 1;
+        }
+
+        $ci->{data} = &$parser("/etc/pve/$filename", $data, undef, $options);
         $ci->{version} = $version;
     }
 
@@ -581,7 +589,7 @@ sub cfs_read_file {
     my ($version, $info) = cfs_file_version($filename);
     my $parser = $info->{parser};
 
-    return &$ccache_read($filename, $parser, $version);
+    return &$ccache_read($filename, $parser, $version, $info->{utf8});
 }
 
 sub cfs_write_file {
@@ -599,6 +607,8 @@ sub cfs_write_file {
         $ci->{version} = undef;
     }
 
+    $force_utf8 = 1 if $info->{utf8};
+
     PVE::Tools::file_set_contents($fsname, $raw, undef, $force_utf8);
 }
 
-- 
2.47.3

Re: [PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file

[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 <f.ebner@proxmox.com>
> ---
> 
> 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

not sure if we need one, but are we missing a
```
+use Encode qw(decode);
```
in Cluster.pm?


[1]
https://perldoc.perl.org/5.8.2/Encode#$string-=-decode(ENCODING,-$octets-%5B,-CHECK%5D)

> +            $options->{utf8} = 1;
> +        }
> +
> +        $ci->{data} = &$parser("/etc/pve/$filename", $data, undef, $options);
>          $ci->{version} = $version;
>      }
>  
> @@ -581,7 +589,7 @@ sub cfs_read_file {
>      my ($version, $info) = cfs_file_version($filename);
>      my $parser = $info->{parser};
>  
> -    return &$ccache_read($filename, $parser, $version);
> +    return &$ccache_read($filename, $parser, $version, $info->{utf8});
>  }
>  
>  sub cfs_write_file {
> @@ -599,6 +607,8 @@ sub cfs_write_file {
>          $ci->{version} = undef;
>      }
>  
> +    $force_utf8 = 1 if $info->{utf8};
> +
>      PVE::Tools::file_set_contents($fsname, $raw, undef, $force_utf8);
>  }
>  

Re: [PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file

[Fiona Ebner]

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 <f.ebner@proxmox.com>
>> ---
>>
>> 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)

Re: [PATCH cluster v4 5/5] cluster files: support registering UTF-8 configuration file

[Hannes Laimer]

On 2026-03-10 12:00, Fiona Ebner wrote:
> 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 <f.ebner@proxmox.com>
>>> ---
>>>
>>> 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.

actually its fine like that, `decode` seems to have a check for that
and just return undef[2] in that case :P sorry for the noise :)

[2] https://metacpan.org/pod/Encode#decode

> 
>> 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)