[pve-devel] [PATCH http/common/manager/wt/proxmox-backup/pmg] Tasklog download button

[pve-devel] [PATCH http/common/manager/wt/proxmox-backup/pmg] Tasklog download button

[Daniel Tschlatscher]

This patch series' aim is to add a download button in the tasklog-
viewer GUI so that users may access all parts of the tasklog more
easily (The tasklog-viewer only displays 50 lines at a time).

For this change in the GUI (proxmox-widget-toolkit) there were revised
implementations for all backends (PVE, PMG and PBS) needed.
The change was implemented by setting the URL parameter 'limit' for
the .../log call and streaming a file in this case. (Before this
call returned 0 lines).

I also revised a few different parts in the code which had redundant
implementations for "downloading" a file locally from a string.
For this I added a function in the proxmox-widget-toolkit Utils class
which all occurences (that I found) now call.

During the implementation of the file stream download I found a bug
concerning chromium browsers because of which downloads "randomly"
fail.
The problem was, that when implementing the download through an html a
tag and setting its property "download" to anything (even undefined)
with a self-signed certificate (not imported) will fail after the
SSL connection is reset.
In my testing this was the case after about 5 seconds. This problem is
especially pronounced in the PMG as there are less background calls in
contrast to the PVE. However, the PBS does not seem to suffer this
error at all, because the SSL handshake timeout seems to not run out.
 
Without the 'download' attribute you can't set the filename though.
The file will only have a proper name if the server adds the correct
'content-disposition' header. A few calls to the 'downloadAsFile'
function set the filename anyway, because in most cases the browser
frontend is making enough polling calls so that the SSL handshake
does not have to be executed for the download. (That is also why it
probably did not come up before)
This problem does not apply in Firefox as far as I could tell. 

Daniel Tschlatscher (1):
  fix #3971: tasklog download in the backup server backend

 Cargo.toml               |   2 +-
 src/api2/node/tasks.rs   | 154 +++++++++++++++++++++++++--------------
 www/Subscription.js      |  14 +---
 www/datastore/Content.js |   7 +-
 4 files changed, 105 insertions(+), 72 deletions(-)

Daniel Tschlatscher (1):
  fix #3971: Make tasklog downloadable

 src/PVE/APIServer/AnyEvent.pm | 3 +++
 1 file changed, 3 insertions(+)

Daniel Tschlatscher (1):
  fix #3971: Create log file stream for download

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

Daniel Tschlatscher (1):
  fix #3971: Revised task log API call when parameter 'limit' is 0

 PVE/API2/Tasks.pm                 | 13 ++++++++++---
 www/manager6/node/Subscription.js | 20 ++++----------------
 2 files changed, 14 insertions(+), 19 deletions(-)

Daniel Tschlatscher (1):
  fix #3971: Download button in TaskViewer for PMG

 src/PMG/API2/Tasks.pm | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

Daniel Tschlatscher (1):
  fix #3971: Download button in the TaskViewer

 src/Utils.js              | 18 ++++++++++++++++++
 src/window/FileBrowser.js | 12 +++---------
 src/window/TaskViewer.js  | 16 ++++++++++++++--
 3 files changed, 35 insertions(+), 11 deletions(-)

Daniel Tschlatscher (1):
  Replaced the system-report file download implementation

 js/Subscription.js | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)
-- 
2.30.2

[pve-devel] [PATCH proxmox-backup 1/1] fix #3971: tasklog download in the backup server backend

[Daniel Tschlatscher]

The API call for getting the tasklog now returns a file stream when
the URL parameter 'limit' is set to 0, in accordance with the changes
in the PMG and PVE backends.
To make this work I had to use one of the lower level apimethod types
from the proxmox-router. Therefore I also had to change declarations
for the API routing and the corresponding Object Schemas.
If the parameter 'limit' is not set to 0 the API should behave the
same as before.

I also took the chance to revise two "download as file" calls in the
PBS frontend and replaced them with a simple call to the newly created
Utils function "downloadAsFile" which was sourced in the
proxmox-widget-toolkit repository in this patch-series.

