[pve-devel] [PATCH common] fix #3747: download_file_from_url: trim whitespace before comparing checksum

[pve-devel] [PATCH common] fix #3747: download_file_from_url: trim whitespace before comparing checksum

[Oguz Bektas]

so that we don't get checksum mismatch in case the user accidentally
copies whitespace in the checksum field.

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
 src/PVE/Tools.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 787942a..fa14f2f 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1921,6 +1921,7 @@ sub download_file_from_url {
 	if (defined($opts->{"${_}sum"})) {
 	    $checksum_algorithm = $_;
 	    $checksum_expected = $opts->{"${_}sum"};
+	    $checksum_expected =~ s/^\s+|\s+$//g;
 	    last;
 	}
     }
-- 
2.30.2

Re: [pve-devel] [PATCH common] fix #3747: download_file_from_url: trim whitespace before comparing checksum

[Fabian Ebner]

Am 02.12.21 um 14:38 schrieb Oguz Bektas:
> so that we don't get checksum mismatch in case the user accidentally
> copies whitespace in the checksum field.
> 

Isn't this better done in the UI? Or at least upon parameter validation 
(but we don't do this for any other parameter, or)?.

> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>   src/PVE/Tools.pm | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 787942a..fa14f2f 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -1921,6 +1921,7 @@ sub download_file_from_url {
>   	if (defined($opts->{"${_}sum"})) {
>   	    $checksum_algorithm = $_;
>   	    $checksum_expected = $opts->{"${_}sum"};
> +	    $checksum_expected =~ s/^\s+|\s+$//g;
>   	    last;
>   	}
>       }
> 

Re: [pve-devel] [PATCH common] fix #3747: download_file_from_url: trim whitespace before comparing checksum

[Fabian Grünbichler]

On December 3, 2021 9:03 am, Fabian Ebner wrote:
> Am 02.12.21 um 14:38 schrieb Oguz Bektas:
>> so that we don't get checksum mismatch in case the user accidentally
>> copies whitespace in the checksum field.
>> 
> 
> Isn't this better done in the UI? Or at least upon parameter validation 
> (but we don't do this for any other parameter, or)?.
> 
>> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
>> ---
>>   src/PVE/Tools.pm | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
>> index 787942a..fa14f2f 100644
>> --- a/src/PVE/Tools.pm
>> +++ b/src/PVE/Tools.pm
>> @@ -1921,6 +1921,7 @@ sub download_file_from_url {
>>   	if (defined($opts->{"${_}sum"})) {
>>   	    $checksum_algorithm = $_;
>>   	    $checksum_expected = $opts->{"${_}sum"};
>> +	    $checksum_expected =~ s/^\s+|\s+$//g;

PVE::Tools::trim ?

>>   	    last;
>>   	}
>>       }
>> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH common] fix #3747: download_file_from_url: trim whitespace before comparing checksum

[Thomas Lamprecht]

On 03.12.21 09:03, Fabian Ebner wrote:
> Am 02.12.21 um 14:38 schrieb Oguz Bektas:
>> so that we don't get checksum mismatch in case the user accidentally
>> copies whitespace in the checksum field.
>>
> 
> Isn't this better done in the UI? Or at least upon parameter validation (but we don't do this for any other parameter, or)?.
> 

Yeah, I do not like messing with the checksum in the backend to much,
maybe we can enforce the format such that it doesn't allow any whitespace
and explicitly errors out for api users that send some.

I trimmed for now in the frontend:

https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=9e9a3abd0a5eedc76a41f6ebcd719dca72d536b8