[pve-devel] [PATCH-SERIES v6 manager/common/storage] fix #1710: add download from url button

[pve-devel] [PATCH-SERIES v6 manager/common/storage] fix #1710: add download from url button

[Lorenz Stechauner]

changes to v5:
changes in frontend

patches:
1 - manager: add query_url_metadata method
2 - common: add download_file_from_url
3 - manager: refactor aplinfo to use common download function
4 - storage: add download_url method
5 - manager: add HashAlgorithmSelector
6 - manager: change download task format
7 - manager: fix #1710: add download from url button


pve-manager:

Lorenz Stechauner (5):
  api: nodes: add query_url_metadata method
  api: nodes: refactor aplinfo to use common download function
  ui: add HashAlgorithmSelector
  ui: Utils: change download task format
  fix #1710: ui: storage: add download from url button

 PVE/API2/Nodes.pm                          | 159 +++++++-----
 www/manager6/Makefile                      |   1 +
 www/manager6/Utils.js                      |   2 +-
 www/manager6/form/HashAlgorithmSelector.js |  16 ++
 www/manager6/storage/Browser.js            |   8 +
 www/manager6/storage/ContentView.js        | 284 +++++++++++++++++++--
 6 files changed, 379 insertions(+), 91 deletions(-)
 create mode 100644 www/manager6/form/HashAlgorithmSelector.js


pve-common:

Lorenz Stechauner (1):
  tools: add download_file_from_url

 src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)


pve-storage:

Lorenz Stechauner (1):
  status: add download_url method

 PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
 PVE/Storage.pm             |  10 ++++
 2 files changed, 123 insertions(+), 5 deletions(-)

-- 
2.20.1

[pve-devel] [PATCH v6 manager 1/7] api: nodes: add query_url_metadata method

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Nodes.pm | 96 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 96 insertions(+)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index e58d9c10..a289ef8b 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -11,6 +11,7 @@ use JSON;
 use POSIX qw(LONG_MAX);
 use Time::Local qw(timegm_nocheck);
 use Socket;
+use IO::Socket::SSL;
 
 use PVE::API2Tools;
 use PVE::APLInfo;
@@ -238,6 +239,7 @@ __PACKAGE__->register_method ({
 	    { name => 'netstat' },
 	    { name => 'network' },
 	    { name => 'qemu' },
+	    { name => 'query-url-metadata' },
 	    { name => 'replication' },
 	    { name => 'report' },
 	    { name => 'rrd' }, # fixme: remove?
@@ -1595,6 +1597,100 @@ __PACKAGE__->register_method({
 	return $rpcenv->fork_worker('download', undef, $user, $worker);
     }});
 
+__PACKAGE__->register_method({
+    name => 'query_url_metadata',
+    path => 'query-url-metadata',
+    method => 'GET',
+    description => "Query metadata of an URL: file size, file name and mime type.",
+    proxyto => 'node',
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    url => {
+		description => "The URL to query the metadata from.",
+		type => 'string',
+		pattern => 'https?://.*',
+	    },
+	    'verify-certificates' => {
+		description => "If false, no SSL/TLS certificates will be verified.",
+		type => 'boolean',
+		optional => 1,
+		default => 1,
+	    }
+	},
+    },
+    returns => {
+	type => "object",
+	properties => {
+	    filename => {
+		type => 'string',
+		optional => 1,
+	    },
+	    size => {
+		type => 'integer',
+		renderer => 'bytes',
+		optional => 1,
+	    },
+	    mimetype => {
+		type => 'string',
+		optional => 1,
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $url = $param->{url};
+
+	my $ua = LWP::UserAgent->new();
+
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $ua->proxy('http', $dccfg->{http_proxy});
+	}
+
+	my $verify = $param->{'verify-certificates'} // 1;
+	if (!$verify) {
+	    $ua->ssl_opts(
+		verify_hostname => 0,
+		SSL_verify_mode => IO::Socket::SSL::SSL_VERIFY_NONE,
+	    );
+	}
+
+	my $req = HTTP::Request->new(HEAD => $url);
+	my $res = $ua->request($req);
+
+	die "invalid server response: '" . $res->status_line() . "'\n" if ($res->code() != 200);
+
+	my $size = $res->header("Content-Length");
+	my $disposition = $res->header("Content-Disposition");
+	my $type = $res->header("Content-Type");
+
+	my $filename;
+
+	if ($disposition && $disposition =~ m/filename=(.+)/) {
+	    $filename = $1;
+	} elsif ($url =~ m!^[^?]+/([^?/]*)(?:\?.*)?$!) {
+	    $filename = $1;
+	}
+
+	# Content-Type: text/html; charset=utf-8
+	if ($type && $type =~ m/^([^;]+);/) {
+	    $type = $1;
+	}
+
+	my $ret = {};
+	$ret->{filename} = $filename if $filename;
+	$ret->{size} = $size + 0 if $size;
+	$ret->{mimetype} = $type if $type;
+
+	return $ret;
+    }});
+
 __PACKAGE__->register_method({
     name => 'report',
     path => 'report',
-- 
2.20.1

[pve-devel] [PATCH v6 common 2/7] tools: add download_file_from_url

[Lorenz Stechauner]

code is based on
manager:PVE/API2/Nodes.pm:aplinfo

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index 16ae3d2..7b82e00 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -1829,4 +1829,128 @@ sub safe_compare {
     return $cmp->($left, $right);
 }
 
+
+# opts
+#  -> hash_required
+#       if 1, at least one checksum has to be specified otherwise an error will be thrown
+#  -> http_proxy
+#  -> https_proxy
+#  -> verify_certificates
+#  -> sha(1|224|256|384|512)sum
+#  -> md5sum
+sub download_file_from_url {
+    my ($dest, $url, $opts) = @_;
+
+    my $tmpdest = "$dest.tmp.$$";
+
+    my $algorithm;
+    my $expected;
+
+    for ('sha512', 'sha384', 'sha256', 'sha224', 'sha1', 'md5') {
+	if (defined($opts->{"${_}sum"})) {
+	    $algorithm = $_;
+	    $expected = $opts->{"${_}sum"};
+	    last;
+	}
+    }
+
+    die "checksum required but not specified\n" if ($opts->{hash_required} && !$algorithm);
+
+    my $worker = sub  {
+	my $upid = shift;
+
+	print "downloading $url to $dest\n";
+
+	eval {
+	    if (-f $dest && $algorithm) {
+		print "calculating checksum of existing file...\n";
+		my $correct = check_file_hash($algorithm, $expected, $dest);
+
+		if ($correct) {
+		    print "file already exists, no need to download\n";
+		    return;
+		} else {
+		    print "mismatch, downloading\n";
+		}
+	    }
+
+	    my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $url);
+
+	    local %ENV;
+	    if ($opts->{http_proxy}) {
+		$ENV{http_proxy} = $opts->{http_proxy};
+	    }
+	    if ($opts->{https_proxy}) {
+		$ENV{https_proxy} = $opts->{https_proxy};
+	    }
+
+	    my $verify = $opts->{verify_certificates} // 1;
+	    if (!$verify) {
+		push @cmd, '--no-check-certificate';
+	    }
+
+	    if (run_command([[@cmd]]) != 0) {
+		die "download failed: $!\n";
+	    }
+
+	    if ($algorithm) {
+		print "calculating checksum...\n";
+
+		my $correct = check_file_hash($algorithm, $expected, $tmpdest);
+
+		if ($correct) {
+		    print "checksum verified\n";
+		} else {
+		    die "checksum mismatch\n";
+		}
+	    } else {
+		print "no checksum for verification specified\n";
+	    }
+
+	    if (!rename($tmpdest, $dest)) {
+		die "unable to save file: $!\n";
+	    }
+	};
+	my $err = $@;
+
+	unlink $tmpdest;
+
+	if ($err) {
+	    print "\n";
+	    die $err;
+	}
+
+	print "download finished\n";
+    };
+
+    my $rpcenv = PVE::RPCEnvironment::get();
+    my $user = $rpcenv->get_user();
+
+    (my $filename = $dest) =~ s!.*/([^/]*)$!$1!;
+
+    return $rpcenv->fork_worker('download', $filename, $user, $worker);
+}
+
+sub check_file_hash {
+    my ($algorithm, $expected, $filename) = @_;
+
+    my $algorithm_map = {
+	'md5' => sub { Digest::MD5->new },
+	'sha1' => sub { Digest::SHA->new(1) },
+	'sha224' => sub { Digest::SHA->new(224) },
+	'sha256' => sub { Digest::SHA->new(256) },
+	'sha384' => sub { Digest::SHA->new(384) },
+	'sha512' => sub { Digest::SHA->new(512) },
+    };
+
+    my $digester = $algorithm_map->{$algorithm}->() or die "unknown algorithm '$algorithm'\n";
+
+    open(my $fh, '<', $filename) or die "unable to open '$filename': $!\n";
+    binmode($fh);
+
+    my $digest = $digester->addfile($fh)->hexdigest;
+
+    return lc($digest) eq lc($expected);
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH v6 manager 3/7] api: nodes: refactor aplinfo to use common download function

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Nodes.pm | 89 ++++++-----------------------------------------
 1 file changed, 10 insertions(+), 79 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index a289ef8b..16b10925 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -1513,88 +1513,19 @@ __PACKAGE__->register_method({
 	my $src = $pd->{location};
 	my $tmpldir = PVE::Storage::get_vztmpl_dir($cfg, $param->{storage});
 	my $dest = "$tmpldir/$template";
-	my $tmpdest = "$tmpldir/${template}.tmp.$$";
 
-	my $worker = sub  {
-	    my $upid = shift;
-
-	    print "starting template download from: $src\n";
-	    print "target file: $dest\n";
-
-	    my $check_hash = sub {
-		my ($template_info, $filename, $noerr) = @_;
-
-		my $digest;
-		my $expected;
-
-		eval {
-		    open(my $fh, '<', $filename) or die "Can't open '$filename': $!";
-		    binmode($fh);
-		    if (defined($template_info->{sha512sum})) {
-			$expected = $template_info->{sha512sum};
-			$digest = Digest::SHA->new(512)->addfile($fh)->hexdigest;
-		    } elsif (defined($template_info->{md5sum})) {
-			#fallback to MD5
-			$expected = $template_info->{md5sum};
-			$digest = Digest::MD5->new->addfile($fh)->hexdigest;
-		    } else {
-			die "no expected checksum defined";
-		    }
-		    close($fh);
-		};
-
-		die "checking hash failed - $@\n" if $@ && !$noerr;
-
-		return ($digest, $digest ? lc($digest) eq lc($expected) : 0);
-	    };
-
-	    eval {
-		if (-f $dest) {
-		    my ($hash, $correct) = &$check_hash($pd, $dest, 1);
-
-		    if ($hash && $correct) {
-			print "file already exists $hash - no need to download\n";
-			return;
-		    }
-		}
-
-		local %ENV;
-		my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
-		if ($dccfg->{http_proxy}) {
-		    $ENV{http_proxy} = $dccfg->{http_proxy};
-		}
-
-		my @cmd = ('/usr/bin/wget', '--progress=dot:mega', '-O', $tmpdest, $src);
-		if (system (@cmd) != 0) {
-		    die "download failed - $!\n";
-		}
-
-		my ($hash, $correct) = &$check_hash($pd, $tmpdest);
-
-		die "could not calculate checksum\n" if !$hash;
-
-		if (!$correct) {
-		    my $expected = $pd->{sha512sum} // $pd->{md5sum};
-		    die "wrong checksum: $hash != $expected\n";
-		}
-
-		if (!rename($tmpdest, $dest)) {
-		    die "unable to save file - $!\n";
-		}
-	    };
-	    my $err = $@;
-
-	    unlink $tmpdest;
-
-	    if ($err) {
-		print "\n";
-		die $err if $err;
-	    }
-
-	    print "download finished\n";
+	my $opts = {
+	    hash_required => 1,
+	    sha512sum => $pd->{sha512sum},
+	    md5sum => $pd->{md5sum},
 	};
 
-	return $rpcenv->fork_worker('download', undef, $user, $worker);
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $opts->{http_proxy} = $dccfg->{http_proxy};
+	}
+
+	return PVE::Tools::download_file_from_url($dest, $src, $opts);
     }});
 
 __PACKAGE__->register_method({
-- 
2.20.1

[pve-devel] [PATCH v6 storage 4/7] status: add download_url method

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
 PVE/Storage.pm             |  10 ++++
 2 files changed, 123 insertions(+), 5 deletions(-)

diff --git a/PVE/API2/Storage/Status.pm b/PVE/API2/Storage/Status.pm
index 897b4a7..4c1d1fc 100644
--- a/PVE/API2/Storage/Status.pm
+++ b/PVE/API2/Storage/Status.pm
@@ -412,11 +412,7 @@ __PACKAGE__->register_method ({
 	my $size = -s $tmpfilename;
 	die "temporary file '$tmpfilename' does not exist\n" if !defined($size);
 
-	my $filename = $param->{filename};
-
-	chomp $filename;
-	$filename =~ s/^.*[\/\\]//;
-	$filename =~ s/[^-a-zA-Z0-9_.]/_/g;
+	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
 
 	my $path;
 
@@ -497,4 +493,116 @@ __PACKAGE__->register_method ({
 	return $upid;
    }});
 
+__PACKAGE__->register_method({
+    name => 'download_url',
+    path => '{storage}/download-url',
+    method => 'POST',
+    description => "Download templates and ISO images by using an URL.",
+    proxyto => 'node',
+    permissions => {
+	check => [ 'and',
+	    ['perm', '/storage/{storage}', [ 'Datastore.AllocateTemplate' ]],
+	    ['perm', '/', [ 'Sys.Audit', 'Sys.Modify' ]],
+	],
+    },
+    protected => 1,
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    storage => get_standard_option('pve-storage-id'),
+	    url => {
+		description => "The URL to download the file from.",
+		type => 'string',
+		pattern => 'https?://.*',
+	    },
+	    content => {
+		description => "Content type.",
+		type => 'string', format => 'pve-storage-content',
+	    },
+	    filename => {
+		description => "The name of the file to create.",
+		type => 'string',
+	    },
+	    checksum => {
+		description => "The expected checksum of the file.",
+		type => 'string',
+		requires => 'checksum-algorithm',
+		optional => 1,
+	    },
+	    'checksum-algorithm' => {
+		description => "The algorithm to calculate the checksum of the file.",
+		type => 'string',
+		enum => ['md5', 'sha1', 'sha224', 'sha256', 'sha384', 'sha512'],
+		requires => 'checksum',
+		optional => 1,
+	    },
+	    'verify-certificates' => {
+		description => "If false, no SSL/TLS certificates will be verified.",
+		type => 'boolean',
+		optional => 1,
+		default => 1,
+	    }
+	},
+    },
+    returns => {
+	type => "string"
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $cfg = PVE::Storage::config();
+
+	my ($node, $storage) = $param->@{'node', 'storage'};
+	my $scfg = PVE::Storage::storage_check_enabled($cfg, $storage, $node);
+
+	die "can't upload to storage type '$scfg->{type}, not a file based storage!'\n" if !defined($scfg->{path});
+
+	my ($content, $url) = $param->@{'content', 'url'};
+
+	my $filename = PVE::Storage::normalize_content_filename($param->{filename});
+	my $path;
+
+	# MIME type is checked in front end only
+	# this check is omitted here intentionally and replaced by file extension check
+	if ($content eq 'iso') {
+	    if ($filename !~ m![^/]+$PVE::Storage::iso_extension_re$!) {
+		raise_param_exc({ filename => "missing '.iso' or '.img' extension" });
+	    }
+	    $path = PVE::Storage::get_iso_dir($cfg, $storage);
+	} elsif ($content eq 'vztmpl') {
+	    if ($filename !~ m![^/]+\.tar\.[gx]z$!) {
+		raise_param_exc({ filename => "missing '.tar.gz' or '.tar.xz' extension" });
+	    }
+	    $path = PVE::Storage::get_vztmpl_dir($cfg, $storage);
+	} else {
+	    raise_param_exc({ content => "upload content type '$content' not allowed" });
+	}
+
+	die "storage '$storage' does not support '$content' content\n" if !$scfg->{content}->{$content};
+
+	PVE::Storage::activate_storage($cfg, $storage);
+	File::Path::make_path($path);
+
+	my $dest = "$path/$filename";
+
+	my $opts = {
+	    hash_required => 0,
+	    verify_certificates => $param->{'verify-certificates'} // 1,
+	};
+
+	my ($checksum, $checksum_algorithm) = $param->@{'checksum', 'checksum-algorithm'};
+	if ($checksum) {
+	    $opts->{"${checksum_algorithm}sum"} = $checksum;
+	    $opts->{hash_required} = 1;
+	}
+
+	my $dccfg = PVE::Cluster::cfs_read_file('datacenter.cfg');
+	if ($dccfg->{http_proxy}) {
+	    $opts->{http_proxy} = $dccfg->{http_proxy};
+	}
+
+	return PVE::Tools::download_file_from_url($dest, $url, $opts);
+    }});
+
 1;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 122c3e9..d57fd43 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1931,4 +1931,14 @@ sub assert_sid_unused {
     return undef;
 }
 
+sub normalize_content_filename {
+    my ($filename) = @_;
+
+    chomp $filename;
+    $filename =~ s/^.*[\/\\]//;
+    $filename =~ s/[^-a-zA-Z0-9_.]/_/g;
+
+    return $filename;
+}
+
 1;
-- 
2.20.1

[pve-devel] [PATCH v6 manager 5/7] ui: add HashAlgorithmSelector

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/Makefile                      |  1 +
 www/manager6/form/HashAlgorithmSelector.js | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 www/manager6/form/HashAlgorithmSelector.js

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index afed3283..8e6557d8 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -38,6 +38,7 @@ JSSRC= 							\
 	form/GlobalSearchField.js			\
 	form/GroupSelector.js				\
 	form/GuestIDSelector.js				\
+	form/HashAlgorithmSelector.js			\
 	form/HotplugFeatureSelector.js			\
 	form/IPProtocolSelector.js			\
 	form/IPRefSelector.js				\
diff --git a/www/manager6/form/HashAlgorithmSelector.js b/www/manager6/form/HashAlgorithmSelector.js
new file mode 100644
index 00000000..5ae7a08b
--- /dev/null
+++ b/www/manager6/form/HashAlgorithmSelector.js
@@ -0,0 +1,16 @@
+Ext.define('PVE.form.hashAlgorithmSelector', {
+    extend: 'Proxmox.form.KVComboBox',
+    alias: ['widget.pveHashAlgorithmSelector'],
+    config: {
+	deleteEmpty: false,
+    },
+    comboItems: [
+	['__default__', 'None'],
+	['md5', 'MD5'],
+	['sha1', 'SHA-1'],
+	['sha224', 'SHA-224'],
+	['sha256', 'SHA-256'],
+	['sha384', 'SHA-384'],
+	['sha512', 'SHA-512'],
+    ],
+});
-- 
2.20.1

[pve-devel] [PATCH v6 manager 6/7] ui: Utils: change download task format

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/Utils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 581d2040..38306c11 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1819,7 +1819,7 @@ Ext.define('PVE.Utils', {
 	    clusterjoin: ['', gettext('Join Cluster')],
 	    dircreate: [gettext('Directory Storage'), gettext('Create')],
 	    dirremove: [gettext('Directory'), gettext('Remove')],
-	    download: ['', gettext('Download')],
+	    download: [gettext('File'), gettext('Download')],
 	    hamigrate: ['HA', gettext('Migrate')],
 	    hashutdown: ['HA', gettext('Shutdown')],
 	    hastart: ['HA', gettext('Start')],
-- 
2.20.1

[pve-devel] [PATCH v6 manager 7/7] fix #1710: ui: storage: add download from url button

[Lorenz Stechauner]

Signed-off-by: Lorenz Stechauner <l.stechauner@proxmox.com>
---
 www/manager6/storage/Browser.js     |   8 +
 www/manager6/storage/ContentView.js | 284 +++++++++++++++++++++++++---
 2 files changed, 268 insertions(+), 24 deletions(-)

diff --git a/www/manager6/storage/Browser.js b/www/manager6/storage/Browser.js
index 5fee94c7..fe5df3e2 100644
--- a/www/manager6/storage/Browser.js
+++ b/www/manager6/storage/Browser.js
@@ -53,6 +53,9 @@ Ext.define('PVE.storage.Browser', {
 	    let plugin = res.plugintype;
 	    let contents = res.content.split(',');
 
+	    let enableUpload = !!caps.storage['Datastore.AllocateTemplate'];
+	    let enableDownloadUrl = enableUpload && !!(caps.nodes['Sys.Audit'] && caps.nodes['Sys.Modify']);
+
 	    if (contents.includes('backup')) {
 		me.items.push({
 		    xtype: 'pveStorageBackupView',
@@ -91,6 +94,8 @@ Ext.define('PVE.storage.Browser', {
 		    itemId: 'contentIso',
 		    content: 'iso',
 		    pluginType: plugin,
+		    enableUploadButton: enableUpload,
+		    enableDownloadUrlButton: enableDownloadUrl,
 		    useUploadButton: true,
 		});
 	    }
@@ -101,6 +106,9 @@ Ext.define('PVE.storage.Browser', {
 		    iconCls: 'fa fa-file-o lxc',
 		    itemId: 'contentVztmpl',
 		    pluginType: plugin,
+		    enableUploadButton: enableUpload,
+		    enableDownloadUrlButton: enableDownloadUrl,
+		    useUploadButton: true,
 		});
 	    }
 	    if (contents.includes('snippets')) {
diff --git a/www/manager6/storage/ContentView.js b/www/manager6/storage/ContentView.js
index dd6df4b1..f562c260 100644
--- a/www/manager6/storage/ContentView.js
+++ b/www/manager6/storage/ContentView.js
@@ -191,6 +191,228 @@ Ext.define('PVE.storage.Upload', {
     },
 });
 
+Ext.define('PVE.storage.DownloadUrl', {
+    extend: 'Proxmox.window.Edit',
+    alias: 'widget.pveStorageDownloadUrl',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    isCreate: true,
+
+    method: 'POST',
+
+    showTaskViewer: true,
+
+    title: gettext('Download from URL'),
+    submitText: gettext('Download'),
+
+    cbindData: function(initialConfig) {
+	var me = this;
+	return {
+	    nodename: me.nodename,
+	    storage: me.storage,
+	    contents: me.contents,
+	    content: me.contents[0],
+	};
+    },
+
+    cbind: {
+	url: '/nodes/{nodename}/storage/{storage}/download-url',
+    },
+
+    controller: {
+	xclass: 'Ext.app.ViewController',
+
+	urlChange: function(field) {
+	    let me = this;
+	    let view = me.getView();
+	    field = view.down('[name=url]');
+	    field.setValidation("Waiting for response...");
+	    field.validate();
+	    view.setValues({ size: "" });
+	    Proxmox.Utils.API2Request({
+		url: `/nodes/${view.nodename}/query-url-metadata`,
+		method: 'GET',
+		params: {
+		    url: field.getValue(),
+		    'verify-certificates': view.getValues()['verify-certificates'],
+		},
+		failure: function(res, opt) {
+		    field.setValidation(res.result.message);
+		    field.validate();
+		    view.setValues({
+			size: "",
+			mimetype: "",
+		    });
+		},
+		success: function(res, opt) {
+		    field.setValidation();
+		    field.validate();
+
+		    let data = res.result.data;
+		    view.setValues({
+			filename: data.filename || "",
+			size: (data.size && Proxmox.Utils.format_size(data.size)) || "",
+			mimetype: data.mimetype || "",
+		    });
+		},
+	    });
+	},
+
+	hashChange: function(field) {
+	    let checksum = Ext.getCmp('downloadUrlChecksum');
+	    if (field.getValue() === '__default__') {
+		checksum.setDisabled(true);
+		checksum.setValue("");
+		checksum.allowBlank = true;
+	    } else {
+		checksum.setDisabled(false);
+		checksum.allowBlank = false;
+	    }
+	},
+
+	typeChange: function(field) {
+	    let me = this;
+	    let view = me.getView();
+	    let content = view.getValues().content;
+	    let type = field.getValue();
+
+	    const types = {
+		iso: [
+		    'application/octet-stream',
+		    'application/x-iso9660-image',
+		    'application/x-ima',
+		],
+		vztmpl: [
+		    'application/octet-stream',
+		    'application/gzip',
+		    'application/tar',
+		    'application/tar+gzip',
+		    'application/x-gzip',
+		    'application/x-gtar',
+		    'application/x-tgz',
+		    'application/x-tar',
+		],
+	    };
+
+	    if (type === "" || (types[content] && types[content].includes(type))) {
+		field.setValidation();
+		field.setDisabled(true);
+	    } else {
+		field.setDisabled(false);
+		field.setValidation("Invalid type");
+	    }
+	    field.validate();
+	},
+    },
+
+    items: [
+	{
+	    xtype: 'inputpanel',
+	    waitMsgTarget: true,
+	    border: false,
+	    columnT: [
+		{
+		    xtype: 'textfield',
+		    name: 'url',
+		    allowBlank: false,
+		    fieldLabel: gettext('URL'),
+		    listeners: {
+			change: {
+			    buffer: 500,
+			    fn: 'urlChange',
+			},
+		    },
+		},
+		{
+		    xtype: 'textfield',
+		    name: 'filename',
+		    allowBlank: false,
+		    fieldLabel: gettext('File name'),
+		},
+	    ],
+	    column1: [
+		{
+		    xtype: 'pveContentTypeSelector',
+		    fieldLabel: gettext('Content'),
+		    name: 'content',
+		    allowBlank: false,
+		    cbind: {
+			cts: '{contents}',
+			value: '{content}',
+		    },
+		},
+	    ],
+	    column2: [
+		{
+		    xtype: 'textfield',
+		    name: 'size',
+		    disabled: true,
+		    fieldLabel: gettext('File size'),
+		    emptyText: gettext('unknown'),
+		},
+	    ],
+	    advancedColumn1: [
+		{
+		    xtype: 'textfield',
+		    name: 'checksum',
+		    fieldLabel: gettext('Checksum'),
+		    allowBlank: true,
+		    disabled: true,
+		    emptyText: gettext('none'),
+		    id: 'downloadUrlChecksum',
+		},
+		{
+		    xtype: 'pveHashAlgorithmSelector',
+		    name: 'checksum-algorithm',
+		    fieldLabel: gettext('Hash algorithm'),
+		    allowBlank: true,
+		    hasNoneOption: true,
+		    value: '__default__',
+		    listeners: {
+			change: 'hashChange',
+		    },
+		},
+	    ],
+	    advancedColumn2: [
+		{
+		    xtype: 'textfield',
+		    fieldLabel: gettext('MIME type'),
+		    name: 'mimetype',
+		    disabled: true,
+		    editable: false,
+		    emptyText: gettext('unknown'),
+		    listeners: {
+			change: 'typeChange',
+		    },
+		},
+		{
+		    xtype: 'proxmoxcheckbox',
+		    name: 'verify-certificates',
+		    fieldLabel: gettext('Verify certificates'),
+		    uncheckedValue: 0,
+		    checked: true,
+		    listeners: {
+			change: 'urlChange',
+		    },
+		},
+	    ],
+	},
+    ],
+
+    initComponent: function() {
+        var me = this;
+
+	if (!me.nodename) {
+	    throw "no node name specified";
+	}
+	if (!me.storage) {
+	    throw "no storage ID specified";
+	}
+
+        me.callParent();
+    },
+});
+
 Ext.define('PVE.storage.ContentView', {
     extend: 'Ext.grid.GridPanel',
 
@@ -249,36 +471,50 @@ Ext.define('PVE.storage.ContentView', {
 
 	Proxmox.Utils.monStoreErrors(me, store);
 
-	let uploadButton = Ext.create('Proxmox.button.Button', {
-	    text: gettext('Upload'),
-	    handler: function() {
-		let win = Ext.create('PVE.storage.Upload', {
-		    nodename: nodename,
-		    storage: storage,
-		    contents: [content],
-		});
-		win.show();
-		win.on('destroy', reload);
-	    },
-	});
-
-	let removeButton = Ext.create('Proxmox.button.StdRemoveButton', {
-	    selModel: sm,
-	    delay: 5,
-	    callback: function() {
-		reload();
-	    },
-	    baseurl: baseurl + '/',
-	});
-
 	if (!me.tbar) {
 	    me.tbar = [];
 	}
 	if (me.useUploadButton) {
-	    me.tbar.push(uploadButton);
+	    me.tbar.unshift(
+		{
+		    xtype: 'button',
+		    text: gettext('Upload'),
+		    disabled: !me.enableUploadButton,
+		    handler: function() {
+			Ext.create('PVE.storage.Upload', {
+			    nodename: nodename,
+			    storage: storage,
+			    contents: [content],
+			    autoShow: true,
+			    taskDone: () => reload(),
+			});
+		    },
+		},
+		{
+		    xtype: 'button',
+		    text: gettext('Download from URL'),
+		    disabled: !me.enableDownloadUrlButton,
+		    handler: function() {
+			Ext.create('PVE.storage.DownloadUrl', {
+			    nodename: nodename,
+			    storage: storage,
+			    contents: [content],
+			    autoShow: true,
+			    taskDone: () => reload(),
+			});
+		    },
+		},
+		'-',
+	    );
 	}
 	if (!me.useCustomRemoveButton) {
-	    me.tbar.push(removeButton);
+	    me.tbar.push({
+		xtype: 'proxmoxStdRemoveButton',
+		selModel: sm,
+		delay: 5,
+		callback: () => reload(),
+		baseurl: baseurl + '/',
+	    });
 	}
 	me.tbar.push(
 	    '->',
-- 
2.20.1

Re: [pve-devel] [PATCH-SERIES v6 manager/common/storage] fix #1710: add download from url button

[Lorenz Stechauner]

ping

On 17.05.21 10:03, Lorenz Stechauner wrote:
> changes to v5:
> changes in frontend
>
> patches:
> 1 - manager: add query_url_metadata method
> 2 - common: add download_file_from_url
> 3 - manager: refactor aplinfo to use common download function
> 4 - storage: add download_url method
> 5 - manager: add HashAlgorithmSelector
> 6 - manager: change download task format
> 7 - manager: fix #1710: add download from url button
>
>
> pve-manager:
>
> Lorenz Stechauner (5):
>    api: nodes: add query_url_metadata method
>    api: nodes: refactor aplinfo to use common download function
>    ui: add HashAlgorithmSelector
>    ui: Utils: change download task format
>    fix #1710: ui: storage: add download from url button
>
>   PVE/API2/Nodes.pm                          | 159 +++++++-----
>   www/manager6/Makefile                      |   1 +
>   www/manager6/Utils.js                      |   2 +-
>   www/manager6/form/HashAlgorithmSelector.js |  16 ++
>   www/manager6/storage/Browser.js            |   8 +
>   www/manager6/storage/ContentView.js        | 284 +++++++++++++++++++--
>   6 files changed, 379 insertions(+), 91 deletions(-)
>   create mode 100644 www/manager6/form/HashAlgorithmSelector.js
>
>
> pve-common:
>
> Lorenz Stechauner (1):
>    tools: add download_file_from_url
>
>   src/PVE/Tools.pm | 124 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 124 insertions(+)
>
>
> pve-storage:
>
> Lorenz Stechauner (1):
>    status: add download_url method
>
>   PVE/API2/Storage/Status.pm | 118 +++++++++++++++++++++++++++++++++++--
>   PVE/Storage.pm             |  10 ++++
>   2 files changed, 123 insertions(+), 5 deletions(-)
>

Re: [pve-devel] [PATCH v6 manager 1/7] api: nodes: add query_url_metadata method

[Dominic Jäger]

On Mon, May 17, 2021 at 10:03:12AM +0200, Lorenz Stechauner wrote:
> +
> +	my $size = $res->header("Content-Length");
> +	my $disposition = $res->header("Content-Disposition");
When I use the Proxmox VE 6.4 image from the website
> pvesh get nodes/dev/query-url-metadata --url https://www.proxmox.com/en/downloads\?task\=callelement\&format\=raw\&item_id\=589\&element\=f85c494b-2b32-4109-b8c1-083cca2b7db6\&method\=download\&args\[0\]\=b9d30b3515e7a12dff5619ff1e1843be

then I get as filename
> "proxmox-ve_6.4-1.iso"; modification-date="Tue, 27 Apr 2021 15:25:15 +0200"; size=928993280;

and it gets directly into the GUI like that.

It did work for a Debian image.
https://cdimage.debian.org/debian-cd/current/amd64/iso-dvd/debian-10.9.0-amd64-DVD-1.iso

Tested-by: Dominic Jäger <d.jaeger@proxmox.com>

Re: [pve-devel] [PATCH v6 manager 7/7] fix #1710: ui: storage: add download from url button

[Dominic Jäger]

On Mon, May 17, 2021 at 10:03:18AM +0200, Lorenz Stechauner wrote:
> +		iso: [ > +		    'application/octet-stream',
> +		    'application/x-iso9660-image',
> +		    'application/x-ima',
When I try do download a Proxmox VE image I get type x-cd-image.
https://www.proxmox.com/en/downloads?task=callelement&format=raw&item_id=589&element=f85c494b-2b32-4109-b8c1-083cca2b7db6&method=download&args[0]=b9d30b3515e7a12dff5619ff1e1843be
Then the download button should not be deactivated.

> +	    advancedColumn1: [
> +		{
> +		    xtype: 'textfield',
> +		    name: 'checksum',
> +		    fieldLabel: gettext('Checksum'),
> +		    allowBlank: true,
> +		    disabled: true,
> +		    emptyText: gettext('none'),
> +		    id: 'downloadUrlChecksum',
> +		},
> +		{
> +		    xtype: 'pveHashAlgorithmSelector',
> +		    name: 'checksum-algorithm',
> +		    fieldLabel: gettext('Hash algorithm'),
> +		    allowBlank: true,
> +		    hasNoneOption: true,
> +		    value: '__default__',
> +		    listeners: {
> +			change: 'hashChange',
> +		    },
> +		},
An algorithm must be selected before entering something into the textfield.
Therefore, I'd place the algorithm selector above/before the textfield.
> +	    ],
> +	    advancedColumn2: [
> +		{
> +		    xtype: 'textfield',
> +		    fieldLabel: gettext('MIME type'),
> +		    name: 'mimetype',
> +		    disabled: true,
> +		    editable: false,
Non-editable might be good in general, but I cannot download the PVE image with
(unsupported) type x-cd-image because it will remain invalid forever.
> +		    emptyText: gettext('unknown'),
> +		    listeners: {
> +			change: 'typeChange',
> +		    },
> +		},
> +		{
> +		    xtype: 'proxmoxcheckbox',
> +		    name: 'verify-certificates',
> +		    fieldLabel: gettext('Verify certificates'),
> +		    uncheckedValue: 0,
> +		    checked: true,
> +		    listeners: {
> +			change: 'urlChange',
> +		    },
> +		},
> +	    ],
> +	},
> +    ],
> +

Downloading a Debian image and verifying with SHA512 worked for me
https://cdimage.debian.org/debian-cd/current/amd64/iso-dvd/debian-10.9.0-amd64-DVD-1.iso

Tested-by: Dominic Jäger <d.jaeger@proxmox.com>

Re: [pve-devel] [PATCH v6 manager 7/7] fix #1710: ui: storage: add download from url button

[Lorenz Stechauner]

Thanks for the feedback :)

On 11.06.21 09:27, Dominic Jäger wrote:
> On Mon, May 17, 2021 at 10:03:18AM +0200, Lorenz Stechauner wrote:
>> +		iso: [ > +		    'application/octet-stream',
>> +		    'application/x-iso9660-image',
>> +		    'application/x-ima',
> When I try do download a Proxmox VE image I get type x-cd-image.
> https://www.proxmox.com/en/downloads?task=callelement&format=raw&item_id=589&element=f85c494b-2b32-4109-b8c1-083cca2b7db6&method=download&args[0]=b9d30b3515e7a12dff5619ff1e1843be
> Then the download button should not be deactivated.

I'll add x-cd-image to the list. Maybe a "I know, what I'm doing" 
checkbox in "advanced" will also work? (Which activates the download 
button without checking the mime type)

The goal was, to restrict the user as much as possible to avoid possible 
errors in the future.

>> +	    advancedColumn1: [
>> +		{
>> +		    xtype: 'textfield',
>> +		    name: 'checksum',
>> +		    fieldLabel: gettext('Checksum'),
>> +		    allowBlank: true,
>> +		    disabled: true,
>> +		    emptyText: gettext('none'),
>> +		    id: 'downloadUrlChecksum',
>> +		},
>> +		{
>> +		    xtype: 'pveHashAlgorithmSelector',
>> +		    name: 'checksum-algorithm',
>> +		    fieldLabel: gettext('Hash algorithm'),
>> +		    allowBlank: true,
>> +		    hasNoneOption: true,
>> +		    value: '__default__',
>> +		    listeners: {
>> +			change: 'hashChange',
>> +		    },
>> +		},
> An algorithm must be selected before entering something into the textfield.
> Therefore, I'd place the algorithm selector above/before the textfield.
>> +	    ],
>> +	    advancedColumn2: [
>> +		{
>> +		    xtype: 'textfield',
>> +		    fieldLabel: gettext('MIME type'),
>> +		    name: 'mimetype',
>> +		    disabled: true,
>> +		    editable: false,
> Non-editable might be good in general, but I cannot download the PVE image with
> (unsupported) type x-cd-image because it will remain invalid forever.
>> +		    emptyText: gettext('unknown'),
>> +		    listeners: {
>> +			change: 'typeChange',
>> +		    },
>> +		},
>> +		{
>> +		    xtype: 'proxmoxcheckbox',
>> +		    name: 'verify-certificates',
>> +		    fieldLabel: gettext('Verify certificates'),
>> +		    uncheckedValue: 0,
>> +		    checked: true,
>> +		    listeners: {
>> +			change: 'urlChange',
>> +		    },
>> +		},
>> +	    ],
>> +	},
>> +    ],
>> +
> Downloading a Debian image and verifying with SHA512 worked for me
> https://cdimage.debian.org/debian-cd/current/amd64/iso-dvd/debian-10.9.0-amd64-DVD-1.iso
>
> Tested-by: Dominic Jäger <d.jaeger@proxmox.com>