Also bumped the version of proxmox-router to apply a hotfix for
editing RPCEnv by index.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 Cargo.toml               |   2 +-
 src/api2/node/tasks.rs   | 154 +++++++++++++++++++++++++--------------
 www/Subscription.js      |  14 +---
 www/datastore/Content.js |   7 +-
 4 files changed, 105 insertions(+), 72 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index a1ea8248..2a161d2b 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -96,7 +96,7 @@ pxar = { version = "0.10.1", features = [ "tokio-io" ] }
 proxmox-http = { version = "0.6", features = [ "client", "http-helpers", "websocket" ] }
 proxmox-io = "1"
 proxmox-lang = "1.1"
-proxmox-router = { version = "1.2", features = [ "cli" ] }
+proxmox-router = { version = "1.2.1", features = [ "cli" ] }
 proxmox-schema = { version = "1.3.1", features = [ "api-macro" ] }
 proxmox-section-config = "1"
 proxmox-tfa = { version = "2", features = [ "api", "api-types" ] }
diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
index a0c30cca..3f6a6e03 100644
--- a/src/api2/node/tasks.rs
+++ b/src/api2/node/tasks.rs
@@ -1,11 +1,21 @@
 use std::fs::File;
 use std::io::{BufRead, BufReader};
+use std::path::PathBuf;
 
 use anyhow::{bail, Error};
+use futures::FutureExt;
+use http::request::Parts;
+use http::{header, Response, StatusCode};
+use hyper::Body;
+use proxmox_async::stream::AsyncReaderStream;
 use serde_json::{json, Value};
 
