[pve-devel] [PATCH v2 storage 1/2] fix #3894: cast 'size' and 'used' to integer

[pve-devel] [PATCH v2 storage 1/2] fix #3894: cast 'size' and 'used' to integer

[Mira Limbeck]

Perl's automatic conversion can lead to integers being converted to
strings, for example by matching it in a regex.

To make sure we always return an integer in the API calls, add
explicit casts to integer.

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
v2: new

 PVE/API2/Storage/Content.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
index 45b8de8..42bff81 100644
--- a/PVE/API2/Storage/Content.pm
+++ b/PVE/API2/Storage/Content.pm
@@ -140,6 +140,8 @@ __PACKAGE__->register_method ({
 	    eval {  PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $item->{volid}); };
 	    next if $@;
 	    $item->{vmid} = int($item->{vmid}) if (defined($item->{vmid}));
+	    $item->{size} = int($item->{size}) if (defined($item->{size}));
+	    $item->{used} = int($item->{used}) if (defined($item->{used}));
 	    push @$res, $item;
 	}
 
@@ -326,8 +328,8 @@ __PACKAGE__->register_method ({
 
 	my $entry = {
 	    path => $path,
-	    size => $size,
-            used => $used,
+	    size => int($size), # cast to integer in case it was changed to a string previously
+            used => int($used),
 	    format => $format,
 	};
 
-- 
2.30.2

[pve-devel] [PATCH v2 storage 2/2] file_size_info: cast 'size' and 'used' to integer

[Mira Limbeck]

`qemu-img info --output=json` returns the size and used values as integers in
the JSON format, but the regex match converts them to strings.
As we know they only contain digits, we can simply cast them back to integers
after the regex.

The API requires them to be integers.

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
v2:
 - reworded commit subject and message

 PVE/Storage/Plugin.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 12f1b4b..bcc0cc0 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -892,7 +892,13 @@ sub file_size_info {
     my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format actual-size backing-filename)};
 
     ($size) = ($size =~ /^(\d+)$/) or die "size '$size' not an integer\n"; # untaint
+    # the regex above changes the type of $size to 'string'
+    # we know it is an integer based on the regex above, so simply cast it back
+    # this is required by the API
+    $size = int($size);
     ($used) = ($used =~ /^(\d+)$/) or die "used '$used' not an integer\n"; # untaint
+    # same as $size above
+    $used = int($used);
     ($format) = ($format =~ /^(\S+)$/) or die "format '$format' includes whitespace\n"; # untaint
     if (defined($parent)) {
 	($parent) = ($parent =~ /^(\S+)$/) or die "parent '$parent' includes whitespace\n"; # untaint
-- 
2.30.2

Re: [pve-devel] [PATCH v2 storage 2/2] file_size_info: cast 'size' and 'used' to integer

[Fabian Ebner]

Am 17.02.22 um 15:54 schrieb Mira Limbeck:
> `qemu-img info --output=json` returns the size and used values as integers in
> the JSON format, but the regex match converts them to strings.
> As we know they only contain digits, we can simply cast them back to integers
> after the regex.
> 
> The API requires them to be integers.
> 
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> v2:
>  - reworded commit subject and message
> 
>  PVE/Storage/Plugin.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 12f1b4b..bcc0cc0 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -892,7 +892,13 @@ sub file_size_info {
>      my ($size, $format, $used, $parent) = $info->@{qw(virtual-size format actual-size backing-filename)};
>  
>      ($size) = ($size =~ /^(\d+)$/) or die "size '$size' not an integer\n"; # untaint
> +    # the regex above changes the type of $size to 'string'
> +    # we know it is an integer based on the regex above, so simply cast it back
> +    # this is required by the API

Nit: Not sure if it's worth to put such a lengthy comment rather than
something like "coerce back from string". One can still read the commit
message in the git history if there ever is confusion about why it's there.

> +    $size = int($size);
>      ($used) = ($used =~ /^(\d+)$/) or die "used '$used' not an integer\n"; # untaint
> +    # same as $size above
> +    $used = int($used);
>      ($format) = ($format =~ /^(\S+)$/) or die "format '$format' includes whitespace\n"; # untaint
>      if (defined($parent)) {
>  	($parent) = ($parent =~ /^(\S+)$/) or die "parent '$parent' includes whitespace\n"; # untaint

Re: [pve-devel] [PATCH v2 storage 1/2] fix #3894: cast 'size' and 'used' to integer

[Fabian Ebner]

Am 17.02.22 um 15:54 schrieb Mira Limbeck:
> Perl's automatic conversion can lead to integers being converted to
> strings, for example by matching it in a regex.
> 
> To make sure we always return an integer in the API calls, add
> explicit casts to integer.
> 
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> v2: new
> 
>  PVE/API2/Storage/Content.pm | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/API2/Storage/Content.pm b/PVE/API2/Storage/Content.pm
> index 45b8de8..42bff81 100644
> --- a/PVE/API2/Storage/Content.pm
> +++ b/PVE/API2/Storage/Content.pm
> @@ -140,6 +140,8 @@ __PACKAGE__->register_method ({
>  	    eval {  PVE::Storage::check_volume_access($rpcenv, $authuser, $cfg, undef, $item->{volid}); };
>  	    next if $@;
>  	    $item->{vmid} = int($item->{vmid}) if (defined($item->{vmid}));
> +	    $item->{size} = int($item->{size}) if (defined($item->{size}));
> +	    $item->{used} = int($item->{used}) if (defined($item->{used}));

Style nit: superfluous parentheses (of course the pre-existing one is to
blame ;))

>  	    push @$res, $item;
>  	}
>  
> @@ -326,8 +328,8 @@ __PACKAGE__->register_method ({
>  
>  	my $entry = {
>  	    path => $path,
> -	    size => $size,
> -            used => $used,
> +	    size => int($size), # cast to integer in case it was changed to a string previously
> +            used => int($used),

Style nit: please fix the pre-existing white-space error while modifying
the line

>  	    format => $format,
>  	};
>  

Just minor nits, so both patches:

Reviewed-by: Fabian Ebner <f.ebner@proxmox.com>