[pbs-devel] [RFC PATCH widget-toolkit/backup] percent encode the task status line

[pbs-devel] [RFC PATCH widget-toolkit/backup] percent encode the task status line

[Dominik Csapak]

to avoid having a newline (and other control characters) in the task status,
percent-encode the last line

decoding happens in the web ui

alternatively, it would make sense to decode it again in the backend
directly, since only the writing into the logfile is problematic. this would
make the api a bit nicer, since an api user would not have to do this
themselves. on the other hand, not having control characters in the api
result makes it a bit safer i suppose...

proxmox-widget-toolkit:

Dominik Csapak (1):
  Utils: percent-decode the task status

 src/Utils.js             | 12 +++++++++++-
 src/panel/LogView.js     |  6 ++++++
 src/window/TaskViewer.js |  2 +-
 3 files changed, 18 insertions(+), 2 deletions(-)

proxmox-backup:

Dominik Csapak (1):
  rest-server: tasks: percent-encode the task status line

 pbs-tools/src/percent_encoding.rs      | 5 +++++
 proxmox-rest-server/src/worker_task.rs | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.30.2

[pbs-devel] [RFC PATCH widget-toolkit 1/1] Utils: percent-decode the task status

[Dominik Csapak]

we percent-encode the taskstatus in case of an error, so that we do not
have control characters (especially newlines) in the task log

so on showing, we decode that again

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/Utils.js             | 12 +++++++++++-
 src/panel/LogView.js     |  6 ++++++
 src/window/TaskViewer.js |  2 +-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/Utils.js b/src/Utils.js
index c52bef2..e18a403 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -785,11 +785,21 @@ utilities: {
 	return 'error';
     },
 
+    percent_decode_ctrl_chars: function(status) {
+	return status.replaceAll(/%([0-9a-f]{2})/ig, (match, val) => {
+	    let value = parseInt(val, 16);
+	    if (value < 32 || value === 127) { // 0x00-0x1F && 0x7F (DEL)
+		return String.fromCharCode(value);
+	    }
+	    return match;
+	});
+    },
+
     format_task_status: function(status) {
 	let parsed = Proxmox.Utils.parse_task_status(status);
 	switch (parsed) {
 	    case 'unknown': return Proxmox.Utils.unknownText;
-	    case 'error': return Proxmox.Utils.errorText + ': ' + status;
+	    case 'error': return Proxmox.Utils.errorText + ': ' + Proxmox.Utils.percent_decode_ctrl_chars(status);
 	    case 'warning': return status.replace('WARNINGS', Proxmox.Utils.warningsText);
 	    case 'ok': // fall-through
 	    default: return status;
diff --git a/src/panel/LogView.js b/src/panel/LogView.js
index 1772737..4558772 100644
--- a/src/panel/LogView.js
+++ b/src/panel/LogView.js
@@ -97,6 +97,12 @@ Ext.define('Proxmox.panel.LogView', {
 			lines[line.n - 1] = Ext.htmlEncode(line.t);
 		    });
 
+		    // if last line contains a task error
+		    let last_line = lines[lines.length - 1];
+		    if (last_line.indexOf('TASK ERROR') !== -1) {
+			lines[lines.length - 1] = Proxmox.Utils.percent_decode_ctrl_chars(last_line);
+		    }
+
 		    lines.length = total;
 		    me.updateView(lines.join('<br>'), first - 1, total);
 		    me.running = false;
diff --git a/src/window/TaskViewer.js b/src/window/TaskViewer.js
index 996a41b..5444842 100644
--- a/src/window/TaskViewer.js
+++ b/src/window/TaskViewer.js
@@ -113,7 +113,7 @@ Ext.define('Proxmox.window.TaskViewer', {
 		    }
 		    let es = statgrid.getObjectValue('exitstatus');
 		    if (es) {
-			return value + ': ' + es;
+			return value + ': ' + Proxmox.Utils.percent_decode_ctrl_chars(es);
 		    }
 		    return 'unknown';
 		},
-- 
2.30.2

[pbs-devel] [RFC PATCH proxmox-backup 1/1] rest-server: tasks: percent-encode the task status line

[Dominik Csapak]

we always want the status line of the task log to be a single line
(makes the parsing easier), for that purpose, percent-encode it
(we can decode it again when we need to)

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pbs-tools/src/percent_encoding.rs      | 5 +++++
 proxmox-rest-server/src/worker_task.rs | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/pbs-tools/src/percent_encoding.rs b/pbs-tools/src/percent_encoding.rs
index afe011e2..dae9a50d 100644
--- a/pbs-tools/src/percent_encoding.rs
+++ b/pbs-tools/src/percent_encoding.rs
@@ -20,3 +20,8 @@ pub const DEFAULT_ENCODE_SET: &AsciiSet = &percent_encoding::CONTROLS // 0..1f a
 pub fn percent_encode_component(comp: &str) -> String {
     utf8_percent_encode(comp, percent_encoding::NON_ALPHANUMERIC).to_string()
 }
+
+/// percent encode control characters in a string
+pub fn percent_encode_control_characters(comp: &str) -> String {
+    utf8_percent_encode(comp, percent_encoding::CONTROLS).to_string()
+}
diff --git a/proxmox-rest-server/src/worker_task.rs b/proxmox-rest-server/src/worker_task.rs
index fb0dea9b..fb101b50 100644
--- a/proxmox-rest-server/src/worker_task.rs
+++ b/proxmox-rest-server/src/worker_task.rs
@@ -23,6 +23,7 @@ use proxmox_schema::upid::UPID;
 
 use pbs_tools::task::WorkerTaskContext;
 use pbs_tools::logrotate::{LogRotate, LogRotateFiles};
+use pbs_tools::percent_encoding::percent_encode_control_characters;
 
 use crate::{CommandSocket, FileLogger, FileLogOptions};
 
@@ -853,7 +854,8 @@ impl WorkerTask {
     /// Log task result, remove task from running list
     pub fn log_result(&self, result: &Result<(), Error>) {
         let state = self.create_state(result);
-        self.log_message(state.result_text());
+        // percent encode it to avoid control characters (e.g. newlines in the status line)
+        self.log_message(percent_encode_control_characters(&state.result_text()));
 
         WORKER_TASK_LIST.lock().unwrap().remove(&self.upid.task_id);
         let _ = self.setup.update_active_workers(None);
-- 
2.30.2