From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id AC7321FF15C
for <inbox@lore.proxmox.com>; Fri, 4 Apr 2025 16:12:41 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 5C92D33548;
Fri, 4 Apr 2025 16:12:27 +0200 (CEST)
Message-ID: <05101c04-ba1c-4baa-9e2f-49b2a5522837@proxmox.com>
Date: Fri, 4 Apr 2025 16:11:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Dominik Csapak <d.csapak@proxmox.com>
References: <20250401082318.1312661-1-d.csapak@proxmox.com>
<20250401082318.1312661-2-d.csapak@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20250401082318.1312661-2-d.csapak@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL -0.037 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
Subject: Re: [pve-devel] [PATCH storage v5 1/1] import: allow upload of
guest images files into import storage
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
Am 01.04.25 um 10:23 schrieb Dominik Csapak:
> so users can upload qcow2/raw/vmdk files directly in the ui
>
Pre-existing, but we put all uploads to /var/tmp/pveupload-XYZ first,
right? This already makes some users unhappy with ISOs IIRC and for
images we can expect it to get worse as those are usually even larger.
Should we at least show a warning/hint about this in the UI?
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> no changes in v5
>
> src/PVE/API2/Storage/Status.pm | 17 ++++++++++++++++-
> src/PVE/Storage.pm | 3 ++-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
> index c854b53..b23d283 100644
> --- a/src/PVE/API2/Storage/Status.pm
> +++ b/src/PVE/API2/Storage/Status.pm
The API method descriptions don't mention support for
uploading/downloading images yet.
> @@ -456,6 +456,7 @@ __PACKAGE__->register_method ({
>
> my $path;
> my $isOva = 0;
> + my $imageFormat;
Style nit: This is not how we usually name multi-word Perl variables
(also pre-existing for isOva).
>
> if ($content eq 'iso') {
> if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> @@ -472,7 +473,12 @@ __PACKAGE__->register_method ({
> raise_param_exc({ filename => "invalid filename or wrong extension" });
> }
Nit: if you already extract the extension from matching above here, you
don't need to match again below.
>
> - $isOva = 1;
> + if ($filename =~ m/\.ova$/) {
> + $isOva = 1;
> + } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
> + $imageFormat = $1;
> + }
> +
> $path = PVE::Storage::get_import_dir($cfg, $storage);
> } else {
> raise_param_exc({ content => "upload content type '$content' not allowed" });
> @@ -543,6 +549,9 @@ __PACKAGE__->register_method ({
>
> if ($isOva) {
> assert_ova_contents($tmpfilename);
> + } elsif (defined($imageFormat)) {
> + # checks untrusted image
> + PVE::Storage::file_size_info($tmpfilename, 10, $imageFormat, 1);
> }
> };
> if (my $err = $@) {
> @@ -667,6 +676,7 @@ __PACKAGE__->register_method({
>
> my $path;
> my $isOva = 0;
> + my $imageFormat;
>
> if ($content eq 'iso') {
> if ($filename !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) {
> @@ -685,6 +695,8 @@ __PACKAGE__->register_method({
>
Similar here regarding extension matching, then you don't even need to
define a second regex.
> if ($filename =~ m/\.ova$/) {
> $isOva = 1;
> + } elsif ($filename =~ m/${PVE::Storage::UPLOAD_IMPORT_IMAGE_EXT_RE_1}$/) {
> + $imageFormat = $1;
> }
>
> $path = PVE::Storage::get_import_dir($cfg, $storage);
> @@ -717,6 +729,9 @@ __PACKAGE__->register_method({
>
> if ($isOva) {
> assert_ova_contents($tmp_path);
> + } elsif (defined($imageFormat)) {
> + # checks untrusted image
> + PVE::Storage::file_size_info($tmp_path, 10, $imageFormat, 1);
> }
> };
>
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index c5d4ff8..09d9883 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -116,7 +116,8 @@ our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPR
>
> our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/;
>
> -our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova)/;
> +our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova|qcow2|raw|vmdk)/;
> +our $UPLOAD_IMPORT_IMAGE_EXT_RE_1 = qr/\.(qcow2|raw|vmdk)/;
>
> our $SAFE_CHAR_CLASS_RE = qr/[a-zA-Z0-9\-\.\+\=\_]/;
> our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = qr/[ a-zA-Z0-9\-\.\+\=\_]/;
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel