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 BCE481FF0AB for ; Wed, 23 Sep 2026 17:09:45 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 9AD40218B1; Wed, 23 Sep 2026 17:07:09 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH pve-storage v2 22/50] tree-wide: introduce parsing module and replace usages of ISO_EXT_RE_0 Date: Wed, 23 Sep 2026 17:05:36 +0200 Message-ID: <20260923150606.531239-23-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: 1790176012825 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.051 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_ADVERT4 0.75 This is probably an unwanted commercial email... 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: 3L3AMRCYSTVS4ONMHJYY3NATUPLPXLC7 X-Message-ID-Hash: 3L3AMRCYSTVS4ONMHJYY3NATUPLPXLC7 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: Introduce the `PVE::Storage::Common::Parse` module with the following subroutines: * `parse_rel_path_as_volname_parts($path, $vtype)` Parses a given file path into several smaller parts that constitute a volume name for the given volume type. * `parse_rel_path_as_volname($path, $vtype)` Parses a given file path directly into a volume name. * `parse_volname_as_parts($volname)` Parses an existing volume name into its constituent parts. * `parse_abs_path_as_volid_parts($storeid, $scfg, $path, $vtype)` Parses a given file path into several smaller parts that constitute a volume ID for the given storage ID, its configuration and volume type. * `parse_abs_path_as_volid($storeid, $scfg, $path, $vtype)` Like `parse_abs_path_as_volid_parts()`, but parses the given path directly into a volume ID. * `parse_volid_as_parts($volid)` Parses an existing volume ID into its storage ID and volume name parts. As of this commit, these parsing functions only support the 'iso' volume type, with the exception of `parse_volid_as_parts()`, which does not depend on knowing the volume type. Using the newly introduced parsing helpers, replace all occurrences of the `PVE::Storage::ISO_EXT_RE_0` regex across the repository. Since the `ISO_EXT_RE_0` regex is now completely unused, note its removal in `ApiChangeLog` and add a FIXME to note that it should be removed on the next APIAGE reset. Additional Notes Regarding the new Parsers ========================================== Support for other volume types will be added individually in future commits. The new *private* regex used for 'iso' vtype parsing is still matching file extensions case-insensitively, but also matches the entire path and file name portions of 'iso' file paths and volume names. These parts are extracted using named regex groups, as that is much easier to handle and keep track of mentally, even with smaller regexes. These named groups are the "constituent parts" that are returned by some of the new parser subroutines. However, one important difference here is that named regex groups do not support dashes `-` in their names, only underscores `_`. To keep things consistent with our style (using dashes instead of underscores in hash keys and the API), these named groups are formatted before being returned—underscores are simply substituted with dashes. Finally, all parser subroutines check whether a parent directory reference (`..` or "double dots") is contained in the passed or extracted file path, and return early if there is. This is an additional safety measure that is intentionally introduced in this commit to guard against any mishaps in the future as the parsers gain more functionality, such as supporting nested directories inside different volume types' subdirectories. Signed-off-by: Max R. Carrara --- ApiChangeLog | 9 + src/PVE/API2/Storage/Status.pm | 9 +- src/PVE/Storage.pm | 8 +- src/PVE/Storage/Common.pm | 2 + src/PVE/Storage/Common/Makefile | 1 + src/PVE/Storage/Common/Parse.pm | 389 ++++++++++++++++++++++++++++++++ src/PVE/Storage/Plugin.pm | 19 +- 7 files changed, 427 insertions(+), 10 deletions(-) create mode 100644 src/PVE/Storage/Common/Parse.pm diff --git a/ApiChangeLog b/ApiChangeLog index aefc0ff8..7f79efb5 100644 --- a/ApiChangeLog +++ b/ApiChangeLog @@ -28,6 +28,15 @@ Future changes should be documented in here. In order to make it clear that this is just a helper, it is replaced with the more explicit `PVE::Storage::Common::plugin_get_vtype_subdir()` subroutine. +* Remove the following regular expressions: + * `$PVE::Storage::ISO_EXT_RE_0` (`iso` volume type) + + These regular expressions were mostly used for parsing file paths and volume + names corresponding to their volume types, noted in parentheses above. + + Instead, The parsing functions in `PVE::Storage::Common::Parse` should be + used. + ## Version 15: * Add new `$snapname` parameter to the `volume_resize()` plugin method diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm index 45a79d56..310677d6 100644 --- a/src/PVE/API2/Storage/Status.pm +++ b/src/PVE/API2/Storage/Status.pm @@ -23,6 +23,9 @@ use PVE::Storage; use PVE::Storage::Common qw( plugin_get_vtype_subdir ); +use PVE::Storage::Common::Parse qw( + parse_rel_path_as_volname_parts +); use base qw(PVE::RESTHandler); @@ -73,11 +76,13 @@ my sub parse_transferred_file_path_extension : prototype($$) { my ($path, $vtype) = @_; if ($vtype eq 'iso') { - if ($path !~ m![^/]+$PVE::Storage::ISO_EXT_RE_0$!) { + my $parts = parse_rel_path_as_volname_parts($path, $vtype); + + if (!defined($parts)) { raise_param_exc({ filename => "wrong file extension" }); } - my $ext = $1; + my $ext = $parts->{ext}; return $ext; } diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 485bd822..5358be3f 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -26,6 +26,9 @@ use PVE::Storage::Common qw( plugin_get_default_vtype_subdirs plugin_get_vtype_subdir ); +use PVE::Storage::Common::Parse qw( + parse_abs_path_as_volid +); use PVE::RESTEnvironment qw(log_warn); use PVE::Storage::Plugin; @@ -117,6 +120,7 @@ PVE::Storage::Plugin->init(); # the following REs indicate the number or capture groups via the trailing digit # CAUTION don't forget to update the digits accordingly after messing with the capture groups +# FIXME: remove this regex on the next APIAGE reset. our $ISO_EXT_RE_0 = qr/\.(?:iso|img)/i; our $VZTMPL_EXT_RE_1 = qr/\.(?|(tar)(?!\.)|tar\.(gz|xz|zst|bz2))/i; @@ -754,9 +758,7 @@ sub path_to_volume_id { } if ($vtype eq 'iso') { - return if $filename !~ m!/([^/]+$ISO_EXT_RE_0)$!; - my $name = $1; - return "$sid:iso/$name"; + return parse_abs_path_as_volid($sid, $scfg, $path, $vtype); } if ($vtype eq 'vztmpl') { diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm index 4b200d9d..7efa279d 100644 --- a/src/PVE/Storage/Common.pm +++ b/src/PVE/Storage/Common.pm @@ -56,6 +56,8 @@ be grouped in a submodule can also be found here. =over +=item * C> + =back =head1 STANDARD OPTIONS FOR JSON SCHEMA diff --git a/src/PVE/Storage/Common/Makefile b/src/PVE/Storage/Common/Makefile index 0c4bba5b..0d9b1be1 100644 --- a/src/PVE/Storage/Common/Makefile +++ b/src/PVE/Storage/Common/Makefile @@ -1,4 +1,5 @@ SOURCES = \ + Parse.pm \ .PHONY: install diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm new file mode 100644 index 00000000..1141a715 --- /dev/null +++ b/src/PVE/Storage/Common/Parse.pm @@ -0,0 +1,389 @@ +package PVE::Storage::Common::Parse; + +use v5.36; + +use PVE::Storage::Common qw( + plugin_get_vtype_subdir +); + +use Exporter qw(import); + +our @EXPORT_OK = qw( + parse_rel_path_as_volname_parts + parse_rel_path_as_volname + parse_volname_as_parts + + parse_abs_path_as_volid_parts + parse_abs_path_as_volid + parse_volid_as_parts +); + +=head1 NAME + +C - Storage-related Parsing Functions + +=head1 DESCRIPTION + +This module contains various parsing functions for use within C>, +its submodules (including storage plugins) and other related modules. + +Parsing functions are categorized by their main purpose / area of application +and may be further subdivided depending on what kind of data type they are +primarily related to. + +=cut + +my $RE_PARENT_DIR = quotemeta('..'); +my $RE_CONTAINS_PARENT_DIR = qr! + ( ^$RE_PARENT_DIR/ ) # ../ --> Beginning of path + | + ( /$RE_PARENT_DIR/ ) # /../ --> Between two path components + | + ( /$RE_PARENT_DIR$ ) # /.. --> End of path +!xn; + +my $RE_ISO_FILE_PATH = qr! + (? + (? [^/]+ \. (? (?i: iso|img) ) ) + ) +!xn; + +my $RE_FILE_PATH_FOR_VTYPE = { + iso => qr/^$RE_ISO_FILE_PATH$/, +}; + +my $RE_VOLNAME_FOR_VTYPE = { + iso => qr/^$RE_ISO_FILE_PATH$/, +}; + +my sub contains_parent_dir($path) { + return $path =~ $RE_CONTAINS_PARENT_DIR; +} + +my sub strip_leading_path_separators($path) { + return $path =~ s!^/+!!r; +} + +my sub strip_trailing_path_separators($path) { + return $path =~ s!/+$!!r; +} + +my sub format_named_groups(%groups) { + my $result = {}; + + for my $old_key (keys %groups) { + my $new_key = $old_key =~ s/_/-/gr; + $result->{$new_key} = $groups{$old_key}; + } + + my @disk_path_components = (); + + if (defined($result->{file})) { + $result->{file} = strip_leading_path_separators($result->{file}); + push(@disk_path_components, $result->{file}); + } + + if (scalar(@disk_path_components)) { + $result->{'disk-path'} = join('/', @disk_path_components); + } + + return $result; +} + +my sub split_leading_dir_from_path($path, $directory) { + $directory = strip_trailing_path_separators($directory); + + if ("$directory/" eq substr($path, 0, length($directory) + 1, '')) { + return ($directory, $path); + } + + return; +} + +=head1 PARSERS RELATED TO VOLUMES + +The parsing functions in this section primarily deal with parsing data related +to storage volumes, primarily C>s and C>s. + +=head2 VOLUME NAMES + +All subroutines ending in C<_parts> in this section have a set of common +"parts" that they return. + +These are: + + { + # The name of the file the volume points to + file => 'custom-debian.iso', + + # The extension (suffix) of the file the volume points to + ext => 'iso', + + # The "real" path on disk + 'disk-path' => 'custom-debian.iso', + + # The "whole" path that the volume references, + path => 'custom-debian.iso', + + # The volume's type + vtype => 'iso', + + # The volname, which is "${vtype}/${path}" + volname => 'iso/custom-debian.iso', + } + +=cut + +=head3 parse_rel_path_as_volname_parts + +Parses the given relative file path C<$path> according to the given C<$vtype>, +returning a hashref containing the parts that make up a C on success. + +For example, C<$path = "custom-debian.iso"> and C<$vtype = "iso"> are turned +into: + + { + file => 'custom-debian.iso', + ext => 'iso', + 'disk-path' => 'custom-debian.iso', + path => 'custom-debian.iso', + vtype => 'iso', + volname => 'iso/custom-debian.iso', + } + +See L for more details. + +On failure or when the provided C<$vtype> is not supported or does not exist, +returns C in scalar context, and an empty list in list context. + +B This function assumes that C<$path> is already relative to the +directory that corresponds to the given C<$vtype>. + +If you want to parse an absolute path for an already known storage instead, see +C>>. + +For a counterpart to this function, see +C>>. + +=cut + +sub parse_rel_path_as_volname_parts : prototype($$) ($path, $vtype) { + return if contains_parent_dir($path); + + # TODO: vtype split: Handle parsing for 'images' and 'rootdir' vtypes. + + my $re_filepath = $RE_FILE_PATH_FOR_VTYPE->{$vtype}; + return if !defined($re_filepath); + + return if $path !~ $re_filepath; + + my $parts = format_named_groups(%+); + $parts->{vtype} = $vtype; + $parts->{volname} = $vtype . '/' . $parts->{path}; + + return $parts; +} + +=head3 parse_rel_path_as_volname + +Like C>>, but +instead of extracting the individual parts of the relative C<$path>, returns +the correctly formatted C directly. + +For example, C<$path = "custom-debian.iso"> and C<$vtype = "iso"> are turned +into C<"iso/custom-debian.iso">. + +For a counterpart to this function, see +C>>. + +=cut + +sub parse_rel_path_as_volname : prototype($$) ($path, $vtype) { + return if contains_parent_dir($path); + + # TODO: vtype split: Handle parsing for 'images' and 'rootdir' vtypes. + + my $re_filepath = $RE_FILE_PATH_FOR_VTYPE->{$vtype}; + return if !defined($re_filepath); + + return if $path !~ $re_filepath; + + return $vtype . '/' . $+{path}; +} + +=head3 parse_volname_as_parts + +Parses the provided C<$volname> and returns its constituent parts in a hashref +upon success. + +For example, C<$volname = "iso/custom-debian.iso"> is turned into: + + { + file => 'custom-debian.iso', + ext => 'iso', + 'disk-path' => 'custom-debian.iso', + path => 'custom-debian.iso', + vtype => 'iso', + volname => 'iso/custom-debian.iso', + } + +Returns C in scalar context and an empty list in list context, if +C<$volname> is prefixed with an unknown or unsupported C, or if the path +after the C prefix cannot be parsed. + +This function can be seen as a counterpart to +C>> and +C>> and can be used to +extract the information embedded within an already existing volume name, such +as file extensions or the name of the file that a volume refers to. + +=cut + +sub parse_volname_as_parts : prototype($) ($volname) { + # TODO: vtype split: Handle volname for 'images' and 'rootdir' vtypes. + my ($vtype, $path) = split('/', $volname, 2); + + # Either variable could be undef or an empty string here + return if !$vtype || !$path; + + return if contains_parent_dir($path); + + my $re_volname = $RE_VOLNAME_FOR_VTYPE->{$vtype}; + return if !defined($re_volname); + + return if $path !~ $re_volname; + + my $parts = format_named_groups(%+); + $parts->{vtype} = $vtype; + $parts->{volname} = $volname; + + return $parts; +} + +=head2 VOLUME IDS + +All subroutines ending in C<_parts> in this section have a set of common +"parts" that they return. Most of there parts are shared with the common parts +described in L, with the exception of C. + +These are: + + { + # See VOLUME NAMES + file => 'custom-debian.iso', + ext => 'iso', + 'disk-path' => 'custom-debian.iso', + path => 'custom-debian.iso', + vtype => 'iso', + volname => 'iso/custom-debian.iso', + + # Exclusive to volid parsing + volid => 'local:iso/custom-debian.iso', + } + +=cut + +my $RE_VOLID = qr! + ^ + (? + (? (?i: [a-z][a-z0-9\-\_\.]*[a-z0-9] ) ) + : # separated by colon + (? .+) + ) + $ +!xn; + +=head3 parse_abs_path_as_volid_parts + +Parses the given absolute file path C<$path> according to the given C<$vtype> +and returns a hashref containing the parts that make up a C<$volid> on success. +C<$path> must exist for the passed C<$storeid> and its config C<$scfg>. + +For example, C<$path = "/var/lib/vz/template/iso/custom-debian.iso"> and +C<$vtype = "iso"> on the C directory storage are turned into: + + { + # Identical to volume name parsing: + file => 'custom-debian.iso', + ext => 'iso', + 'disk-path' => 'custom-debian.iso', + path => 'custom-debian.iso', + vtype => 'iso', + volname => 'iso/custom-debian.iso', + + # Specific to volume ID parsers: + # "${storeid}:${volname}" + volid => 'local:iso/custom-debian.iso', + } + +See L for more details. + +On failure or when the provided C<$vtype> is not supported or does not exist, +returns C in scalar context, and an empty list in list context. + +If you are only interested in parsing the portion of the path belonging inside +the C directory independent of a C<$storeid>, see +C>>. + +For a counterpart to this function, see +C>>. + +=cut + +sub parse_abs_path_as_volid_parts : prototype($$$$) ($storeid, $scfg, $path, $vtype) { + my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype); + + my ($leading_dir, $remainder) = split_leading_dir_from_path($path, $vtype_subdir); + return if !defined($leading_dir); + + my $volid_parts = parse_rel_path_as_volname_parts($remainder, $vtype); + return if !defined($volid_parts); + + $volid_parts->{volid} = $storeid . ':' . $volid_parts->{volname}; + + return $volid_parts; +} + +=head3 parse_abs_path_as_volid + +Like C>>, but +instead of extracting the individual parts of the C<$path>, returns the +correctly formatted C directly. + +For a counterpart to this function, see +C>>. + +=cut + +sub parse_abs_path_as_volid : prototype($$$$) ($storeid, $scfg, $path, $vtype) { + my $vtype_subdir = plugin_get_vtype_subdir($scfg, $vtype); + + my ($leading_dir, $remainder) = split_leading_dir_from_path($path, $vtype_subdir); + return if !defined($leading_dir); + + my $volname = parse_rel_path_as_volname($remainder, $vtype); + return if !defined($volname); + + return $storeid . ':' . $volname; +} + +=head3 parse_volid_as_parts + +Parses the provided C<$volid> and returns its C and C parts +in a hashref. + +On failure, returns C in scalar context, and an empty list in list context. + +The C part of volumes that represent files can be further parsed into +its constituent parts using the +C>> function. + +=cut + +sub parse_volid_as_parts : prototype($) ($volid) { + return if $volid !~ $RE_VOLID; + + return format_named_groups(%+); +} + +1; diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm index 6a113dcb..d4860344 100644 --- a/src/PVE/Storage/Plugin.pm +++ b/src/PVE/Storage/Plugin.pm @@ -19,6 +19,10 @@ use PVE::Storage::Common qw( plugin_get_default_vtype_subdirs plugin_get_vtype_subdir ); +use PVE::Storage::Common::Parse qw( + parse_volname_as_parts + parse_abs_path_as_volid_parts +); use JSON; @@ -821,8 +825,12 @@ sub parse_volname { return ('images', $name, $vmid, undef, undef, $isBase, $format); } - if ($volname =~ m!^iso/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!) { - return ('iso', $1, undef, undef, undef, undef, 'raw'); + if (defined(my $parts = parse_volname_as_parts($volname))) { + my ($vtype, $volume_path) = $parts->@{qw(vtype path)}; + + if ($vtype eq 'iso') { + return ($vtype, $volume_path, undef, undef, undef, undef, 'raw'); + } } if ($volname =~ m!^vztmpl/([^/]+$PVE::Storage::VZTMPL_EXT_RE_1)$!) { @@ -1716,11 +1724,12 @@ my sub get_subdir_files { } if ($vtype eq 'iso') { - return if $filename !~ m!/([^/]+$PVE::Storage::ISO_EXT_RE_0)$!i; + my $parts = parse_abs_path_as_volid_parts($storeid, $scfg, $path, $vtype); + return if !defined($parts); return { - volid => "$storeid:iso/$1", - format => 'iso', + volid => $parts->{volid}, + format => 'iso', # always 'iso' even if we have a file ending in .img }; } -- 2.47.3