* [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
@ 2024-07-16 9:22 Maximiliano Sandoval
2024-07-17 15:49 ` Max Carrara
2024-07-22 6:12 ` [pbs-devel] applied: " Thomas Lamprecht
0 siblings, 2 replies; 7+ messages in thread
From: Maximiliano Sandoval @ 2024-07-16 9:22 UTC (permalink / raw)
To: pbs-devel
As per [RFC9110] the Deflate encoding is a "zlib" data format. This
makes the rest-server compatible with the http-client.
[RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
By default the builder will use the default compression level. Please let me
know if the Level::Default should be set explicitly.
proxmox-rest-server/src/rest.rs | 35 ++++++++++++++++++---------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
index 3a3c287c..4be36074 100644
--- a/proxmox-rest-server/src/rest.rs
+++ b/proxmox-rest-server/src/rest.rs
@@ -30,7 +30,7 @@ use proxmox_router::{http_bail, http_err};
use proxmox_schema::{ObjectSchemaType, ParameterSchema};
use proxmox_async::stream::AsyncReaderStream;
-use proxmox_compression::{DeflateEncoder, Level};
+use proxmox_compression::DeflateEncoder;
use proxmox_log::FileLogger;
use crate::{
@@ -557,12 +557,13 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
CompressionMethod::Deflate.content_encoding(),
);
resp.map(|body| {
- Body::wrap_stream(DeflateEncoder::with_quality(
- TryStreamExt::map_err(body, |err| {
+ Body::wrap_stream(
+ DeflateEncoder::builder(TryStreamExt::map_err(body, |err| {
proxmox_lang::io_format_err!("error during compression: {}", err)
- }),
- Level::Default,
- ))
+ }))
+ .zlib(true)
+ .build(),
+ )
})
}
None => resp,
@@ -651,12 +652,13 @@ async fn handle_unformatted_api_request<Env: RpcEnvironment, S: 'static + BuildH
CompressionMethod::Deflate.content_encoding(),
);
resp.map(|body| {
- Body::wrap_stream(DeflateEncoder::with_quality(
- TryStreamExt::map_err(body, |err| {
+ Body::wrap_stream(
+ DeflateEncoder::builder(TryStreamExt::map_err(body, |err| {
proxmox_lang::io_format_err!("error during compression: {}", err)
- }),
- Level::Default,
- ))
+ }))
+ .zlib(true)
+ .build(),
+ )
})
}
None => resp,
@@ -711,7 +713,7 @@ async fn simple_static_file_download(
let mut response = match compression {
Some(CompressionMethod::Deflate) => {
- let mut enc = DeflateEncoder::with_quality(data, Level::Default);
+ let mut enc = DeflateEncoder::builder(data).zlib(true).build();
enc.compress_vec(&mut file, CHUNK_SIZE_LIMIT as usize)
.await?;
let mut response = Response::new(enc.into_inner().into());
@@ -752,10 +754,11 @@ async fn chunked_static_file_download(
header::CONTENT_ENCODING,
CompressionMethod::Deflate.content_encoding(),
);
- Body::wrap_stream(DeflateEncoder::with_quality(
- AsyncReaderStream::new(file),
- Level::Default,
- ))
+ Body::wrap_stream(
+ DeflateEncoder::builder(AsyncReaderStream::new(file))
+ .zlib(true)
+ .build(),
+ )
}
None => Body::wrap_stream(AsyncReaderStream::new(file)),
};
--
2.39.2
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-16 9:22 [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers Maximiliano Sandoval
@ 2024-07-17 15:49 ` Max Carrara
2024-07-18 16:08 ` Thomas Lamprecht
2024-07-22 6:12 ` [pbs-devel] applied: " Thomas Lamprecht
1 sibling, 1 reply; 7+ messages in thread
From: Max Carrara @ 2024-07-17 15:49 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On Tue Jul 16, 2024 at 11:22 AM CEST, Maximiliano Sandoval wrote:
> As per [RFC9110] the Deflate encoding is a "zlib" data format. This
> makes the rest-server compatible with the http-client.
>
> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>
> By default the builder will use the default compression level. Please let me
> know if the Level::Default should be set explicitly.
This patch looks simple enough to me and does what it says on the tin;
I can't really spot anything wrong here.
Regarding your comment: I think the default level is perfectly fine.
Of course, one could be pedantic and try to benchmark any differences
between the levels, but IMHO that's rather redundant - if there are any
potential gains here, they're most likely miniscule.
In other words: LGTM.
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
>
> proxmox-rest-server/src/rest.rs | 35 ++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
> index 3a3c287c..4be36074 100644
> --- a/proxmox-rest-server/src/rest.rs
> +++ b/proxmox-rest-server/src/rest.rs
> @@ -30,7 +30,7 @@ use proxmox_router::{http_bail, http_err};
> use proxmox_schema::{ObjectSchemaType, ParameterSchema};
>
> use proxmox_async::stream::AsyncReaderStream;
> -use proxmox_compression::{DeflateEncoder, Level};
> +use proxmox_compression::DeflateEncoder;
> use proxmox_log::FileLogger;
>
> use crate::{
> @@ -557,12 +557,13 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
> CompressionMethod::Deflate.content_encoding(),
> );
> resp.map(|body| {
> - Body::wrap_stream(DeflateEncoder::with_quality(
> - TryStreamExt::map_err(body, |err| {
> + Body::wrap_stream(
> + DeflateEncoder::builder(TryStreamExt::map_err(body, |err| {
> proxmox_lang::io_format_err!("error during compression: {}", err)
> - }),
> - Level::Default,
> - ))
> + }))
> + .zlib(true)
> + .build(),
> + )
> })
> }
> None => resp,
> @@ -651,12 +652,13 @@ async fn handle_unformatted_api_request<Env: RpcEnvironment, S: 'static + BuildH
> CompressionMethod::Deflate.content_encoding(),
> );
> resp.map(|body| {
> - Body::wrap_stream(DeflateEncoder::with_quality(
> - TryStreamExt::map_err(body, |err| {
> + Body::wrap_stream(
> + DeflateEncoder::builder(TryStreamExt::map_err(body, |err| {
> proxmox_lang::io_format_err!("error during compression: {}", err)
> - }),
> - Level::Default,
> - ))
> + }))
> + .zlib(true)
> + .build(),
> + )
> })
> }
> None => resp,
> @@ -711,7 +713,7 @@ async fn simple_static_file_download(
>
> let mut response = match compression {
> Some(CompressionMethod::Deflate) => {
> - let mut enc = DeflateEncoder::with_quality(data, Level::Default);
> + let mut enc = DeflateEncoder::builder(data).zlib(true).build();
> enc.compress_vec(&mut file, CHUNK_SIZE_LIMIT as usize)
> .await?;
> let mut response = Response::new(enc.into_inner().into());
> @@ -752,10 +754,11 @@ async fn chunked_static_file_download(
> header::CONTENT_ENCODING,
> CompressionMethod::Deflate.content_encoding(),
> );
> - Body::wrap_stream(DeflateEncoder::with_quality(
> - AsyncReaderStream::new(file),
> - Level::Default,
> - ))
> + Body::wrap_stream(
> + DeflateEncoder::builder(AsyncReaderStream::new(file))
> + .zlib(true)
> + .build(),
> + )
> }
> None => Body::wrap_stream(AsyncReaderStream::new(file)),
> };
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-17 15:49 ` Max Carrara
@ 2024-07-18 16:08 ` Thomas Lamprecht
2024-07-22 5:56 ` Max Carrara
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Lamprecht @ 2024-07-18 16:08 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Max Carrara
Am 17/07/2024 um 17:49 schrieb Max Carrara:
> On Tue Jul 16, 2024 at 11:22 AM CEST, Maximiliano Sandoval wrote:
>> As per [RFC9110] the Deflate encoding is a "zlib" data format. This
>> makes the rest-server compatible with the http-client.
>>
>> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
>>
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>>
>> By default the builder will use the default compression level. Please let me
>> know if the Level::Default should be set explicitly.
>
> This patch looks simple enough to me and does what it says on the tin;
> I can't really spot anything wrong here.
Does the "does what it says on the tin" part also mean you tested this
explicitly? Just wondering why no T-b was given and how closely I need
to check this out ;-)
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-18 16:08 ` Thomas Lamprecht
@ 2024-07-22 5:56 ` Max Carrara
2024-07-22 6:20 ` Thomas Lamprecht
0 siblings, 1 reply; 7+ messages in thread
From: Max Carrara @ 2024-07-22 5:56 UTC (permalink / raw)
To: Thomas Lamprecht, Proxmox Backup Server development discussion
On Thu Jul 18, 2024 at 6:08 PM CEST, Thomas Lamprecht wrote:
> Am 17/07/2024 um 17:49 schrieb Max Carrara:
> > On Tue Jul 16, 2024 at 11:22 AM CEST, Maximiliano Sandoval wrote:
> >> As per [RFC9110] the Deflate encoding is a "zlib" data format. This
> >> makes the rest-server compatible with the http-client.
> >>
> >> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
> >>
> >> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> >> ---
> >>
> >> By default the builder will use the default compression level. Please let me
> >> know if the Level::Default should be set explicitly.
> >
> > This patch looks simple enough to me and does what it says on the tin;
> > I can't really spot anything wrong here.
>
> Does the "does what it says on the tin" part also mean you tested this
> explicitly? Just wondering why no T-b was given and how closely I need
> to check this out ;-)
Maybe I should use less colorful sayings next time, but no, I hadn't
tested this, hence the missing T-b tag ;)
Though, for good measure I tested it now - all responses use the
"Content-Encoding: deflate" header and compress the correctly in
Firefox. Furthermore, when making a request without the header, the
content is not encoded at all (tested with curl).
So, my trailers are now as follows:
Reviewed-by: Max Carrara <m.carrara@proxmox.com>
Tested-by: Max Carrara <m.carrara@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-22 5:56 ` Max Carrara
@ 2024-07-22 6:20 ` Thomas Lamprecht
2024-07-25 7:35 ` Maximiliano Sandoval
0 siblings, 1 reply; 7+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 6:20 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Max Carrara
Am 22/07/2024 um 07:56 schrieb Max Carrara:
>> Does the "does what it says on the tin" part also mean you tested this
>> explicitly? Just wondering why no T-b was given and how closely I need
>> to check this out 😉
>
> Maybe I should use less colorful sayings next time, but no, I hadn't
> tested this, hence the missing T-b tag 😉
:-)
> Though, for good measure I tested it now - all responses use the
> "Content-Encoding: deflate" header and compress the correctly in
> Firefox. Furthermore, when making a request without the header, the
> content is not encoded at all (tested with curl).
>
> So, my trailers are now as follows:
>
> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
> Tested-by: Max Carrara <m.carrara@proxmox.com>
Many thanks! While I appreciate the R-b, for such changes that change
semantics one some public interface/protocol/service I'd slightly prefer
the T-b over it, at least if one would have only time to do either
review or testing; doing both test and review is naturally always nicer.
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-22 6:20 ` Thomas Lamprecht
@ 2024-07-25 7:35 ` Maximiliano Sandoval
0 siblings, 0 replies; 7+ messages in thread
From: Maximiliano Sandoval @ 2024-07-25 7:35 UTC (permalink / raw)
To: Thomas Lamprecht; +Cc: Proxmox Backup Server development discussion
Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
> Am 22/07/2024 um 07:56 schrieb Max Carrara:
>>> Does the "does what it says on the tin" part also mean you tested this
>>> explicitly? Just wondering why no T-b was given and how closely I need
>>> to check this out 😉
>>
>> Maybe I should use less colorful sayings next time, but no, I hadn't
>> tested this, hence the missing T-b tag 😉
>
> :-)
>
>> Though, for good measure I tested it now - all responses use the
>> "Content-Encoding: deflate" header and compress the correctly in
>> Firefox. Furthermore, when making a request without the header, the
>> content is not encoded at all (tested with curl).
>>
>> So, my trailers are now as follows:
>>
>> Reviewed-by: Max Carrara <m.carrara@proxmox.com>
>> Tested-by: Max Carrara <m.carrara@proxmox.com>
>
> Many thanks! While I appreciate the R-b, for such changes that change
> semantics one some public interface/protocol/service I'd slightly prefer
> the T-b over it, at least if one would have only time to do either
> review or testing; doing both test and review is naturally always nicer.
One thing that I forgot to mention is that as per the note in [RFC 9110,
8.4.1.2] it is relatively common to have non-conformant implementations.
Hence, some clients (notably browsers) might support decoding
non-conformant payloads without the zlib header set, explaining why this
was working up to this point.
[RFC 9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
--
Maximiliano
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] applied: [PATCH proxmox] rest-server: Encode with zlib headers
2024-07-16 9:22 [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers Maximiliano Sandoval
2024-07-17 15:49 ` Max Carrara
@ 2024-07-22 6:12 ` Thomas Lamprecht
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Lamprecht @ 2024-07-22 6:12 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Maximiliano Sandoval
Am 16/07/2024 um 11:22 schrieb Maximiliano Sandoval:
> As per [RFC9110] the Deflate encoding is a "zlib" data format. This
> makes the rest-server compatible with the http-client.
>
> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#field.content-encoding
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>
> By default the builder will use the default compression level. Please let me
> know if the Level::Default should be set explicitly.
>
> proxmox-rest-server/src/rest.rs | 35 ++++++++++++++++++---------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
>
applied, with Max's R-b and T-b, thanks!
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-07-25 7:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-16 9:22 [pbs-devel] [PATCH proxmox] rest-server: Encode with zlib headers Maximiliano Sandoval
2024-07-17 15:49 ` Max Carrara
2024-07-18 16:08 ` Thomas Lamprecht
2024-07-22 5:56 ` Max Carrara
2024-07-22 6:20 ` Thomas Lamprecht
2024-07-25 7:35 ` Maximiliano Sandoval
2024-07-22 6:12 ` [pbs-devel] applied: " Thomas Lamprecht
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.