-use proxmox_router::{list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap};
-use proxmox_schema::api;
+use proxmox_router::{
+    list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router,
+    RpcEnvironment, SubdirMap,
+};
+use proxmox_schema::{api, IntegerSchema, Schema};
+use proxmox_schema::{BooleanSchema, ObjectSchema};
 use proxmox_sys::sortable;
 
 use pbs_api_types::{
@@ -19,6 +29,20 @@ use crate::api2::pull::check_pull_privs;
 use pbs_config::CachedUserInfo;
 use proxmox_rest_server::{upid_log_path, upid_read_status, TaskListInfoIterator, TaskState};
 
+pub const START_PARAM_SCHEMA: Schema =
+    IntegerSchema::new("Start at this line when reading the tasklog")
+        .minimum(0)
+        .schema();
+
+pub const LIMIT_PARAM_SCHEMA: Schema =
+    IntegerSchema::new("The amount of lines to read from the tasklog")
+        .minimum(0)
+        .schema();
+
+pub const TEST_STATUS_PARAM_SCHEMA: Schema =
+    BooleanSchema::new("Test task status, and set result attribute \"active\" accordingly.")
+        .schema();
+
 // matches respective job execution privileges
 fn check_job_privs(auth_id: &Authid, user_info: &CachedUserInfo, upid: &UPID) -> Result<(), Error> {
     match (upid.worker_type.as_str(), &upid.worker_id) {
@@ -259,58 +283,89 @@ fn extract_upid(param: &Value) -> Result<UPID, Error> {
     upid_str.parse::<UPID>()
 }
 
-#[api(
-    input: {
-        properties: {
-            node: {
-                schema: NODE_SCHEMA,
-            },
-            upid: {
-                schema: UPID_SCHEMA,
-            },
-            "test-status": {
-                type: bool,
-                optional: true,
-                description: "Test task status, and set result attribute \"active\" accordingly.",
-            },
-            start: {
-                type: u64,
-                optional: true,
-                description: "Start at this line.",
-                default: 0,
-            },
-            limit: {
-                type: u64,
-                optional: true,
-                description: "Only list this amount of lines.",
-                default: 50,
-            },
-        },
-    },
-    access: {
-        description: "Users can access their own tasks, or need Sys.Audit on /system/tasks.",
-        permission: &Permission::Anybody,
-    },
-)]
-/// Read task log.
-async fn read_task_log(param: Value, mut rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
-    let upid = extract_upid(&param)?;
+#[sortable]
+pub const API_METHOD_READ_TASK_LOG: ApiMethod = ApiMethod::new(
+    &ApiHandler::AsyncHttp(&download_task_log),
+    &ObjectSchema::new(
+        "Read the task log",
+        &sorted!([
+            ("node", false, &NODE_SCHEMA),
+            ("upid", false, &UPID_SCHEMA),
+            ("start", true, &START_PARAM_SCHEMA),
+            ("limit", true, &LIMIT_PARAM_SCHEMA),
+            ("test-status", true, &TEST_STATUS_PARAM_SCHEMA)
+        ]),
+    ),
+)
+.access(
+    Some("Users can access their own tasks, or need Sys.Audit on /system/tasks."),
+    &Permission::Anybody,
+);
+fn download_task_log(
+    _parts: Parts,
+    _req_body: Body,
+    param: Value,
+    _info: &ApiMethod,
+    rpcenv: Box<dyn RpcEnvironment>,
+) -> ApiResponseFuture {
+    async move {
+        let upid: UPID = extract_upid(&param)?;
 
-    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        check_task_access(&auth_id, &upid)?;
 
-    check_task_access(&auth_id, &upid)?;
+        let test_status = param["test-status"].as_bool().unwrap_or(false);
+        let start = param["start"].as_u64().unwrap_or(0);
+        let limit = param["limit"].as_u64().unwrap_or(50);
 
-    let test_status = param["test-status"].as_bool().unwrap_or(false);
+        let path = upid_log_path(&upid)?;
 
-    let start = param["start"].as_u64().unwrap_or(0);
-    let mut limit = param["limit"].as_u64().unwrap_or(50);
+        if limit == 0 {
+            let file = tokio::fs::File::open(path).await?;
 
-    let mut count: u64 = 0;
+            let header_disp = format!("attachment; filename={}", &upid.to_string());
+
+            let stream = AsyncReaderStream::new(file);
+
+            Ok(Response::builder()
+                .status(StatusCode::OK)
+                .header(header::CONTENT_TYPE, "text/plain")
+                .header(header::CONTENT_DISPOSITION, &header_disp)
+                .body(Body::wrap_stream(stream))
+                .unwrap())
+        } else {
+            let (count, lines) = read_tasklog_lines(path, start, limit).unwrap();
 
-    let path = upid_log_path(&upid)?;
+            let mut json = json!({
+                "data": lines,
+                "total": count,
+                "success": 1,
+            });
 
+            if test_status {
+                let active = proxmox_rest_server::worker_is_active(&upid).await?;
+
+                json["test-status"] = Value::from(active);
+            }
+
+            Ok(Response::builder()
+                .status(StatusCode::OK)
+                .header(header::CONTENT_TYPE, "application/json")
+                .body(Body::from(json.to_string()))
+                .unwrap())
+        }
+    }
+    .boxed()
+}
+
+fn read_tasklog_lines(
+    path: PathBuf,
+    start: u64,
+    mut limit: u64,
+) -> Result<(u64, Vec<Value>), Error> {
     let file = File::open(path)?;
 
+    let mut count: u64 = 0;
     let mut lines: Vec<Value> = vec![];
 
     for line in BufReader::new(file).lines() {
@@ -335,14 +390,7 @@ async fn read_task_log(param: Value, mut rpcenv: &mut dyn RpcEnvironment) -> Res
         }
     }
 
-    rpcenv["total"] = Value::from(count);
-
-    if test_status {
-        let active = proxmox_rest_server::worker_is_active(&upid).await?;
-        rpcenv["active"] = Value::from(active);
-    }
-
-    Ok(json!(lines))
+    Ok((count, lines))
 }
 
 #[api(
diff --git a/www/Subscription.js b/www/Subscription.js
index b641f2f3..83e31d6b 100644
--- a/www/Subscription.js
+++ b/www/Subscription.js
@@ -63,19 +63,7 @@ Ext.define('PBS.Subscription', {
 			var fileContent = Ext.String.htmlDecode(reportWindow.getComponent('system-report-view').html);
 			var fileName = getReportFileName();
 
-			// Internet Explorer
-			if (window.navigator.msSaveOrOpenBlob) {
-			    navigator.msSaveOrOpenBlob(new Blob([fileContent]), fileName);
-			} else {
-			    var element = document.createElement('a');
-			    element.setAttribute('href', 'data:text/plain;charset=utf-8,' +
-			      encodeURIComponent(fileContent));
-			    element.setAttribute('download', fileName);
-			    element.style.display = 'none';
-			    document.body.appendChild(element);
-			    element.click();
-			    document.body.removeChild(element);
-			}
+			Proxmox.Utils.downloadStringAsFile(fileContent, fileName);
 		    },
 		},
 	    ],
diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index 1be63e0c..d9efdf6a 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -590,16 +590,13 @@ Ext.define('PBS.DataStoreContent', {
 
 	    let idx = file.lastIndexOf('.');
 	    let filename = file.slice(0, idx);
-	    let atag = document.createElement('a');
-	    params['file-name'] = file;
-	    atag.download = filename;
 	    let url = new URL(`/api2/json/admin/datastore/${view.datastore}/download-decoded`,
 	                      window.location.origin);
 	    for (const [key, value] of Object.entries(params)) {
 		url.searchParams.append(key, value);
 	    }
-	    atag.href = url.href;
-	    atag.click();
+
+	    Proxmox.Utils.downloadAsFile(url.href, filename);
 	},
 
 	openPxarBrowser: function(tv, rI, Ci, item, e, rec) {
-- 
2.30.2

[pve-devel] [PATCH http-server 1/1] fix #3971: Make tasklog downloadable

[Daniel Tschlatscher]

To make files downloable via download stream the content-dispostion
header needs to be acknowledged.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 src/PVE/APIServer/AnyEvent.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index 7dd7d2d..8c5d74e 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -419,6 +419,7 @@ sub send_file_start {
 	    if (ref($download) eq 'HASH') {
 		$mime = $download->{'content-type'};
 		my $encoding = $download->{'content-encoding'};
+		my $disposition = $download->{'content-disposition'};
 
 		if ($download->{path} && $download->{stream} &&
 		    $reqstate->{request}->header('PVEDisableProxy'))
@@ -431,6 +432,7 @@ sub send_file_start {
 			Content_Type => $mime,
 		    );
 		    $header->header('Content-Encoding' => $encoding) if defined($encoding);
+		    $header->header('Content-Disposition' => $disposition) if defined($disposition);
 		    # we need some data so Content-Length gets set correctly and
 		    # the proxy doesn't wait for more data - place a canary
 		    my $resp = HTTP::Response->new(200, "OK", $header, "error canary");
@@ -449,6 +451,7 @@ sub send_file_start {
 		if ($download->{stream}) {
 		    my $header = HTTP::Headers->new(Content_Type => $mime);
 		    $header->header('Content-Encoding' => $encoding) if defined($encoding);
+		    $header->header('Content-Disposition' => $disposition) if defined($disposition);
 		    my $resp = HTTP::Response->new(200, "OK", $header);
 		    $self->response($reqstate, $resp, undef, 1, 0, $fh);
 		    return;
-- 
2.30.2

[pve-devel] [PATCH common 1/1] fix #3971: Create log file stream for download

[Daniel Tschlatscher]

Creating the filestream for the tasklog download is sourced in its own
function to avoid redundant implementations in pmg-api and pve-manager
.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 src/PVE/Tools.pm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
index dac0a2b..ec8681a 100644
--- a/src/PVE/Tools.pm
+++ b/src/PVE/Tools.pm
@@ -2016,4 +2016,31 @@ sub get_file_hash {
     return lc($digest);
 }
 
+sub stream_logfile {
+    my ($filename, $compress, $suggested_name) = @_;
+
+    my $fh;
+    $suggested_name = $suggested_name // $filename;
+
+    if ($compress) {
+	my $cmd = ["/usr/bin/gzip", "-c", "$filename"];
+
+	open($fh, "-|", join(' ', @$cmd))
+	    or die "Could not create compressed file stream for $filename - $!";
+    } else {
+	open($fh, '<', $filename)
+	    or die "Could not open file $filename - $!";
+    }
+
+    return {
+	download => {
+	    fh => $fh,
+	    stream => 1,
+	    'content-encoding' => $compress ? 'gzip' : undef,
+	    'content-type' => 'text/plain',
+	    'content-disposition' => "attachment; filename=\"$suggested_name\"",
+	},
+    },
+}
+
 1;
-- 
2.30.2

[pve-devel] [PATCH manager 1/1] fix #3971: Revised task log API call when parameter 'limit' is 0

[Daniel Tschlatscher]

The API call for fetching a tasklog with limit=0 now returns the whole
log as a file stream rather than reading all lines in memory and then
transfering them in JSON format. The behaviour when the url parameter
limit is undefined or not 0 is the same as before, in accordance
with the API specification.

Also replaced the system-report and PBSEdit file download with a call
to the newly added downloadStringAsFile() function in the proxmox-
widget-toolkit repository in the Utils class (Proxmox.Utils).

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 PVE/API2/Tasks.pm                 | 13 ++++++++++---
 www/manager6/node/Subscription.js | 20 ++++----------------
 2 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/PVE/API2/Tasks.pm b/PVE/API2/Tasks.pm
index 9cd1e56b..5572f826 100644
--- a/PVE/API2/Tasks.pm
+++ b/PVE/API2/Tasks.pm
@@ -5,6 +5,7 @@ use warnings;
 use POSIX;
 use IO::File;
 use File::ReadBackwards;
+use File::stat;
 use PVE::Tools;
 use PVE::SafeSyslog;
 use PVE::RESTHandler;
@@ -387,11 +388,17 @@ __PACKAGE__->register_method({
 	    $rpcenv->check($user, "/nodes/$node", [ 'Sys.Audit' ]);
 	}
 
-	my ($count, $lines) = PVE::Tools::dump_logfile($filename, $start, $limit);
+	if ($limit == 0) {
+	    # TCP Max Transfer Unit size is 1500, compression for lower numbers has no effect
+	    my $use_compression = stat($filename)->size > 1500;
+	    return PVE::Tools::stream_logfile($filename, $use_compression, $param->{upid});
+	} else {
+	    my ($count, $lines) = PVE::Tools::dump_logfile($filename, $start, $limit);
 
-	$rpcenv->set_result_attrib('total', $count);
+	    $rpcenv->set_result_attrib('total', $count);
 
-	return $lines;
+	    return $lines;
+	}
     }});
 
 
diff --git a/www/manager6/node/Subscription.js b/www/manager6/node/Subscription.js
index 2558f523..147eeed1 100644
--- a/www/manager6/node/Subscription.js
+++ b/www/manager6/node/Subscription.js
@@ -58,22 +58,10 @@ Ext.define('PVE.node.Subscription', {
 		{
 		    text: gettext('Download'),
 		    handler: function() {
-			var fileContent = Ext.String.htmlDecode(reportWindow.getComponent('system-report-view').html);
-			var fileName = getReportFileName();
-
-			// Internet Explorer
-			if (window.navigator.msSaveOrOpenBlob) {
-			    navigator.msSaveOrOpenBlob(new Blob([fileContent]), fileName);
-			} else {
-			    var element = document.createElement('a');
-			    element.setAttribute('href', 'data:text/plain;charset=utf-8,' +
-			      encodeURIComponent(fileContent));
-			    element.setAttribute('download', fileName);
-			    element.style.display = 'none';
-			    document.body.appendChild(element);
-			    element.click();
-			    document.body.removeChild(element);
-			}
+			let fileContent = Ext.String.htmlDecode(reportWindow.getComponent('system-report-view').html);
+			let fileName = getReportFileName();
+
+			Proxmox.Utils.downloadStringAsFile(fileContent, fileName);
 		    },
 		},
 	    ],
-- 
2.30.2

[pve-devel] [PATCH pmg-api 1/1] fix #3971: Download button in TaskViewer for PMG

[Daniel Tschlatscher]

With the newly added button in the tasklog the implementation for the
PMG server needs to be adapted. I saw an opportunity here to clear
some redundant code for displaying the tasklog and replace it with a
call to dump_logfile(), akin to how this is handled in pve-manager.

The tasklog download functionality now streams the file by invoking
the newly created function in pve-common. Tasklog files can become
rather large, therefore sent files are compressed if they are bigger
than 1500 bytes (The 'Maximum Transfer Unit' for TCP). Smaller files
will not benefit from compression here, as they still use one (1)
data unit packet to transfer.
Please note that the size of 1500 bytes / MTU can vary, but seems to
the most common for internet networking.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 src/PMG/API2/Tasks.pm | 34 +++++++++++-----------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/src/PMG/API2/Tasks.pm b/src/PMG/API2/Tasks.pm
index 598fb4d..20f6f96 100644
--- a/src/PMG/API2/Tasks.pm
+++ b/src/PMG/API2/Tasks.pm
@@ -5,6 +5,7 @@ use warnings;
 use POSIX;
 use IO::File;
 use File::ReadBackwards;
+use File::stat;
 use PVE::Tools;
 use PVE::SafeSyslog;
 use PVE::RESTHandler;
@@ -272,33 +273,20 @@ __PACKAGE__->register_method({
 
 	my $restenv = PMG::RESTEnvironment->get();
 
-	my $fh = IO::File->new($filename, "r");
-	raise_param_exc({ upid => "no such task - unable to open file - $!" }) if !$fh;
+	my $start = $param->{start} // 0;
+	my $limit = $param->{limit} // 50;
 
-	my $start = $param->{start} || 0;
-	my $limit = $param->{limit} || 50;
-	my $count = 0;
-	my $line;
-	while (defined ($line = <$fh>)) {
-	    next if $count++ < $start;
-	    next if $limit <= 0;
-	    chomp $line;
-	    push @$lines, { n => $count, t => $line};
-	    $limit--;
-	}
+	if ($limit == 0) {
+	    # TCP Max Transfer Unit size is 1500, compression for lower numbers has no effect
+	    my $use_compression = stat($filename)->size > 1500;
+	    return PVE::Tools::stream_logfile($filename, $use_compression, $param->{upid});
+	} else {
+	    my ($count, $lines) = PVE::Tools::dump_logfile($filename, $start, $limit);
 
-	close($fh);
+	    $restenv->set_result_attrib('total', $count);
 
-	# HACK: ExtJS store.guaranteeRange() does not like empty array
-	# so we add a line
-	if (!$count) {
-	    $count++;
-	    push @$lines, { n => $count, t => "no content"};
+	    return $lines;
 	}
-
-	$restenv->set_result_attrib('total', $count);
-
-	return $lines;
     }});
 
 
-- 
2.30.2

[pve-devel] [PATCH widget-toolkit 1/1] fix #3971: Download button in the TaskViewer

[Daniel Tschlatscher]

The taskviewer now has a button which implements functionality for
downloading the current tasklog as a file. The code for saving the log
to a file was taken from the pve System Report class, adapted and
moved to its own function in the Utils file.

Also simplified the PVE-Subscription report file download to use the
newly created function in the Utils file. I simplified the "download
as file" calls for invokations in the FileBrowser UI as well.

Tested on Firefox 91.7.0esr and Chromium Version 99 in the PMG, PBS
and the PVE webinterfaces.

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 src/Utils.js              | 18 ++++++++++++++++++
 src/window/FileBrowser.js | 12 +++---------
 src/window/TaskViewer.js  | 16 ++++++++++++++--
 3 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/Utils.js b/src/Utils.js
index 6a03057..8d4d1cc 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -1272,6 +1272,24 @@ utilities: {
 	    .map(val => val.charCodeAt(0)),
 	);
     },
+
+    // Setting filename here when downloading from a remote url sometimes fails in chromium browsers
+    // because of a bug when using attribute download in conjunction with a self signed certificate.
+    // For more info see https://bugs.chromium.org/p/chromium/issues/detail?id=993362
+    downloadAsFile: function(source, fileName) {
+	let hiddenElement = document.createElement('a');
+	hiddenElement.href = source;
+	hiddenElement.target = '_blank';
+	if (fileName) {
+	    hiddenElement.download = fileName;
+	}
+	hiddenElement.click();
+    },
+
+    downloadStringAsFile: function(fileContent, fileName) {
+	let source = `data:attachment/text;charset=utf-8,${encodeURIComponent(fileContent)}`;
+	this.downloadAsFile(source, fileName);
+    },
 },
 
     singleton: true,
diff --git a/src/window/FileBrowser.js b/src/window/FileBrowser.js
index 99a7a85..2dac7c9 100644
--- a/src/window/FileBrowser.js
+++ b/src/window/FileBrowser.js
@@ -98,17 +98,11 @@ Ext.define("Proxmox.window.FileBrowser", {
 
 	    let data = selection[0].data;
 
-	    let atag = document.createElement('a');
-
-	    atag.download = data.text;
 	    let params = { ...view.extraParams };
 	    params.filepath = data.filepath;
-	    atag.download = data.text;
-	    if (data.type === 'd') {
-		atag.download += ".zip";
-	    }
-	    atag.href = me.buildUrl(view.downloadURL, params);
-	    atag.click();
+	    let filename = data.type === 'd' ? `${data.text}.zip` : data.text;
+
+	    Proxmox.Utils.downloadAsFile(me.buildUrl(view.downloadURL, params), filename);
 	},
 
 	fileChanged: function() {
diff --git a/src/window/TaskViewer.js b/src/window/TaskViewer.js
index 996a41b..e47a4d8 100644
--- a/src/window/TaskViewer.js
+++ b/src/window/TaskViewer.js
@@ -229,11 +229,22 @@ Ext.define('Proxmox.window.TaskViewer', {
 	    border: false,
 	});
 
+	let downloadBtn = new Ext.Button({
+	    text: gettext('Download'),
+	    iconCls: 'fa fa-download',
+	    handler: function() {
+		let url = '/api2/extjs/nodes/' +
+		    `${task.node}/tasks/${encodeURIComponent(me.upid)}/log?limit=0`;
+
+		Proxmox.Utils.downloadAsFile(url);
+	    },
+	});
+
 	let logView = Ext.create('Proxmox.panel.LogView', {
 	    title: gettext('Output'),
-	    tbar: [stop_btn2],
+	    tbar: [stop_btn2, '->', downloadBtn],
 	    border: false,
-	    url: "/api2/extjs/nodes/" + task.node + "/tasks/" + encodeURIComponent(me.upid) + "/log",
+	    url: `/api2/extjs/nodes/${task.node}/tasks/${encodeURIComponent(me.upid)}/log`,
 	});
 
 	me.mon(statstore, 'load', function() {
@@ -248,6 +259,7 @@ Ext.define('Proxmox.window.TaskViewer', {
 
 	    stop_btn1.setDisabled(status !== 'running');
 	    stop_btn2.setDisabled(status !== 'running');
+	    downloadBtn.setDisabled(status === 'running');
 	});
 
 	statstore.startUpdate();
-- 
2.30.2

[pve-devel] [PATCH pmg-gui 1/1] Replaced the system-report file download implementation

[Daniel Tschlatscher]

with the newly added downloadStringAsFile() function in the proxmox-
widget-toolkit repository in the Utils class (Proxmox.Utils).

Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
---
 js/Subscription.js | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/js/Subscription.js b/js/Subscription.js
index 8d3bc80..16c4c82 100644
--- a/js/Subscription.js
+++ b/js/Subscription.js
@@ -62,19 +62,7 @@ Ext.define('PMG.Subscription', {
 			let fileContent = Ext.String.htmlDecode(reportWindow.getComponent('system-report-view').html);
 			let fileName = getReportFileName();
 
-			// Internet Explorer
-			if (window.navigator.msSaveOrOpenBlob) {
-			    navigator.msSaveOrOpenBlob(new Blob([fileContent]), fileName);
-			} else {
-			    let element = document.createElement('a');
-			    element.setAttribute('href', 'data:text/plain;charset=utf-8,'
-			      + encodeURIComponent(fileContent));
-			    element.setAttribute('download', fileName);
-			    element.style.display = 'none';
-			    document.body.appendChild(element);
-			    element.click();
-			    document.body.removeChild(element);
-			}
+			Proxmox.Utils.downloadStringAsFile(fileContent, fileName);
 		    },
 		},
 	    ],
-- 
2.30.2

Re: [pve-devel] [PATCH common 1/1] fix #3971: Create log file stream for download

[Thomas Lamprecht]

On 26.04.22 14:35, Daniel Tschlatscher wrote:
> Creating the filestream for the tasklog download is sourced in its own
> function to avoid redundant implementations in pmg-api and pve-manager
> .
> 
> Signed-off-by: Daniel Tschlatscher <d.tschlatscher@proxmox.com>
> ---
>  src/PVE/Tools.pm | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index dac0a2b..ec8681a 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -2016,4 +2016,31 @@ sub get_file_hash {
>      return lc($digest);
>  }
>  
> +sub stream_logfile {

could be a more generic `stream_file` too with an additional $content_type param that defaults
to 'text/plain', as there's nothing really log specific in there.

But just for that you won't need to send a v2, I'll need to look at the rest more closely
later and may only get around that earliest in the next week .

Re: [pve-devel] [PATCH http/common/manager/wt/proxmox-backup/pmg] Tasklog download button

[Daniel Tschlatscher]

ping

(And now that I see it, the patches should actually count up to 7, they
are not in fact 1/1 standalone patches)

On 4/26/22 14:35, Daniel Tschlatscher wrote:
> This patch series' aim is to add a download button in the tasklog-
> viewer GUI so that users may access all parts of the tasklog more
> easily (The tasklog-viewer only displays 50 lines at a time).
> 
> For this change in the GUI (proxmox-widget-toolkit) there were revised
> implementations for all backends (PVE, PMG and PBS) needed.
> The change was implemented by setting the URL parameter 'limit' for
> the .../log call and streaming a file in this case. (Before this
> call returned 0 lines).
> 
> I also revised a few different parts in the code which had redundant
> implementations for "downloading" a file locally from a string.
> For this I added a function in the proxmox-widget-toolkit Utils class
> which all occurences (that I found) now call.
> 
> During the implementation of the file stream download I found a bug
> concerning chromium browsers because of which downloads "randomly"
> fail.
> The problem was, that when implementing the download through an html a
> tag and setting its property "download" to anything (even undefined)
> with a self-signed certificate (not imported) will fail after the
> SSL connection is reset.
> In my testing this was the case after about 5 seconds. This problem is
> especially pronounced in the PMG as there are less background calls in
> contrast to the PVE. However, the PBS does not seem to suffer this
> error at all, because the SSL handshake timeout seems to not run out.
>  
> Without the 'download' attribute you can't set the filename though.
> The file will only have a proper name if the server adds the correct
> 'content-disposition' header. A few calls to the 'downloadAsFile'
> function set the filename anyway, because in most cases the browser
> frontend is making enough polling calls so that the SSL handshake
> does not have to be executed for the download. (That is also why it
> probably did not come up before)
> This problem does not apply in Firefox as far as I could tell. 
> 
> Daniel Tschlatscher (1):
>   fix #3971: tasklog download in the backup server backend
> 
>  Cargo.toml               |   2 +-
>  src/api2/node/tasks.rs   | 154 +++++++++++++++++++++++++--------------
>  www/Subscription.js      |  14 +---
>  www/datastore/Content.js |   7 +-
>  4 files changed, 105 insertions(+), 72 deletions(-)
> 
> Daniel Tschlatscher (1):
>   fix #3971: Make tasklog downloadable
> 
>  src/PVE/APIServer/AnyEvent.pm | 3 +++
>  1 file changed, 3 insertions(+)
> 
> Daniel Tschlatscher (1):
>   fix #3971: Create log file stream for download
> 
>  src/PVE/Tools.pm | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> Daniel Tschlatscher (1):
>   fix #3971: Revised task log API call when parameter 'limit' is 0
> 
>  PVE/API2/Tasks.pm                 | 13 ++++++++++---
>  www/manager6/node/Subscription.js | 20 ++++----------------
>  2 files changed, 14 insertions(+), 19 deletions(-)
> 
> Daniel Tschlatscher (1):
>   fix #3971: Download button in TaskViewer for PMG
> 
>  src/PMG/API2/Tasks.pm | 34 +++++++++++-----------------------
>  1 file changed, 11 insertions(+), 23 deletions(-)
> 
> Daniel Tschlatscher (1):
>   fix #3971: Download button in the TaskViewer
> 
>  src/Utils.js              | 18 ++++++++++++++++++
>  src/window/FileBrowser.js | 12 +++---------
>  src/window/TaskViewer.js  | 16 ++++++++++++++--
>  3 files changed, 35 insertions(+), 11 deletions(-)
> 
> Daniel Tschlatscher (1):
>   Replaced the system-report file download implementation
> 
>  js/Subscription.js | 14 +-------------
>  1 file changed, 1 insertion(+), 13 deletions(-)