Add features for other tls types #497
Conversation
Turned out to be rather easy due to all tls being handled by our dependencies. Don't love the _fallback-tls feature, however with it we can use cargo-all-features, which helps reduce feature related bugs. Downside of cargo-all-features is it takes a long time to execute. Fixes: 1c3t3a#407, 1c3t3a#366
There's still an error with ci. engineio/src/lib.rs/test/tls_connector doesn't function the same as native-tls one (as it properly validates hostnames) Needs further investigation as to why the hostname shows as invalid in ci. If we no longer need that, then this PR can be merged.
Turned out to be rather easy due to all tls being handled by our dependencies. Don't love the _fallback-tls feature, however with it we can use cargo-all-features, which helps reduce feature related bugs. Downside of cargo-all-features is it takes a long time to execute. Fixes: 1c3t3a#407, 1c3t3a#366
There's still an error with ci. engineio/src/lib.rs/test/tls_connector doesn't function the same as native-tls one (as it properly validates hostnames) Needs further investigation as to why the hostname shows as invalid in ci. If we no longer need that, then this PR can be merged.
NXD-0: Update webpki due to vulnerabilities
…s-types' into tyler/feat-add-features-for-other-tls-types
| .build() | ||
| .unwrap(), | ||
| ) | ||
| // .tls_config( |
There was a problem hiding this comment.
I can remove this, since it isn't needed for the tests to pass.
|
|
||
| sender | ||
| .send(Message::text(Cow::Borrowed(from_utf8(&Bytes::from( | ||
| .send(Message::text(Utf8Bytes::try_from(Bytes::from( |
There was a problem hiding this comment.
This and the other changes in this file are due to upgrading tungstenite. This avoids an extra allocation that was happening inside of the text call.
| use std::io::Read; | ||
|
|
||
| pub(crate) fn tls_connector() -> error::Result<TlsConnector> { | ||
| pub fn tls_connector() -> error::Result<TlsConfig> { |
There was a problem hiding this comment.
this is exposed to the socketio crate for use in tests.
| #[cfg_attr( | ||
| feature = "_native-tls", | ||
| doc = r#" | ||
| # Example for native-tls |
There was a problem hiding this comment.
These look less appealing, but allows the docs build for all feature configurations
|
are there any update for this PR? |
Yes, this is stable. It will only have requested changes needed to make requested fixes (if that ever happens) You can also use the |
|
is this PR ready for review? |
|
It works very well on my project. It's a very important feature... |
|
This is ready for review, unless you want it broken up, which I can do, but this is the effect of making a change and having a couple of dependency bumps and a couple of bug fixes over time. More that 60% of the code would remain on this pr if I did break it up. |
|
I'd also love to see this one merged. |
|
What's the hold up? |
|
Could this be extended to build without tls at all? I don't need tls in my project and it is a nuisance to cross build ( |
9 participants
This is a continuation of #465 that was originally done by @ctrlaltf24.
My company has been using that branch with a few minor improvements. I just got the tests passing by changing how the certificates and keys are generated so that rustls will accept
localhostin a secure connection.