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

[Stefan Sterz <s.sterz@proxmox.com>]

On 2/9/23 15:16, Wolfgang Bumiller wrote:
> 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?
> 

yeah this has been discussed briefly here in the bugtracker [1].
underscores sound good to me.

[1]: https://bugzilla.proxmox.com/show_bug.cgi?id=4521

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