-
Notifications
You must be signed in to change notification settings - Fork 77
feat(ws): add native-tls support #517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request adds native-tls support to the compio-ws crate and refactors TLS handling to match the tokio-tungstenite API. The changes consolidate TLS support for both native-tls and rustls into a unified module, remove deprecated code, and introduce a new flush method to the WebSocketStream.
Key Changes:
- Unified TLS support through a new
tls.rsmodule that handles both native-tls and rustls - API alignment with tokio-tungstenite, including renamed functions and updated signatures
- Breaking change to
MaybeTlsStreamin compio-tls (changed from enum to struct wrapper) - New public
flushmethod for WebSocketStream - Removal of deprecated code (stream.rs, rustls.rs, get_inner method)
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| compio/Cargo.toml | Version bump to 0.16.2 and added native-tls feature propagation to compio-ws |
| compio-ws/Cargo.toml | Version bump to 0.2.0, restructured features for native-tls and rustls support |
| compio-ws/src/tls.rs | New unified TLS module supporting both native-tls and rustls with fallback logic |
| compio-ws/src/lib.rs | Removed deprecated code, trait bound simplification, added flush method |
| compio-ws/src/stream.rs | Removed deprecated module |
| compio-ws/src/rustls.rs | Removed in favor of unified tls.rs |
| compio-tls/src/maybe.rs | Breaking API change: MaybeTlsStream changed from public enum to struct wrapper with new_plain/new_tls constructors |
| compio-tls/Cargo.toml | Made native-tls dependency use workspace version |
| compio-ws/examples/ | Updated examples to use new API (TlsConnector, connect_async_tls_with_config) and switched from env_logger to tracing_subscriber |
| Cargo.toml | Added native-tls, rustls-platform-verifier, and webpki-roots to workspace dependencies, bumped compio-ws version |
| compio-quic/Cargo.toml | Updated dependencies to use workspace versions for rustls-platform-verifier and webpki-roots |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
George-Miao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
tokio-tungstenite.MaybeTlsStream, but we haven't published it so it's OK.flushmethod.I have run the tests and the examples except the autobahn ones because I don't have docker installed.