sdroege / async-tungstenite

Async binding for Tungstenite, the Lightweight stream-based WebSocket implementation
MIT License
395 stars 61 forks source link

TLS Features are not additive #78

Open MikailBag opened 3 years ago

MikailBag commented 3 years ago

This code uses cargo features in a non-additive way.

Imagine a library A that enables tokio-rustls feature and uses rustls types (e.g. rustls-based Connector).

Also, imagine a library B that enables tokio-openssl features and uses openssl types (e.g. openssl-based Connector).

In isolation they will work.

But when they appear in one crate graph, asunc-tungstenite will select only rustls-based implementation, and library B will fail to compile.

sdroege commented 3 years ago

Indeed, thanks! Would you be interested to send a PR to untangle that?

There could be separate functions per TLS backend, and the functions that are not TLS-backend specific could select one of them if multiple are enabled.

swanandx commented 9 months ago

Facing similar issue with rustls & native-tls:

// this enables tokio-native-tls feature
#[cfg(feature = "use-native-tls")]
let connector = tls::native_tls_connector(&tls_config).await?;

// enables tokio-rustls-native-certs & tokio-rustls-webpki-roots
#[cfg(feature = "use-rustls")]
let connector = tls::rustls_connector(&tls_config).await?;

// if both features are enabled together, it will generate error:
// expected `tokio_native_tls::TlsConnector`, found `tokio_rustls::TlsConnector`
// ideally, there should be way to choose the connector 
// even when multiple features are enabled!
let (socket, response) = async_tungstenite::tokio::client_async_tls_with_connector(
    request,
    tcp_stream,
    Some(connector),
)
.await?;

There could be separate functions per TLS backend, and the functions that are not TLS-backend specific could select one of them if multiple are enabled.

we would need to export those backend specific fns as well ( otherwise, the issue of some feature taking precedence will still be there ). Instead of that, can we export the tls modules?

This way I can choose which fns I want to use manuall like:

async_tungstenite::tokio::tokio_native_tls::client_async_tls_with_connector(..)

// or 
async_tungstenite::tokio::tokio_rustls::client_async_tls_with_connector(..)

// for existing behaviour
async_tungstenite::tokio::client_async_tls_with_connector(..)

ps: I hope I am correct saying this two issue are same haha, Thank you :)

swanandx commented 7 months ago

any updates @sdroege ?

sdroege commented 7 months ago

Not really, didn't have time to look into this yet in detail. If someone has the time to untangle it and propose a solution that would be great!

swanandx commented 7 months ago

Not really, didn't have time to look into this yet in detail. If someone has the time to untangle it and propose a solution that would be great!

I suggest earlier that, can we export the tls modules?

This way I can choose which fns I want to use manually like:

async_tungstenite::tokio::tokio_native_tls::client_async_tls_with_connector(..)

// or
async_tungstenite::tokio::tokio_rustls::client_async_tls_with_connector(..) 

// for existing behaviour
async_tungstenite::tokio::client_async_tls_with_connector(..)

So more like having more control by importing than just relying in feature flags, wdyt? This would also help in keeping existing behaviour intact, so won't be a breaking change!

sdroege commented 7 months ago

I like the idea