feat: replace ProtocolName with AsRef<str>

Previously, a protocol could be any sequence of bytes as long as it started with `/`. Now, we directly parse a protocol as `String` which enforces it to be valid UTF8.

To notify users of this change, we delete the `ProtocolName` trait. The new requirement is that users need to provide a type that implements `AsRef<str>`.

We also add a `StreamProtocol` newtype in `libp2p-swarm` which provides an easy way for users to ensure their protocol strings are compliant. The newtype enforces that protocol strings start with `/`. `StreamProtocol` also implements `AsRef<str>`, meaning users can directly use it in their upgrades.

`multistream-select` by itself only changes marginally with this patch. The only thing we enforce in the type-system is that protocols must implement `AsRef<str>`.

Resolves: #2831.

Pull-Request: #3746.

swarm/src/handler/multi.rs

@@ -31,7 +31,7 @@ use crate::handler::{
 use crate::upgrade::{InboundUpgradeSend, OutboundUpgradeSend, UpgradeInfoSend};
 use crate::NegotiatedSubstream;
 use futures::{future::BoxFuture, prelude::*};
-use libp2p_core::upgrade::{NegotiationError, ProtocolError, ProtocolName, UpgradeError};
+use libp2p_core::upgrade::{NegotiationError, ProtocolError, UpgradeError};
 use libp2p_core::ConnectedPoint;
 use libp2p_identity::PeerId;
 use rand::Rng;
@@ -463,9 +463,9 @@ where
 #[derive(Debug, Clone)]
 pub struct IndexedProtoName<H>(usize, H);
 
-impl<H: ProtocolName> ProtocolName for IndexedProtoName<H> {
-    fn protocol_name(&self) -> &[u8] {
-        self.1.protocol_name()
+impl<H: AsRef<str>> AsRef<str> for IndexedProtoName<H> {
+    fn as_ref(&self) -> &str {
+        self.1.as_ref()
     }
 }
 
@@ -586,7 +586,7 @@ where
     let mut set = HashSet::new();
     for infos in iter {
         for i in infos.protocol_info() {
-            let v = Vec::from(i.protocol_name());
+            let v = Vec::from(i.as_ref());
             if set.contains(&v) {
                 return Err(DuplicateProtonameError(v));
             } else {

swarm/src/lib.rs

@@ -56,7 +56,9 @@
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 
 mod connection;
+mod executor;
 mod registry;
+mod stream_protocol;
 #[cfg(test)]
 mod test;
 mod upgrade;
@@ -64,7 +66,6 @@ mod upgrade;
 pub mod behaviour;
 pub mod dial_opts;
 pub mod dummy;
-mod executor;
 pub mod handler;
 pub mod keep_alive;
 
@@ -130,6 +131,7 @@ pub use handler::{
 #[cfg(feature = "macros")]
 pub use libp2p_swarm_derive::NetworkBehaviour;
 pub use registry::{AddAddressResult, AddressRecord, AddressScore};
+pub use stream_protocol::{InvalidProtocol, StreamProtocol};
 
 use crate::handler::UpgradeInfoSend;
 use connection::pool::{EstablishedConnection, Pool, PoolConfig, PoolEvent};
@@ -142,11 +144,11 @@ use futures::{executor::ThreadPoolBuilder, prelude::*, stream::FusedStream};
 use libp2p_core::muxing::SubstreamBox;
 use libp2p_core::{
     connection::ConnectedPoint,
-    multiaddr::Protocol,
+    multiaddr,
     multihash::Multihash,
     muxing::StreamMuxerBox,
     transport::{self, ListenerId, TransportError, TransportEvent},
-    Endpoint, Multiaddr, Negotiated, ProtocolName, Transport,
+    Endpoint, Multiaddr, Negotiated, Transport,
 };
 use libp2p_identity::PeerId;
 use registry::{AddressIntoIter, Addresses};
@@ -886,7 +888,7 @@ where
                     .listen_protocol()
                     .upgrade()
                     .protocol_info()
-                    .map(|p| p.protocol_name().to_owned())
+                    .map(|p| p.as_ref().as_bytes().to_vec())
                     .collect();
                 let other_established_connection_ids = self
                     .pool
@@ -2015,13 +2017,13 @@ fn p2p_addr(peer: Option<PeerId>, addr: Multiaddr) -> Result<Multiaddr, Multiadd
         None => return Ok(addr),
     };
 
-    if let Some(Protocol::P2p(hash)) = addr.iter().last() {
+    if let Some(multiaddr::Protocol::P2p(hash)) = addr.iter().last() {
         if &hash != peer.as_ref() {
             return Err(addr);
         }
         Ok(addr)
     } else {
-        Ok(addr.with(Protocol::P2p(peer.into())))
+        Ok(addr.with(multiaddr::Protocol::P2p(peer.into())))
     }
 }
 
@@ -2728,7 +2730,7 @@ mod tests {
             }));
 
         let other_id = PeerId::random();
-        let other_addr = address.with(Protocol::P2p(other_id.into()));
+        let other_addr = address.with(multiaddr::Protocol::P2p(other_id.into()));
 
         swarm2.dial(other_addr.clone()).unwrap();
 
@@ -2876,7 +2878,7 @@ mod tests {
                 let failed_addresses = errors.into_iter().map(|(addr, _)| addr).collect::<Vec<_>>();
                 let expected_addresses = addresses
                     .into_iter()
-                    .map(|addr| addr.with(Protocol::P2p(target.into())))
+                    .map(|addr| addr.with(multiaddr::Protocol::P2p(target.into())))
                     .collect::<Vec<_>>();
 
                 assert_eq!(expected_addresses, failed_addresses);

swarm/src/stream_protocol.rs

@@ -0,0 +1,104 @@
+use either::Either;
+use std::fmt;
+use std::hash::{Hash, Hasher};
+use std::sync::Arc;
+
+/// Identifies a protocol for a stream.
+///
+/// libp2p nodes use stream protocols to negotiate what to do with a newly opened stream.
+/// Stream protocols are string-based and must start with a forward slash: `/`.
+#[derive(Debug, Clone, Eq)]
+pub struct StreamProtocol {
+    inner: Either<&'static str, Arc<str>>,
+}
+
+impl StreamProtocol {
+    /// 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 new(s: &'static str) -> Self {
+        match s.as_bytes() {
+            [b'/', ..] => {}
+            _ => panic!("Protocols should start with a /"),
+        }
+
+        StreamProtocol {
+            inner: Either::Left(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 [`StreamProtocol::new`] instead to avoid allocations.
+    pub fn try_from_owned(protocol: String) -> Result<Self, InvalidProtocol> {
+        if !protocol.starts_with('/') {
+            return Err(InvalidProtocol::missing_forward_slash());
+        }
+
+        Ok(StreamProtocol {
+            inner: Either::Right(Arc::from(protocol)), // FIXME: Can we somehow reuse the allocation from the owned string?
+        })
+    }
+}
+
+impl AsRef<str> for StreamProtocol {
+    fn as_ref(&self) -> &str {
+        either::for_both!(&self.inner, s => s)
+    }
+}
+
+impl fmt::Display for StreamProtocol {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        self.inner.fmt(f)
+    }
+}
+
+impl PartialEq<&str> for StreamProtocol {
+    fn eq(&self, other: &&str) -> bool {
+        self.as_ref() == *other
+    }
+}
+
+impl PartialEq<StreamProtocol> for &str {
+    fn eq(&self, other: &StreamProtocol) -> bool {
+        *self == other.as_ref()
+    }
+}
+
+impl PartialEq for StreamProtocol {
+    fn eq(&self, other: &Self) -> bool {
+        self.as_ref() == other.as_ref()
+    }
+}
+
+impl Hash for StreamProtocol {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.as_ref().hash(state)
+    }
+}
+
+#[derive(Debug)]
+pub struct InvalidProtocol {
+    // private field to prevent construction outside of this module
+    _private: (),
+}
+
+impl InvalidProtocol {
+    pub(crate) fn missing_forward_slash() -> Self {
+        InvalidProtocol { _private: () }
+    }
+}
+
+impl fmt::Display for InvalidProtocol {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(
+            f,
+            "invalid protocol: string does not start with a forward slash"
+        )
+    }
+}
+
+impl std::error::Error for InvalidProtocol {}

swarm/src/upgrade.rs

@@ -30,7 +30,7 @@ use libp2p_core::upgrade;
 /// [`UpgradeInfo`](upgrade::UpgradeInfo).
 pub trait UpgradeInfoSend: Send + 'static {
     /// Equivalent to [`UpgradeInfo::Info`](upgrade::UpgradeInfo::Info).
-    type Info: upgrade::ProtocolName + Clone + Send + 'static;
+    type Info: AsRef<str> + Clone + Send + 'static;
     /// Equivalent to [`UpgradeInfo::InfoIter`](upgrade::UpgradeInfo::InfoIter).
     type InfoIter: Iterator<Item = Self::Info> + Send + 'static;
 
@@ -72,7 +72,7 @@ pub trait OutboundUpgradeSend: UpgradeInfoSend {
 impl<T, TInfo> OutboundUpgradeSend for T
 where
     T: upgrade::OutboundUpgrade<NegotiatedSubstream, Info = TInfo> + UpgradeInfoSend<Info = TInfo>,
-    TInfo: upgrade::ProtocolName + Clone + Send + 'static,
+    TInfo: AsRef<str> + Clone + Send + 'static,
     T::Output: Send + 'static,
     T::Error: Send + 'static,
     T::Future: Send + 'static,
@@ -106,7 +106,7 @@ pub trait InboundUpgradeSend: UpgradeInfoSend {
 impl<T, TInfo> InboundUpgradeSend for T
 where
     T: upgrade::InboundUpgrade<NegotiatedSubstream, Info = TInfo> + UpgradeInfoSend<Info = TInfo>,
-    TInfo: upgrade::ProtocolName + Clone + Send + 'static,
+    TInfo: AsRef<str> + Clone + Send + 'static,
     T::Output: Send + 'static,
     T::Error: Send + 'static,
     T::Future: Send + 'static,