From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 6DDC21FF0AB for ; Wed, 23 Sep 2026 17:11:09 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8778E21753; Wed, 23 Sep 2026 17:10:01 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v2 28/50] tree-wide: replace remaining usages of regexes for 'import' vtype Date: Wed, 23 Sep 2026 17:05:42 +0200 Message-ID: <20260923150606.531239-29-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260923150606.531239-1-m.carrara@proxmox.com> References: <20260923150606.531239-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790176025427 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.413 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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: TS3CEG3SZ55ZIGSMXHPQW2RBUMGVOBW2 X-Message-ID-Hash: TS3CEG3SZ55ZIGSMXHPQW2RBUMGVOBW2 X-MailFrom: m.carrara@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: Implement the remaining pieces that were missing from fully supporting the 'import' volume type in `PVE::Storage::Common::Parse`. In particular, parsing volume names for '.ova' files that refer to contents inside the '.ova' file is now fully supported. Also add corresponding test cases to 'parser_tests.pl'. Replace the remaining usages of the `PVE::Storage::IMPORT_EXT_RE_1` and `PVE::Storage::OVA_CONTENT_RE_1` with parsing functions from `::Common::Parse` across the repository. As before with other regexes, mark their removal in `ApiChangeLog` and add corresponding FIXME comments for a future APIAGE reset. Note that the logic in `PVE::Storage::Plugin::parse_volname` can now be simplified a little further, since the `parse_volname_as_parts()` parsing function also handles extracting the inner and outer file extensions, amongst other parts. Since the only remaining usage of the `PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE` regex is now in `PVE::GuestImport::OVF`, handle it the same way as the removed `SAFE_CHAR_CLASS_RE`. This means that we move it into `::Common::Parse`, make it private, and only expose it through a helper subroutine. Also note this in `ApiChangeLog` and add a corresponding FIXME comment once again. This allows us to stop using it in `PVE::GuestImport::OVF` as well, where we instead add a small helper sub to aid us in parsing the obtained file path. Finally, update the test cases in 'guest_import_test.pl' that match for specific parsing-related exception messages. Signed-off-by: Max R. Carrara --- ApiChangeLog | 3 + src/PVE/GuestImport.pm | 28 +- src/PVE/GuestImport/OVF.pm | 24 +- src/PVE/Storage.pm | 7 +- src/PVE/Storage/Common/Parse.pm | 81 ++++++ src/PVE/Storage/Common/test/parser_tests.pl | 282 ++++++++++++++++++++ src/PVE/Storage/Plugin.pm | 20 +- src/test/guest_import_test.pl | 6 +- 8 files changed, 417 insertions(+), 34 deletions(-) diff --git a/ApiChangeLog b/ApiChangeLog index b1b6b88e..30dd0374 100644 --- a/ApiChangeLog +++ b/ApiChangeLog @@ -32,7 +32,9 @@ Future changes should be documented in here. * `$PVE::Storage::ISO_EXT_RE_0` (`iso` volume type) * `$PVE::Storage::VZTMPL_EXT_RE_1` (`vztmpl` volume type) * `$PVE::Storage::BACKUP_EXT_RE_2` (`backup` volume type) + * `$PVE::Storage::IMPORT_EXT_RE_1` (`import` volume type) * `$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1` (`import` volume type) + * `$PVE::Storage::OVA_CONTENT_RE_1` (`import` volume type) These regular expressions were mostly used for parsing file paths and volume names corresponding to their volume types, noted in parentheses above. @@ -42,6 +44,7 @@ Future changes should be documented in here. * Migrate the following regular expressions: * `$PVE::Storage::SAFE_CHAR_CLASS_RE` + * `$PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE` These regular expressions are now defined as private constants in the `PVE::Storage::Common::Parse` module and can be obtained by using the diff --git a/src/PVE/GuestImport.pm b/src/PVE/GuestImport.pm index 3d59dcd7..3be84fc9 100644 --- a/src/PVE/GuestImport.pm +++ b/src/PVE/GuestImport.pm @@ -6,6 +6,9 @@ use warnings; use File::Path; use PVE::Storage; +use PVE::Storage::Common::Parse qw( + parse_volname_as_parts +); use PVE::Tools qw(run_command); sub extract_disk_from_import_file { @@ -13,31 +16,26 @@ sub extract_disk_from_import_file { my ($source_storeid, $volname) = PVE::Storage::parse_volume_id($volid); $target_storeid //= $source_storeid; - my $cfg = PVE::Storage::config(); - my ($vtype, $name, undef, undef, undef, undef, $fmt) = - PVE::Storage::parse_volname($cfg, $volid); + my $parts = parse_volname_as_parts($volname); + die "cannot extract $volid - invalid volname $volname\n" + if !defined($parts); + + my ($vtype, $outer_fmt) = $parts->@{qw(vtype ext)}; die "only files with content type 'import' can be extracted\n" if $vtype ne 'import'; die "only files from 'ova' format can be extracted\n" - if $fmt !~ m/^ova\+/; + if $outer_fmt ne 'ova'; - # extract the inner file from the name - my $archive_volid; - my $inner_file; - my $inner_fmt; - if ($name =~ m!^(.*\.ova)/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+)$!) { - $archive_volid = "$source_storeid:import/$1"; - $inner_file = $2; - ($inner_fmt) = $fmt =~ /^ova\+(.*)$/; - } else { - die "cannot extract $volid - invalid volname $volname\n"; - } + my $archive_volid = $source_storeid . ':' . $vtype . '/' . $parts->{'disk-path'}; + my $inner_file = $parts->{'content-file'}; + my $inner_fmt = $parts->{'content-ext'}; die "cannot determine format of '$volid'\n" if !$inner_fmt; + my $cfg = PVE::Storage::config(); my $ova_path = PVE::Storage::path($cfg, $archive_volid); my $tmpdir = PVE::Storage::get_image_dir($cfg, $target_storeid, $vmid); diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm index 8999cab4..ed76b8ff 100644 --- a/src/PVE/GuestImport/OVF.pm +++ b/src/PVE/GuestImport/OVF.pm @@ -12,6 +12,9 @@ use Cwd 'realpath'; use PVE::Tools; use PVE::Storage; +use PVE::Storage::Common::Parse qw( + get_re_safe_char_with_whitespace_class +); use PVE::Seccomp qw(stmt /^BPF_/); # map OVF resources types to descriptive strings @@ -184,6 +187,22 @@ my sub read_ovf_file { return $raw; } +my $RE_SAFE_FILEPATH = qr/ + ^ (? (${get_re_safe_char_with_whitespace_class()}+) ) $ +/xn; + +my sub parse_filepath_from_xpath { + my ($filepath) = @_; + + # untaint & check no sub/parent dirs + $filepath =~ $RE_SAFE_FILEPATH; + $filepath = $+{filepath}; + + return if !$filepath || $filepath eq '.' || $filepath eq '..'; + + return $filepath; +} + # returns two references, $qm which holds qm.conf style key/values, and \@disks my sub parse_ovf_do { my ($ovf, $ovf_data, $isOva, $debug) = @_; @@ -357,9 +376,8 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id, } print "file path: $filepath\n" if $debug; my $original_filepath = $filepath; - ($filepath) = $filepath =~ m|^(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+)$|; # untaint & check no sub/parent dirs - die "referenced path '$original_filepath' is invalid\n" - if !$filepath || $filepath eq "." || $filepath eq ".."; + $filepath = parse_filepath_from_xpath($filepath); + die "referenced path '$original_filepath' is invalid\n" if !$filepath; push @disks, { diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index ce90f942..98d5f64e 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -28,6 +28,7 @@ use PVE::Storage::Common qw( ); use PVE::Storage::Common::Parse qw( get_re_safe_char_class + get_re_safe_char_with_whitespace_class parse_abs_path_as_volid ); use PVE::RESTEnvironment qw(log_warn); @@ -130,6 +131,7 @@ our $VZTMPL_EXT_RE_1 = qr/\.(?|(tar)(?!\.)|tar\.(gz|xz|zst|bz2))/i; # FIXME: remove this regex on the next APIAGE reset. our $BACKUP_EXT_RE_2 = qr/\.(tgz|(?:tar|vma)(?:\.(${\PVE::Storage::Plugin::COMPRESSOR_RE}))?)/; +# FIXME: remove this regex on the next APIAGE reset. our $IMPORT_EXT_RE_1 = qr/\.(ova|ovf|qcow2|raw|vmdk)/; # FIXME: remove this regex on the next APIAGE reset. @@ -137,8 +139,11 @@ our $UPLOAD_IMPORT_EXT_RE_1 = qr/\.(ova|qcow2|raw|vmdk)/; # FIXME: remove this regex on the next APIAGE reset. our $SAFE_CHAR_CLASS_RE = get_re_safe_char_class(); -our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = qr/[ a-zA-Z0-9\-\.\+\=\_]/; +# FIXME: remove this regex on the next APIAGE reset. +our $SAFE_CHAR_WITH_WHITESPACE_CLASS_RE = get_re_safe_char_with_whitespace_class(); + +# FIXME: remove this regex on the next APIAGE reset. our $OVA_CONTENT_RE_1 = qr/${SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.(qcow2|raw|vmdk)/; # FIXME remove with PVE 9.0, add versioned breaks for pve-manager diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm index 2193aa65..296a6c4d 100644 --- a/src/PVE/Storage/Common/Parse.pm +++ b/src/PVE/Storage/Common/Parse.pm @@ -10,6 +10,7 @@ use Exporter qw(import); our @EXPORT_OK = qw( get_re_safe_char_class + get_re_safe_char_with_whitespace_class parse_rel_path_as_volname_parts parse_rel_path_as_volname @@ -41,6 +42,8 @@ my @BACKUP_COMPRESSION_EXTENSIONS = ('gz', 'lzo', 'zst', 'bz2'); my @IMPORT_EXTENSIONS = ('ova', 'ovf', 'qcow2', 'raw', 'vmdk'); +my @OVA_CONTENT_EXTENSIONS = ('qcow2', 'raw', 'vmdk'); + my sub join_to_re_alternations(@list) { return join('|', map { quotemeta } @list); } @@ -51,8 +54,12 @@ my $RE_BACKUP_COMPRESSION_EXTENSIONS = join_to_re_alternations(@BACKUP_COMPRESSI my $RE_IMPORT_EXTENSIONS = join_to_re_alternations(@IMPORT_EXTENSIONS); +my $RE_OVA_CONTENT_EXTENSIONS = join_to_re_alternations(@OVA_CONTENT_EXTENSIONS); + my $RE_SAFE_CHAR_CLASS = qr/[a-zA-Z0-9\-\.\+\=\_]/; +my $RE_SAFE_CHAR_WITH_WHITESPACE_CLASS = qr/[ a-zA-Z0-9\-\.\+\=\_]/; + my $RE_PARENT_DIR = quotemeta('..'); my $RE_CONTAINS_PARENT_DIR = qr! ( ^$RE_PARENT_DIR/ ) # ../ --> Beginning of path @@ -126,6 +133,42 @@ my $RE_IMPORT_FILE_PATH = qr! ) !xn; +my $RE_OVA_CONTENT = qr! + (? + ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+ \. (? $RE_OVA_CONTENT_EXTENSIONS) + ) +!xn; + +# NOTE: Volume names with the 'import' vtype are treated differently when +# they do not stem from a file path directly - see comments inline. +my $RE_IMPORT_VOLNAME_OVA_FILE_WITH_CONTENT = qr! + (? + # NOTE: Unlike in RE_IMPORT_FILE_PATH, we allow whitespace here + ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+ + \. + (? ova) + ) / (? $RE_OVA_CONTENT) +!xn; + +my $RE_IMPORT_VOLNAME_REGULAR_FILE = qr! + (? + # NOTE: Unlike in RE_IMPORT_FILE_PATH, we allow whitespace here + ($RE_SAFE_CHAR_WITH_WHITESPACE_CLASS)+ + \. + (? $RE_IMPORT_EXTENSIONS) + ) +!xn; + +my $RE_IMPORT_VOLNAME = qr! + (? + # NOTE: Order here matters - the ova+content regex is stricter, + # so try this branch here first + ($RE_IMPORT_VOLNAME_OVA_FILE_WITH_CONTENT) + | + ($RE_IMPORT_VOLNAME_REGULAR_FILE) + ) +!xn; + my $RE_FILE_PATH_FOR_VTYPE = { iso => qr/^$RE_ISO_FILE_PATH$/, vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/, @@ -139,6 +182,9 @@ my $RE_VOLNAME_FOR_VTYPE = { vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/, backup => qr/^$RE_BACKUP_FILE_PATH$/, snippets => qr/^$RE_SNIPPETS_FILE_PATH$/, + + # special cases - not reusing file path regexes: + import => qr/^$RE_IMPORT_VOLNAME$/, }; my sub contains_parent_dir($path) { @@ -223,6 +269,17 @@ sub get_re_safe_char_class() { return $RE_SAFE_CHAR_CLASS; } +=head3 get_re_safe_char_with_whitespace_class + +Like C>>, but the +returned character class also allows a space character (U+0020). + +=cut + +sub get_re_safe_char_with_whitespace_class() { + return $RE_SAFE_CHAR_WITH_WHITESPACE_CLASS; +} + =head1 PARSERS RELATED TO VOLUMES The parsing functions in this section primarily deal with parsing data related @@ -302,6 +359,30 @@ by Proxmox VE, as well as C, C (optional): volname => 'backup/vzdump-qemu-16110-2020_03_30-21_12_45.vma.lzo', } +=item C + +B Only for C>>! + +C volnames may additionally refer to the C inside an OVA file, +which is further differentiated into C and C: + + { + file => 'import.ova', + ext => 'ova', + 'disk-path' => 'import.ova', + path => 'import.ova/disk.raw', + + # Exclusive to 'import' volume names + content => 'disk.raw', + + # Used to further differentiate 'content' + 'content-file' => 'disk.raw', + 'content-ext' => 'raw', + + vtype => 'import', + volname => 'import/import.ova/disk.raw', + } + =back =cut diff --git a/src/PVE/Storage/Common/test/parser_tests.pl b/src/PVE/Storage/Common/test/parser_tests.pl index 137b91b8..2869d3f0 100755 --- a/src/PVE/Storage/Common/test/parser_tests.pl +++ b/src/PVE/Storage/Common/test/parser_tests.pl @@ -396,17 +396,104 @@ my $volname_cases_snippets_valid = [ }, ]; +my $volname_cases_import_valid = [ + { + path => 'import.ova', + expected => { + file => 'import.ova', + ext => 'ova', + 'disk-path' => 'import.ova', + path => 'import.ova', + vtype => 'import', + volname => 'import/import.ova', + }, + }, + { + path => 'import.ovf', + expected => { + file => 'import.ovf', + ext => 'ovf', + 'disk-path' => 'import.ovf', + path => 'import.ovf', + vtype => 'import', + volname => 'import/import.ovf', + }, + }, + { + path => 'disk-0.qcow2', + expected => { + file => 'disk-0.qcow2', + ext => 'qcow2', + 'disk-path' => 'disk-0.qcow2', + path => 'disk-0.qcow2', + vtype => 'import', + volname => 'import/disk-0.qcow2', + }, + }, + { + path => 'raw_disk_v2.5.raw', + expected => { + file => 'raw_disk_v2.5.raw', + ext => 'raw', + 'disk-path' => 'raw_disk_v2.5.raw', + path => 'raw_disk_v2.5.raw', + vtype => 'import', + volname => 'import/raw_disk_v2.5.raw', + }, + }, + { + path => 'disk-1+data.vmdk', + expected => { + file => 'disk-1+data.vmdk', + ext => 'vmdk', + 'disk-path' => 'disk-1+data.vmdk', + path => 'disk-1+data.vmdk', + vtype => 'import', + volname => 'import/disk-1+data.vmdk', + }, + }, +]; + +my $volname_cases_import_invalid = [ + { + description => "Invalid file extension (import)", + args => { + path => 'import.zip', + vtype => 'import', + }, + expected => undef, + }, + { + description => "Uppercase letter in file extension (import)", + args => { + path => 'import.Ova', + vtype => 'import', + }, + expected => undef, + }, + { + description => "Unsafe characters in file name (import)", + args => { + path => '🐪perl-playground🐪.ova', + vtype => 'import', + }, + expected => undef, + }, +]; + my $cases_valid_all = [ $volname_cases_iso_valid, $volname_cases_vztmpl_valid, $volname_cases_backup_valid, $volname_cases_snippets_valid, + $volname_cases_import_valid, ]; my $cases_invalid_all = [ $volname_cases_iso_invalid, $volname_cases_vztmpl_invalid, $volname_cases_backup_invalid, + $volname_cases_import_invalid, ]; { @@ -475,6 +562,179 @@ my sub run_volname_parsing_tests : prototype($) ($volname_tests) { return; } +# NOTE: These run for parse_rel_path_as_volname_parts() and parse_rel_path_as_volname() only! +my $cases_import_special_file_path = [ + { + description => "Whitespace not allowed in volume file paths (import)", + args => { + path => 'Some Disk.qcow2', + vtype => 'import', + }, + expected => undef, + }, + { + description => "Volume file path with disallowed OVA content (raw) (import)", + args => { + path => 'import.ova/disk.raw', + vtype => 'import', + }, + expected => undef, + }, + { + description => "Volume file path with disallowed OVA content (qcow2) (import)", + args => { + path => 'import.ova/disk.qcow2', + vtype => 'import', + }, + expected => undef, + }, + { + description => "Volume file path with disallowed OVA content (vmdk) (import)", + args => { + path => 'import.ova/disk.vmdk', + vtype => 'import', + }, + expected => undef, + }, +]; + +# NOTE: These run for parse_volname_as_parts() only! +my $cases_import_special_volname = [ + { + description => "Whitespace allowed in volume names (import)", + args => { + volname => 'import/Some Disk.qcow2', + }, + expected => { + file => 'Some Disk.qcow2', + ext => 'qcow2', + 'disk-path' => 'Some Disk.qcow2', + path => 'Some Disk.qcow2', + vtype => 'import', + volname => 'import/Some Disk.qcow2', + }, + }, + { + description => "Volume name with OVA content (raw) (import)", + args => { + volname => 'import/import.ova/disk.raw', + }, + expected => { + file => 'import.ova', + ext => 'ova', + 'disk-path' => 'import.ova', + path => 'import.ova/disk.raw', + content => 'disk.raw', + 'content-file' => 'disk.raw', + 'content-ext' => 'raw', + vtype => 'import', + volname => 'import/import.ova/disk.raw', + }, + }, + { + description => "Volume name with OVA content (qcow2) (import)", + args => { + volname => 'import/import.ova/disk.qcow2', + }, + expected => { + file => 'import.ova', + ext => 'ova', + 'disk-path' => 'import.ova', + path => 'import.ova/disk.qcow2', + content => 'disk.qcow2', + 'content-file' => 'disk.qcow2', + 'content-ext' => 'qcow2', + vtype => 'import', + volname => 'import/import.ova/disk.qcow2', + }, + }, + { + description => "Volume name with OVA content (vmdk) (import)", + args => { + volname => 'import/import.ova/disk.vmdk', + }, + expected => { + file => 'import.ova', + ext => 'ova', + 'disk-path' => 'import.ova', + path => 'import.ova/disk.vmdk', + content => 'disk.vmdk', + 'content-file' => 'disk.vmdk', + 'content-ext' => 'vmdk', + vtype => 'import', + volname => 'import/import.ova/disk.vmdk', + }, + }, + { + description => "Whitespace in volume name + OVA content (import)", + args => { + volname => 'import/Some Import.ova/disk.qcow2', + }, + expected => { + file => 'Some Import.ova', + ext => 'ova', + 'disk-path' => 'Some Import.ova', + path => 'Some Import.ova/disk.qcow2', + content => 'disk.qcow2', + 'content-file' => 'disk.qcow2', + 'content-ext' => 'qcow2', + vtype => 'import', + volname => 'import/Some Import.ova/disk.qcow2', + }, + }, +]; + +my sub run_special_import_volname_parsing_tests : prototype() () { + for my $case ($cases_import_special_file_path->@*) { + subtest $case->{description} => sub () { + my ($path, $vtype) = $case->{args}->@{qw(path vtype)}; + + my $got_volname_parts = parse_rel_path_as_volname_parts($path, $vtype); + my $got_volname = parse_rel_path_as_volname($path, $vtype); + + if (defined($case->{expected})) { + eq_or_diff( + $got_volname_parts, + $case->{expected}, + 'parse_rel_path_as_volname_parts() returns expected hashref', + { context => 50000 }, + ); + + is( + $got_volname, + $case->{expected}->{volname}, + 'parse_rel_path_as_volname() returns expected volname', + ); + } else { + is($got_volname_parts, undef, + 'parse_rel_path_as_volname_parts() returns undef'); + is($got_volname, undef, 'parse_rel_path_as_volname() returns undef'); + } + }; + } + + for my $case ($cases_import_special_volname->@*) { + subtest $case->{description} => sub () { + my ($volname) = $case->{args}->@{qw(volname)}; + + my $got_volname = parse_volname_as_parts($volname); + + if (defined($case->{expected})) { + eq_or_diff( + $got_volname, + $case->{expected}, + 'parse_volname_as_parts() returns expected hashref', + { context => 50000 }, + ); + } else { + is($got_volname, undef, 'parse_volname_as_parts() returns undef'); + } + }; + } + + return; +} + my $DEFAULT_STOREID = 'local'; my $DEFAULT_STORAGE_PATH = File::Temp->newdir(); my $DEFAULT_SCFG = { @@ -547,6 +807,26 @@ my $volid_tests = [ $volname_tests->@* ], }, + { + description => + "volid parsers build on volname parsers' behaviors - 'import' vtype file paths (1)", + storedid => $DEFAULT_STOREID, + scfg => $DEFAULT_SCFG, + cases => [ + map { format_volname_case_to_volid_case($DEFAULT_STOREID, $DEFAULT_SCFG, $_) } + $cases_import_special_file_path->@* + ], + }, + { + description => + "volid parsers build on volname parsers' behaviors - 'import' vtype file paths (2)", + storedid => $ALT_STOREID, + scfg => $ALT_SCFG, + cases => [ + map { format_volname_case_to_volid_case($DEFAULT_STOREID, $DEFAULT_SCFG, $_) } + $cases_import_special_file_path->@* + ], + }, ]; my sub run_volid_parsing_tests : prototype($) ($volid_tests) { @@ -606,6 +886,8 @@ my sub main() { unified_diff(); run_volname_parsing_tests($volname_tests); + run_special_import_volname_parsing_tests(); + run_volid_parsing_tests($volid_tests); done_testing(); diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index afddca73..45c0b838 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -845,20 +845,16 @@ sub parse_volname { if ($vtype eq 'snippets') { return ($vtype, $volume_path, undef, undef, undef, undef, 'raw'); } - } - if ($volname =~ - m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+\.ova\/${PVE::Storage::OVA_CONTENT_RE_1})$! - ) { - my $packed_image = $1; - my $format = $2; - return ('import', $packed_image, undef, undef, undef, undef, "ova+$format"); - } + if ($vtype eq 'import') { + my $format = $parts->{ext}; - if ($volname =~ - m!^import/(${PVE::Storage::SAFE_CHAR_WITH_WHITESPACE_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$! - ) { - return ('import', $1, undef, undef, undef, undef, $2); + if (defined(my $content_ext = $parts->{'content-ext'})) { + $format .= "+$content_ext"; + } + + return ($vtype, $volume_path, undef, undef, undef, undef, $format); + } } die "unable to parse directory volume name '$volname'\n"; diff --git a/src/test/guest_import_test.pl b/src/test/guest_import_test.pl index 04eec24d..234013e3 100755 --- a/src/test/guest_import_test.pl +++ b/src/test/guest_import_test.pl @@ -357,7 +357,7 @@ my $tests = [ { volname => 'import/some-import.ova/disk.txt', vmid => 1337, - 'expect-fail' => qr/unable to parse directory volume name/, + 'expect-fail' => qr/invalid volname/, }, ], }, @@ -532,7 +532,7 @@ my $tests = [ { volname => 'import/some-🐧-import.ova/disk.qcow2', vmid => 1337, - 'expect-fail' => qr/unable to parse directory volume name/, + 'expect-fail' => qr/invalid volname/, }, ], }, @@ -619,7 +619,7 @@ my $tests = [ { volname => 'import/some-import.ova', vmid => 1337, - 'expect-fail' => qr/only files from 'ova' format can be extracted/, + 'expect-fail' => qr/cannot determine format of/, }, ], }, -- 2.47.3