public inbox for pve-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal