Re: [pbs-devel] [PATCH proxmox-backup] fix #4521: api/tasks: replace upid as filename for task log downloads

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Thu, Feb 09, 2023 at 12:41:39PM +0100, Stefan Sterz wrote:
> previously the upid would just be used without a file extension when
> downloading a task log. this lead to rather strange filenames that
> appeared unfamiliar to users as the upid is not very prevalent in the
> gui. set a proper file name based on the node name, worker type and a
> time stamp instead. also add the ".log" file extension to indicate
> that these files contain logs.
> 
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
>  src/api2/node/tasks.rs | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/api2/node/tasks.rs b/src/api2/node/tasks.rs
> index d386f805..b0a4f159 100644
> --- a/src/api2/node/tasks.rs
> +++ b/src/api2/node/tasks.rs
> @@ -341,7 +341,12 @@ fn read_task_log(
>                  bail!("Parameter 'download' cannot be used with other parameters");
>              }
>  
> -            let header_disp = format!("attachment; filename={}", &upid.to_string());
> +            let header_disp = format!(
> +                "attachment; filename=task-{}-{}-{}.log",
> +                upid.node,
> +                upid.worker_type,
> +                proxmox_time::epoch_to_rfc3339_utc(upid.starttime)?

Mostly fine, I'm not sure the timestamp format is a good idea?
It contains colons which seem to be turned to spaces by my browser.
Colons in http headers are a bit weird?
Maybe we should just strip them out or replace them with underscores or
something?

> +            );
>              let stream = AsyncReaderStream::new(tokio::fs::File::open(path).await?);
>  
>              return Ok(Response::builder()