[pmg-devel] [PATCH http-server 1/1] AnyEvent: extend send_file with content-type and delete

[Thomas Lamprecht]

On 9/26/19 12:28 PM, Dominik Csapak wrote:
> for pmg, we need to send temporary files (for the attachment quarantine),
> but we cannot know beforehand what content-type it is, so we
> optionally give it to send_file_start
> 
> also we may want to delete these temp files after reading, so we unlink
> them after we got the data out if the 'delete' parameter is given
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/APIServer/AnyEvent.pm | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/PVE/APIServer/AnyEvent.pm b/PVE/APIServer/AnyEvent.pm
> index 2e8ca47..dcb5c95 100644
> --- a/PVE/APIServer/AnyEvent.pm
> +++ b/PVE/APIServer/AnyEvent.pm
> @@ -293,7 +293,7 @@ my $file_extension_info = {
>  };
>  
>  sub send_file_start {
> -    my ($self, $reqstate, $filename) = @_;
> +    my ($self, $reqstate, $filename, $ct, $delete) = @_;

$mime instead of $ct? or $content_type written out, if possible
I really want to avoid those 1 to 3 letter acronyms not telling
anybody anything..

>  
>      eval {
>  	# print "SEND FILE $filename\n";
> @@ -322,14 +322,24 @@ sub send_file_start {
>  	    my $len = sysread($fh, $data,  $stat->size);
>  	    die "got short file\n" if !defined($len) || $len != $stat->size;
>  
> -	    my ($ext) = $filename =~ m/\.([^.]*)$/;
> -	    my $ext_info = $file_extension_info->{$ext};
> +	    # some files are temporary and we want to delete them after sending
> +	    if ($delete) {

maybe call thins and the param "delete_after_sent" 

I mean another alternative could be opening a filehandl/fd already at the
caller, then let the caller delete the file immediately there-after (the
file stays still useable, but unreferenced, until the fh/fd gets closed.
Then use the fh directly here and close it after reading it?

> +		unlink($filename);

Partly this could be nice as $filenames are always racy, while you add
the $$ part to avoid other workers to corrupt it it's still not 100%
ideal.. but that could be overlooked, no big issue here, in practice.

> +	    }
> +
> +	    my $nocomp;
> +	    if (!defined($ct)) {
> +		my ($ext) = $filename =~ m/\.([^.]*)$/;
> +		my $ext_info = $file_extension_info->{$ext};
>  
> -	    die "unable to detect content type" if !$ext_info;
> +		die "unable to detect content type" if !$ext_info;
> +		$ct = $ext_info->{ct};
> +		$nocomp = $ext_info->{nocomp};
> +	    }
>  
> -	    my $header = HTTP::Headers->new(Content_Type => $ext_info->{ct});
> +	    my $header = HTTP::Headers->new(Content_Type => $ct);
>  	    my $resp = HTTP::Response->new(200, "OK", $header, $data);
> -	    $self->response($reqstate, $resp, $mtime, $ext_info->{nocomp});
> +	    $self->response($reqstate, $resp, $mtime, $nocomp);
>  	};
>  	if (my $err = $@) {
>  	    $self->error($reqstate, 501, $err);
> @@ -769,7 +779,7 @@ sub handle_api2_request {
>  	if (defined(my $filename = $res->{download})) {
>  	    my $fh = IO::File->new($filename) ||
>  		die "unable to open file '$filename' - $!\n";
> -	    send_file_start($self, $reqstate, $filename);
> +	    send_file_start($self, $reqstate, $filename, $res->{'content-type'}, $res->{delete});
>  	    return;
>  	}
>  
>