On 22.08.19 14:39, Stefan Reiter wrote:
> To maintain full (backwards) compatibility, leave the type name as
> 'iso' - this makes this patch work without changing every consumer of
> storage APIs.
>
> Note that currently these files can only be attached as a CDROM/DVD
> drive, so USB-only images can be uploaded but might not work in VMs.
>
> Signed-off-by: Stefan Reiter <s.rei...@proxmox.com>
> ---
>
> v1 -> v2:
> * Refactored regex into variable
>
> I left the [Ii][Ss]... since we don't know if the variable is used with //i
> or not.
1. used my instead of our scope, but used the variable in another module,
that cannot work... Now it was poassed a undefined regex value to all
out-of-module users and thus the ending wasn't checked at all anymore,
effectively allowing all endings..
2. You could just added the case insensitive flag to the base regex?
perl -we 'my $re = qr/foo/i; "Foo" =~ /^$re/ or die "not matched\n"'
works just fine...
Applied, with 1. and 2. fixed as followup...
>
> PVE/API2/Storage/Status.pm | 4 ++--
> PVE/Storage.pm | 4 +++-
> PVE/Storage/Plugin.pm | 4 ++--
> 3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
> index ce7e040..91946ac 100644
> --- a/PVE/API2/Storage/Status.pm
> +++ b/PVE/API2/Storage/Status.pm
> @@ -408,8 +408,8 @@ __PACKAGE__->register_method ({
> my $path;
>
> if ($content eq 'iso') {
> - if ($filename !~ m![^/]+\.[Ii][Ss][Oo]$!) {
> - raise_param_exc({ filename => "missing '.iso' extension" });
> + if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
> + raise_param_exc({ filename => "missing '.iso' or '.img'
> extension" });
> }
> $path = PVE::Storage::get_iso_dir($cfg, $param->{storage});
> } elsif ($content eq 'vztmpl') {
> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
> index 755eca8..08b6e14 100755
> --- a/PVE/Storage.pm
> +++ b/PVE/Storage.pm
> @@ -99,6 +99,8 @@ PVE::Storage::Plugin->init();
>
> my $UDEVADM = '/sbin/udevadm';
>
> +my $iso_extension_re = qr/\.(?:[Ii][Ss][Oo]|[Ii][Mm][Gg])/;
> +
> # PVE::Storage utility functions
>
> sub config {
> @@ -501,7 +503,7 @@ sub path_to_volume_id {
> return ('images', $info->{volid});
> }
> }
> - } elsif ($path =~ m!^$isodir/([^/]+\.[Ii][Ss][Oo])$!) {
> + } elsif ($path =~ m!^$isodir/([^/]+$iso_extension_re)$!) {
> my $name = $1;
> return ('iso', "$sid:iso/$name");
> } elsif ($path =~ m!^$tmpldir/([^/]+\.tar\.gz)$!) {
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 27f832f..9a419f1 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -415,7 +415,7 @@ sub parse_volname {
> my ($vmid, $name) = ($1, $2);
> my (undef, $format, $isBase) = parse_name_dir($name);
> return ('images', $name, $vmid, undef, undef, $isBase, $format);
> - } elsif ($volname =~ m!^iso/([^/]+\.[Ii][Ss][Oo])$!) {
> + } elsif ($volname =~ m!^iso/([^/]+$PVE::Storage::iso_extension_re)$!) {
> return ('iso', $1);
> } elsif ($volname =~ m!^vztmpl/([^/]+\.tar\.[gx]z)$!) {
> return ('vztmpl', $1);
> @@ -915,7 +915,7 @@ my $get_subdir_files = sub {
> my $info;
>
> if ($tt eq 'iso') {
> - next if $fn !~ m!/([^/]+\.iso)$!i;
> + next if $fn !~ m!/([^/]+$PVE::Storage::iso_extension_re)$!i;
>
> $info = { volid => "$sid:iso/$1", format => 'iso' };
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
