all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate
@ 2024-04-17 11:31 Maximiliano Sandoval
  2024-04-18  8:13 ` Folke Gleumes
  2024-04-18  8:43 ` Friedrich Weber
  0 siblings, 2 replies; 3+ messages in thread
From: Maximiliano Sandoval @ 2024-04-17 11:31 UTC (permalink / raw)
  To: pve-devel

Add support for compressing the body of responses with
`Content-Encoding: deflate` following [RFC9110]. Note that in this
context `deflate` is actually a "zlib" data format as defined in
[RFC1950].

To preserve the current behavior we prefer `Content-Encoding: gzip`
whenever `gzip` is listed as one of the encodings in the
`Accept-Encoding` header and the data should be compressed.

[RFC9110] https://www.rfc-editor.org/rfc/rfc9110#name-deflate-coding
[RFC1950] https://www.rfc-editor.org/rfc/rfc1950

Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Differences from v1:
 - The commit documents the behavior better
 - Add Suggested-by
 - We set both gzip and deflate in the Accept-Encoding header if available

 src/PVE/APIServer/AnyEvent.pm | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index b60b825..142c322 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -123,6 +123,7 @@ sub cleanup_reqstate {
     delete $reqstate->{request};
     delete $reqstate->{proto};
     delete $reqstate->{accept_gzip};
+    delete $reqstate->{accept_deflate};
     delete $reqstate->{starttime};
 
     if ($reqstate->{tmpfilename}) {
@@ -288,7 +289,7 @@ sub response {
     $reqstate->{hdl}->timeout($self->{timeout});
 
     $nocomp = 1 if !$self->{compression};
-    $nocomp = 1 if !$reqstate->{accept_gzip};
+    $nocomp = 1 if !$reqstate->{accept_gzip} && !$reqstate->{accept_deflate};
 
     my $code = $resp->code;
     my $msg = $resp->message || HTTP::Status::status_message($code);
@@ -333,11 +334,17 @@ sub response {
 	$content_length = length($content);
 
 	if (!$nocomp && ($content_length > 1024)) {
-	    my $comp = Compress::Zlib::memGzip($content);
-	    $resp->header('Content-Encoding', 'gzip');
-	    $content = $comp;
-	    $content_length = length($content);
+	    if ($reqstate->{accept_gzip}) {
+		my $comp = Compress::Zlib::memGzip($content);
+		$resp->header('Content-Encoding', 'gzip');
+		$content = $comp;
+	    } elsif ($reqstate->{accept_deflate}) {
+		my $comp = Compress::Zlib::compress($content);
+		$resp->header('Content-Encoding', 'deflate');
+		$content = $comp;
+	    }
 	}
+	$content_length = length($content);
 	$resp->header("Content-Length" => $content_length);
 	$reqstate->{log}->{content_length} = $content_length;
 
@@ -735,7 +742,15 @@ sub proxy_request {
 	    if $auth->{api_token};
 	$headers->{'CSRFPreventionToken'} = $auth->{token}
 	    if $auth->{token};
-	$headers->{'Accept-Encoding'} = 'gzip' if ($reqstate->{accept_gzip} && $self->{compression});
+	if ($self->{compression}) {
+	    if ($reqstate->{accept_deflate} && $reqstate->{accept_gzip}) {
+		$headers->{'Accept-Encoding'} = 'gzip, deflate' if $reqstate->{accept_deflate};
+	    } elsif ($reqstate->{accept_gzip}) {
+		$headers->{'Accept-Encoding'} = 'gzip' if $reqstate->{accept_gzip};
+	    } elsif ($reqstate->{accept_deflate}) {
+		$headers->{'Accept-Encoding'} = 'deflate' if $reqstate->{accept_gzip};
+	    }
+	}
 
 	if (defined(my $host = $reqstate->{request}->header('Host'))) {
 	    $headers->{Host} = $host;
@@ -1361,6 +1376,7 @@ sub process_header {
     my $conn = $request->header('Connection');
     my $accept_enc = $request->header('Accept-Encoding');
     $reqstate->{accept_gzip} = ($accept_enc && $accept_enc =~ m/gzip/) ? 1 : 0;
+    $reqstate->{accept_deflate} = ($accept_enc && $accept_enc =~ m/deflate/) ? 1 : 0;
 
     if ($conn) {
 	$reqstate->{keep_alive} = 0 if $conn =~ m/close/oi;
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate
  2024-04-17 11:31 [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate Maximiliano Sandoval
@ 2024-04-18  8:13 ` Folke Gleumes
  2024-04-18  8:43 ` Friedrich Weber
  1 sibling, 0 replies; 3+ messages in thread
From: Folke Gleumes @ 2024-04-18  8:13 UTC (permalink / raw)
  To: pve-devel

Gave this a test with:

Accept-encoding: deflate
Accept-encoding: deflate, gzip
Accept-encoding: foobar

Everything worked as expected, the first case returned a zlib
compressed file, the second gzip and the third just plaintext.
Consider this

Tested-by: Folke Gleumes <f.gleumes@proxmox.com>

On Wed, 2024-04-17 at 13:31 +0200, Maximiliano Sandoval wrote:
> Add support for compressing the body of responses with
> `Content-Encoding: deflate` following [RFC9110]. Note that in this
> context `deflate` is actually a "zlib" data format as defined in
> [RFC1950].
> 
> To preserve the current behavior we prefer `Content-Encoding: gzip`
> whenever `gzip` is listed as one of the encodings in the
> `Accept-Encoding` header and the data should be compressed.
> 
> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#name-deflate-coding
> [RFC1950] https://www.rfc-editor.org/rfc/rfc1950
> 
> Suggested-by: Lukas Wagner <l.wagner@proxmox.com>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> 
> Differences from v1:
>  - The commit documents the behavior better
>  - Add Suggested-by
>  - We set both gzip and deflate in the Accept-Encoding header if
> available
> 
>  src/PVE/APIServer/AnyEvent.pm | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/src/PVE/APIServer/AnyEvent.pm
> b/src/PVE/APIServer/AnyEvent.pm
> index b60b825..142c322 100644
> --- a/src/PVE/APIServer/AnyEvent.pm
> +++ b/src/PVE/APIServer/AnyEvent.pm
> @@ -123,6 +123,7 @@ sub cleanup_reqstate {
>      delete $reqstate->{request};
>      delete $reqstate->{proto};
>      delete $reqstate->{accept_gzip};
> +    delete $reqstate->{accept_deflate};
>      delete $reqstate->{starttime};
>  
>      if ($reqstate->{tmpfilename}) {
> @@ -288,7 +289,7 @@ sub response {
>      $reqstate->{hdl}->timeout($self->{timeout});
>  
>      $nocomp = 1 if !$self->{compression};
> -    $nocomp = 1 if !$reqstate->{accept_gzip};
> +    $nocomp = 1 if !$reqstate->{accept_gzip} && !$reqstate-
> >{accept_deflate};
>  
>      my $code = $resp->code;
>      my $msg = $resp->message || HTTP::Status::status_message($code);
> @@ -333,11 +334,17 @@ sub response {
>         $content_length = length($content);
>  
>         if (!$nocomp && ($content_length > 1024)) {
> -           my $comp = Compress::Zlib::memGzip($content);
> -           $resp->header('Content-Encoding', 'gzip');
> -           $content = $comp;
> -           $content_length = length($content);
> +           if ($reqstate->{accept_gzip}) {
> +               my $comp = Compress::Zlib::memGzip($content);
> +               $resp->header('Content-Encoding', 'gzip');
> +               $content = $comp;
> +           } elsif ($reqstate->{accept_deflate}) {
> +               my $comp = Compress::Zlib::compress($content);
> +               $resp->header('Content-Encoding', 'deflate');
> +               $content = $comp;
> +           }
>         }
> +       $content_length = length($content);
>         $resp->header("Content-Length" => $content_length);
>         $reqstate->{log}->{content_length} = $content_length;
>  
> @@ -735,7 +742,15 @@ sub proxy_request {
>             if $auth->{api_token};
>         $headers->{'CSRFPreventionToken'} = $auth->{token}
>             if $auth->{token};
> -       $headers->{'Accept-Encoding'} = 'gzip' if ($reqstate-
> >{accept_gzip} && $self->{compression});
> +       if ($self->{compression}) {
> +           if ($reqstate->{accept_deflate} && $reqstate-
> >{accept_gzip}) {
> +               $headers->{'Accept-Encoding'} = 'gzip, deflate' if
> $reqstate->{accept_deflate};
> +           } elsif ($reqstate->{accept_gzip}) {
> +               $headers->{'Accept-Encoding'} = 'gzip' if $reqstate-
> >{accept_gzip};
> +           } elsif ($reqstate->{accept_deflate}) {
> +               $headers->{'Accept-Encoding'} = 'deflate' if
> $reqstate->{accept_gzip};
> +           }
> +       }
>  
>         if (defined(my $host = $reqstate->{request}->header('Host')))
> {
>             $headers->{Host} = $host;
> @@ -1361,6 +1376,7 @@ sub process_header {
>      my $conn = $request->header('Connection');
>      my $accept_enc = $request->header('Accept-Encoding');
>      $reqstate->{accept_gzip} = ($accept_enc && $accept_enc =~
> m/gzip/) ? 1 : 0;
> +    $reqstate->{accept_deflate} = ($accept_enc && $accept_enc =~
> m/deflate/) ? 1 : 0;
>  
>      if ($conn) {
>         $reqstate->{keep_alive} = 0 if $conn =~ m/close/oi;

_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate
  2024-04-17 11:31 [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate Maximiliano Sandoval
  2024-04-18  8:13 ` Folke Gleumes
@ 2024-04-18  8:43 ` Friedrich Weber
  1 sibling, 0 replies; 3+ messages in thread
From: Friedrich Weber @ 2024-04-18  8:43 UTC (permalink / raw)
  To: Proxmox VE development discussion, Maximiliano Sandoval

On 17/04/2024 13:31, Maximiliano Sandoval wrote:
> [...]
> -	$headers->{'Accept-Encoding'} = 'gzip' if ($reqstate->{accept_gzip} && $self->{compression});
> +	if ($self->{compression}) {
> +	    if ($reqstate->{accept_deflate} && $reqstate->{accept_gzip}) {
> +		$headers->{'Accept-Encoding'} = 'gzip, deflate' if $reqstate->{accept_deflate};

Isn't this post-if redundant because we only enter the block if
$reqstate->{accept_deflate}?

> +	    } elsif ($reqstate->{accept_gzip}) {
> +		$headers->{'Accept-Encoding'} = 'gzip' if $reqstate->{accept_gzip};

same here?

> +	    } elsif ($reqstate->{accept_deflate}) {
> +		$headers->{'Accept-Encoding'} = 'deflate' if $reqstate->{accept_gzip};
> +	    }

Is this correct (accept_deflate in the elsif, accept_gzip in the
post-if)? Do we need a post-if here?


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-04-18  8:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 11:31 [pve-devel] [PATCH http-server v2] http: support Content-Encoding=deflate Maximiliano Sandoval
2024-04-18  8:13 ` Folke Gleumes
2024-04-18  8:43 ` Friedrich Weber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal