-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: enforce protocols to always be valid utf8 strings #3745
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
Closed
Closed
Changes from 12 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
e9de619
Introduce new `UpgradeProtocols` trait
thomaseizinger ad4767e
WIP
thomaseizinger 3c3398f
Make `multistream-select` tests pass
thomaseizinger 1a6ff9a
Remove commented code
thomaseizinger 50edaee
Add TODO
thomaseizinger c139b6c
Use iterators instead of vecs to reduce allocations
thomaseizinger abda420
Merge branch 'master' into feat/string-protocols-2
thomaseizinger c148ed6
Make `from_static` const
thomaseizinger 83fa4c3
Introduce dedicated `Protocol` struct in `libp2p-core`
thomaseizinger 244fc7f
Start deprecating things
thomaseizinger f909139
Make it compile
thomaseizinger 0c63687
Rename `UpgradeProtocols` type
thomaseizinger 4671126
Merge branch 'master' into feat/string-protocols-2
thomaseizinger bf24183
Delete deprecated upgrades
thomaseizinger 885854a
Don't preemptively collect iterator
thomaseizinger 24b77ba
Fix typo
thomaseizinger 0762c46
Convert protocols in identify
thomaseizinger b71e517
Revert hard-coded protocol in webrtc
thomaseizinger 2274e02
Reduce diff
thomaseizinger 5d40167
Fix bad TLS protocol
thomaseizinger 19c0ddf
Merge branch 'master' into feat/string-protocols-2
thomaseizinger dc4ce72
Delete dead code
thomaseizinger ea872f0
Fix docs
thomaseizinger File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,8 @@ mod select; | |
| mod transfer; | ||
|
|
||
| use futures::future::Future; | ||
| use std::fmt; | ||
| use std::iter::FilterMap; | ||
|
|
||
| pub use self::{ | ||
| apply::{apply, apply_inbound, apply_outbound, InboundUpgradeApply, OutboundUpgradeApply}, | ||
|
|
@@ -119,6 +121,7 @@ pub use multistream_select::{NegotiatedComplete, NegotiationError, ProtocolError | |
| /// } | ||
| /// ``` | ||
| /// | ||
| #[deprecated(note = "Use the `Protocol` new-type instead.")] | ||
| pub trait ProtocolName { | ||
| /// The protocol name as bytes. Transmitted on the network. | ||
| /// | ||
|
|
@@ -127,6 +130,7 @@ pub trait ProtocolName { | |
| fn protocol_name(&self) -> &[u8]; | ||
| } | ||
|
|
||
| #[allow(deprecated)] | ||
| impl<T: AsRef<[u8]>> ProtocolName for T { | ||
| fn protocol_name(&self) -> &[u8] { | ||
| self.as_ref() | ||
|
|
@@ -135,6 +139,8 @@ impl<T: AsRef<[u8]>> ProtocolName for T { | |
|
|
||
| /// Common trait for upgrades that can be applied on inbound substreams, outbound substreams, | ||
| /// or both. | ||
| #[deprecated(note = "Implement `UpgradeProtocols` instead.")] | ||
| #[allow(deprecated)] | ||
| pub trait UpgradeInfo { | ||
| /// Opaque type representing a negotiable protocol. | ||
| type Info: ProtocolName + Clone; | ||
|
|
@@ -145,8 +151,117 @@ pub trait UpgradeInfo { | |
| fn protocol_info(&self) -> Self::InfoIter; | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Eq, PartialEq, Hash)] | ||
| pub struct Protocol { | ||
| inner: multistream_select::Protocol, | ||
| } | ||
|
|
||
| impl Protocol { | ||
| /// Construct a new protocol from a static string slice. | ||
| /// | ||
| /// # Panics | ||
| /// | ||
| /// This function panics if the protocol does not start with a forward slash: `/`. | ||
| pub const fn from_static(s: &'static str) -> Self { | ||
| Protocol { | ||
| inner: multistream_select::Protocol::from_static(s), | ||
| } | ||
| } | ||
|
|
||
| /// Attempt to construct a protocol from an owned string. | ||
| /// | ||
| /// This function will fail if the protocol does not start with a forward slash: `/`. | ||
| /// Where possible, you should use [`Protocol::from_static`] instead to avoid allocations. | ||
| pub fn try_from_owned(protocol: String) -> Result<Self, InvalidProtocol> { | ||
| Ok(Protocol { | ||
| inner: multistream_select::Protocol::try_from_owned(protocol) | ||
| .map_err(InvalidProtocol)?, | ||
| }) | ||
| } | ||
|
|
||
| /// View the protocol as a string slice. | ||
| pub fn as_str(&self) -> &str { | ||
| self.inner.as_str() | ||
| } | ||
|
|
||
| pub(crate) fn from_inner(inner: multistream_select::Protocol) -> Self { | ||
| Protocol { inner } | ||
| } | ||
|
|
||
| pub(crate) fn into_inner(self) -> multistream_select::Protocol { | ||
| self.inner | ||
| } | ||
| } | ||
|
|
||
| impl fmt::Display for Protocol { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| self.inner.fmt(f) | ||
| } | ||
| } | ||
|
|
||
| impl PartialEq<str> for Protocol { | ||
| fn eq(&self, other: &str) -> bool { | ||
| self.inner.as_str() == other | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct InvalidProtocol(multistream_select::ProtocolError); | ||
|
|
||
| impl fmt::Display for InvalidProtocol { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| write!(f, "invalid protocol") | ||
| } | ||
| } | ||
|
|
||
| impl std::error::Error for InvalidProtocol { | ||
| fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { | ||
| Some(&self.0) | ||
| } | ||
| } | ||
|
|
||
| /// Each protocol upgrade must implement the [`ToProtocolsIter`] trait. | ||
| /// | ||
| /// The returned iterator should yield the protocols that the upgrade supports. | ||
| pub trait ToProtocolsIter { | ||
| type Iter: Iterator<Item = Protocol>; | ||
thomaseizinger marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
thomaseizinger marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| fn to_protocols_iter(&self) -> Self::Iter; | ||
| } | ||
|
|
||
| #[allow(deprecated)] | ||
| impl<U> ToProtocolsIter for U | ||
| where | ||
| U: UpgradeInfo, | ||
| { | ||
| type Iter = FilterMap<<U::InfoIter as IntoIterator>::IntoIter, fn(U::Info) -> Option<Protocol>>; | ||
|
|
||
| fn to_protocols_iter(&self) -> Self::Iter { | ||
| self.protocol_info() | ||
| .into_iter() | ||
| .filter_map(filter_non_utf8_protocols) | ||
| } | ||
| } | ||
|
|
||
| #[allow(deprecated)] | ||
| fn filter_non_utf8_protocols(p: impl ProtocolName) -> Option<Protocol> { | ||
| let name = p.protocol_name(); | ||
|
|
||
| match multistream_select::Protocol::try_from(name) { | ||
| Ok(p) => Some(Protocol::from_inner(p)), | ||
| Err(e) => { | ||
| log::warn!( | ||
| "ignoring protocol {} during upgrade because it is malformed: {e}", | ||
| String::from_utf8_lossy(name) | ||
| ); | ||
|
|
||
| None | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Possible upgrade on an inbound connection or substream. | ||
| pub trait InboundUpgrade<C>: UpgradeInfo { | ||
| pub trait InboundUpgrade<C>: ToProtocolsIter { | ||
| /// Output after the upgrade has been successfully negotiated and the handshake performed. | ||
| type Output; | ||
| /// Possible error during the handshake. | ||
|
|
@@ -158,7 +273,7 @@ pub trait InboundUpgrade<C>: UpgradeInfo { | |
| /// method is called to start the handshake. | ||
| /// | ||
| /// The `info` is the identifier of the protocol, as produced by `protocol_info`. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs updating. |
||
| fn upgrade_inbound(self, socket: C, info: Self::Info) -> Self::Future; | ||
| fn upgrade_inbound(self, socket: C, selected_protocol: Protocol) -> Self::Future; | ||
| } | ||
|
|
||
| /// Extension trait for `InboundUpgrade`. Automatically implemented on all types that implement | ||
|
|
@@ -186,7 +301,7 @@ pub trait InboundUpgradeExt<C>: InboundUpgrade<C> { | |
| impl<C, U: InboundUpgrade<C>> InboundUpgradeExt<C> for U {} | ||
|
|
||
| /// Possible upgrade on an outbound connection or substream. | ||
| pub trait OutboundUpgrade<C>: UpgradeInfo { | ||
| pub trait OutboundUpgrade<C>: ToProtocolsIter { | ||
| /// Output after the upgrade has been successfully negotiated and the handshake performed. | ||
| type Output; | ||
| /// Possible error during the handshake. | ||
|
|
@@ -198,7 +313,7 @@ pub trait OutboundUpgrade<C>: UpgradeInfo { | |
| /// method is called to start the handshake. | ||
| /// | ||
| /// The `info` is the identifier of the protocol, as produced by `protocol_info`. | ||
| fn upgrade_outbound(self, socket: C, info: Self::Info) -> Self::Future; | ||
| fn upgrade_outbound(self, socket: C, selected_protocol: Protocol) -> Self::Future; | ||
| } | ||
|
|
||
| /// Extention trait for `OutboundUpgrade`. Automatically implemented on all types that implement | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.