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

[Daniel Tschlatscher <d.tschlatscher@proxmox.com>]

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