Re: [PATCH pve-storage v1 27/54] tree-wide: partially replace usages of regexes for 'import' vtype

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Wed, Apr 22, 2026 at 01:12:53PM +0200, Max R. Carrara wrote:
> Add partial support for the 'import' volume type in
> `PVE::Storage::Common::Parse`. What remains unsupported right now is
> parsing volume names for that type, as those require a little more
> care.
> 
> Replace most usages of the `PVE::Storage::IMPORT_EXT_RE_1` regex with
> parsing functions from `::Common::Parse`.
> 
> Replace the one remaining spot where we use the
> `PVE::Storage::UPLOAD_IMPORT_EXT_RE_1` regex with a parsing function
> as well. This regex should now be phased out in a future APIVER +
> APIAGE bump, like the others in previous commits.
> 
> Note that the `UPLOAD_IMPORT_EXT_RE_1` regex only exists to handle the
> special case of excluding .ovf files from the upload / download_url
> API methods. Instead of adding a separate parser (or a flag etc.) to
> handle this case, just parse the path of the up-/downloaded file
> regardless. Then, raise a parameter exception when the `ovf` file
> extension is encountered.
> 
> Also add a bunch of test cases that target the `import` volume type
> for the `list_volumes()` plugin API method. For all of these tests,
> sort the expected and resulting list items by their volume ID in order
> to make the output comparison deterministic. Iterate over the declared
> content / volume types of the mocked storage config instead of using a
> fixed list of vtypes as well.
> 
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
>  src/PVE/API2/Storage/Status.pm  |  13 ++-
>  src/PVE/Storage.pm              |   4 +-
>  src/PVE/Storage/Common/Parse.pm |  13 +++
>  src/PVE/Storage/Plugin.pm       |   9 +-
>  src/test/list_volumes_test.pm   | 180 +++++++++++++++++++++++++++++++-
>  5 files changed, 203 insertions(+), 16 deletions(-)
> 
> diff --git a/src/PVE/API2/Storage/Status.pm b/src/PVE/API2/Storage/Status.pm
> index 03929d26..b578a1ed 100644
> --- a/src/PVE/API2/Storage/Status.pm
> +++ b/src/PVE/API2/Storage/Status.pm
> @@ -96,13 +96,18 @@ my sub parse_transferred_file_path_extension : prototype($$) {
>      }
> 
>      if ($vtype eq 'import') {
> -        if (
> -            $path !~ m!${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::UPLOAD_IMPORT_EXT_RE_1$!
> -        ) {
> +        my $parts = parse_path_as_volname_parts($path, $vtype);
> +
> +        if (!defined($parts)) {
>              raise_param_exc({ filename => "invalid filename or wrong extension" });
>          }
> 
> -        my $ext = $1;
> +        my $ext = $parts->{ext};
> +
> +        if ($ext eq 'ovf') {
> +            raise_param_exc({ filename => "wrong file extension" });
> +        }
> +
>          return $ext;
>      }
> 
> diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
> index 20b7bafa..3a716894 100755
> --- a/src/PVE/Storage.pm
> +++ b/src/PVE/Storage.pm
> @@ -773,9 +773,7 @@ sub path_to_volume_id {
>          }
> 
>          if ($vtype eq 'import') {
> -            return if $filename !~ m!/(${SAFE_CHAR_CLASS_RE}+${IMPORT_EXT_RE_1})$!;
> -            my $name = $1;
> -            return "$sid:import/$name";
> +            return parse_path_as_volid($sid, $scfg, $path, $vtype);
>          }
> 
>          return;
> diff --git a/src/PVE/Storage/Common/Parse.pm b/src/PVE/Storage/Common/Parse.pm
> index 33220758..bd7cec13 100644
> --- a/src/PVE/Storage/Common/Parse.pm
> +++ b/src/PVE/Storage/Common/Parse.pm
> @@ -37,6 +37,8 @@ my @VZTMPL_COMPRESSION_EXTENSIONS = ('gz', 'xz', 'zst', 'bz2');
> 
>  my @BACKUP_COMPRESSION_EXTENSIONS = ('gz', 'lzo', 'zst', 'bz2');
> 
> +my @IMPORT_EXTENSIONS = ('ova', 'ovf', 'qcow2', 'raw', 'vmdk');
> +
>  my sub join_to_re_alternations(@list) {
>      return join('|', map { quotemeta } @list);
>  }
> @@ -45,6 +47,10 @@ my $RE_VZTMPL_COMPRESSION_EXTENSIONS = join_to_re_alternations(@VZTMPL_COMPRESSI
> 
>  my $RE_BACKUP_COMPRESSION_EXTENSIONS = join_to_re_alternations(@BACKUP_COMPRESSION_EXTENSIONS);
> 
> +my $RE_IMPORT_EXTENSIONS = join_to_re_alternations(@IMPORT_EXTENSIONS);
> +
> +my $RE_SAFE_CHAR_CLASS = qr/[a-zA-Z0-9\-\.\+\=\_]/;
> +
>  my $RE_PARENT_DIR = quotemeta('..');
>  my $RE_CONTAINS_PARENT_DIR = qr!
>      ( ^$RE_PARENT_DIR/ )  #  ../ --> Beginning of path
> @@ -112,11 +118,18 @@ my $RE_SNIPPETS_FILE_PATH = qr!
>      )
>  !xn;
> 
> +my $RE_IMPORT_FILE_PATH = qr!
> +    (?<path>
> +        (?<file> ($RE_SAFE_CHAR_CLASS)+ \. (?<ext> $RE_IMPORT_EXTENSIONS) )

This misses an `(?i)`.

After more checking, so does `ext_compression` from the previous `BACKUP_EXT_RE_2` patch.

Perhaps there should be a `join_to_insensitive_alterations()` helper
which generally wraps the alterations inside `(?i:…)` used for all the
file extension regexes?

> +    )
> +!xn;
> +
>  my $RE_FILE_PATH_FOR_VTYPE = {
>      iso => qr/^$RE_ISO_FILE_PATH$/,
>      vztmpl => qr/^$RE_VZTMPL_FILE_PATH$/,
>      backup => qr/^$RE_BACKUP_FILE_PATH$/,
>      snippets => qr/^$RE_SNIPPETS_FILE_PATH$/,
> +    import => qr/^$RE_IMPORT_FILE_PATH$/,
>  };
> 
>  my $RE_VOLNAME_FOR_VTYPE = {
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index b08e038b..39e5cb0d 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -1768,13 +1768,12 @@ my sub get_subdir_files {
>          }
> 
>          if ($vtype eq 'import') {
> -            return
> -                if $filename !~
> -                m!/(${PVE::Storage::SAFE_CHAR_CLASS_RE}+$PVE::Storage::IMPORT_EXT_RE_1)$!i;

(to-the-above-relevant /i (`m!…!i`) for context)

> +            my $parts = parse_path_as_volid_parts($storeid, $scfg, $path, $vtype);
> +            return if !defined($parts);