Skip to content

Commit

Permalink
feat(websocket): remove deflate compression option
Browse files Browse the repository at this point in the history
This option is the only reason we pull in a dependency on the shared `zlib` library. Unfortunately, we cannot use a pure Rust backend here because that one does not support configuring the window bits of the deflate algorithm which is used in the deflate websocket extension.

Using websockets in libp2p is already crazy inefficient because you end up with double encryption when using `/wss` (which is enforced by browsers if your website is served via https). A good encryption algorithm like noise or TLS produces an output that looks completely random. Any attempt in compressing this is pretty much wasted CPU power.

Thus, I think removing this configuration option does not really hurt and allows us to remove the dependency on the `zlib` shared library.

Pull-Request: #3949.
  • Loading branch information
thomaseizinger authored May 16, 2023
1 parent 4fdd83e commit fbd471c
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 70 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ jobs:
env:
CRATE: ${{ matrix.crate }}
steps:
- name: Install zlib for `libp2p-websocket` # https://github.com/paritytech/soketto/issues/72
run: sudo apt-get install zlib1g-dev --yes

- uses: actions/checkout@v3

- uses: r7kamura/rust-problem-matchers@d58b70c4a13c4866d96436315da451d8106f8f08 #v1.3.0
Expand Down
25 changes: 0 additions & 25 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 0 additions & 7 deletions interop-tests/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
# syntax=docker/dockerfile:1.5-labs
FROM rust:1.67.0 as builder

# Install zlib as long as libp2p-websocket requires it: https://github.com/paritytech/soketto/issues/72
RUN apt-get update && \
apt-get download zlib1g && \
mkdir /dpkg && \
for deb in *.deb; do dpkg --extract $deb /dpkg || exit 10; done

# Run with access to the target cache to speed up builds
WORKDIR /workspace
ADD . .
Expand All @@ -19,6 +13,5 @@ RUN --mount=type=cache,target=./target \

FROM gcr.io/distroless/cc
COPY --from=builder /usr/local/bin/testplan /usr/local/bin/testplan
COPY --from=builder /dpkg /
ENV RUST_BACKTRACE=1
ENTRYPOINT ["testplan"]
5 changes: 5 additions & 0 deletions transports/websocket/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
- Raise MSRV to 1.65.
See [PR 3715].

- Remove `WsConfig::use_deflate` option.
This allows us to remove the dependency on the `zlib` shared library.
See [PR 3949].

[PR 3715]: https://github.com/libp2p/rust-libp2p/pull/3715
[PR 3949]: https://github.com/libp2p/rust-libp2p/pull/3949

## 0.41.0

Expand Down
2 changes: 1 addition & 1 deletion transports/websocket/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ log = "0.4.8"
parking_lot = "0.12.0"
quicksink = "0.1"
rw-stream-sink = { workspace = true }
soketto = { version = "0.7.0", features = ["deflate"] }
soketto = "0.7.0"
url = "2.1"
webpki-roots = "0.23"

Expand Down
30 changes: 2 additions & 28 deletions transports/websocket/src/framed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use log::{debug, trace};
use parking_lot::Mutex;
use soketto::{
connection::{self, CloseReason},
extension::deflate::Deflate,
handshake,
};
use std::{collections::HashMap, ops::DerefMut, sync::Arc};
Expand All @@ -51,7 +50,6 @@ pub struct WsConfig<T> {
max_data_size: usize,
tls_config: tls::Config,
max_redirects: u8,
use_deflate: bool,
/// Websocket protocol of the inner listener.
///
/// This is the suffix of the address provided in `listen_on`.
Expand All @@ -67,7 +65,6 @@ impl<T> WsConfig<T> {
max_data_size: MAX_DATA_SIZE,
tls_config: tls::Config::client(),
max_redirects: 0,
use_deflate: false,
listener_protos: HashMap::new(),
}
}
Expand Down Expand Up @@ -99,12 +96,6 @@ impl<T> WsConfig<T> {
self.tls_config = c;
self
}

/// Should the deflate extension (RFC 7692) be used if supported?
pub fn use_deflate(&mut self, flag: bool) -> &mut Self {
self.use_deflate = flag;
self
}
}

type TlsOrPlain<T> = future::Either<future::Either<client::TlsStream<T>, server::TlsStream<T>>, T>;
Expand Down Expand Up @@ -285,19 +276,12 @@ where

let transport = self.transport.clone();
let tls_config = self.tls_config.clone();
let use_deflate = self.use_deflate;
let max_redirects = self.max_redirects;

let future = async move {
loop {
match Self::dial_once(
transport.clone(),
addr,
tls_config.clone(),
use_deflate,
role_override,
)
.await
match Self::dial_once(transport.clone(), addr, tls_config.clone(), role_override)
.await
{
Ok(Either::Left(redirect)) => {
if remaining_redirects == 0 {
Expand All @@ -321,7 +305,6 @@ where
transport: Arc<Mutex<T>>,
addr: WsAddress,
tls_config: tls::Config,
use_deflate: bool,
role_override: Endpoint,
) -> Result<Either<String, Connection<T::Output>>, Error<T::Error>> {
trace!("Dialing websocket address: {:?}", addr);
Expand Down Expand Up @@ -364,10 +347,6 @@ where

let mut client = handshake::Client::new(stream, &addr.host_port, addr.path.as_ref());

if use_deflate {
client.add_extension(Box::new(Deflate::new(connection::Mode::Client)));
}

match client
.handshake()
.map_err(|e| Error::Handshake(Box::new(e)))
Expand Down Expand Up @@ -403,7 +382,6 @@ where
let remote_addr2 = remote_addr.clone(); // used for logging
let tls_config = self.tls_config.clone();
let max_size = self.max_data_size;
let use_deflate = self.use_deflate;

async move {
let stream = upgrade.map_err(Error::Transport).await?;
Expand Down Expand Up @@ -440,10 +418,6 @@ where

let mut server = handshake::Server::new(stream);

if use_deflate {
server.add_extension(Box::new(Deflate::new(connection::Mode::Server)));
}

let ws_key = {
let request = server
.receive_request()
Expand Down
6 changes: 0 additions & 6 deletions transports/websocket/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,6 @@ where
self.transport.inner_mut().set_tls_config(c);
self
}

/// Should the deflate extension (RFC 7692) be used if supported?
pub fn use_deflate(&mut self, flag: bool) -> &mut Self {
self.transport.inner_mut().use_deflate(flag);
self
}
}

impl<T> Transport for WsConfig<T>
Expand Down

0 comments on commit fbd471c

Please sign in to comment.