fix(kad): Skip invalid multiaddr (#3280)

With this commit `libp2p-kad` no longer discards the whole peer payload in case an addr is invalid, but instead logs the failure, skips the invalid multiaddr and parses the remaining payload. See #3244 for details. Co-authored-by: Thomas Eizinger <[email protected]>
libp2p · Dec 26, 2022 · 773c370 · 773c370
1 parent fd48971
commit 773c370
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 5 deletions.
diff --git a/protocols/kad/CHANGELOG.md b/protocols/kad/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 0.42.1
+
+- Skip unparsable multiaddr in `Peer::addrs`. See [PR 3280].
+
+[PR 3280]: https://github.com/libp2p/rust-libp2p/pull/3280
+
 # 0.42.0
 
 - Update to `libp2p-core` `v0.38.0`.

diff --git a/protocols/kad/Cargo.toml b/protocols/kad/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-kad"
 edition = "2021"
 rust-version = "1.62.0"
 description = "Kademlia protocol for libp2p"
-version = "0.42.0"
+version = "0.42.1"
 authors = ["Parity Technologies <[email protected]>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"
@@ -44,7 +44,7 @@ prost-build = "0.11"
 [features]
 serde = ["dep:serde", "bytes/serde"]
 
-# Passing arguments to the docsrs builder in order to properly document cfg's. 
+# Passing arguments to the docsrs builder in order to properly document cfg's.
 # More information: https://docs.rs/about/builds#cross-compiling
 [package.metadata.docs.rs]
 all-features = true

diff --git a/protocols/kad/src/protocol.rs b/protocols/kad/src/protocol.rs
@@ -105,10 +105,13 @@ impl TryFrom<proto::message::Peer> for KadPeer {
 
         let mut addrs = Vec::with_capacity(peer.addrs.len());
         for addr in peer.addrs.into_iter() {
-            let as_ma = Multiaddr::try_from(addr).map_err(invalid_data)?;
-            addrs.push(as_ma);
+            match Multiaddr::try_from(addr) {
+                Ok(a) => addrs.push(a),
+                Err(e) => {
+                    log::debug!("Unable to parse multiaddr: {e}");
+                }
+            };
         }
-        debug_assert_eq!(addrs.len(), addrs.capacity());
 
         let connection_ty = proto::message::ConnectionType::from_i32(peer.connection)
             .ok_or_else(|| invalid_data("unknown connection type"))?
@@ -601,6 +604,29 @@ where
 
 #[cfg(test)]
 mod tests {
+    use super::*;
+
+    #[test]
+    fn skip_invalid_multiaddr() {
+        let valid_multiaddr: Multiaddr = "/ip6/2001:db8::/tcp/1234".parse().unwrap();
+        let valid_multiaddr_bytes = valid_multiaddr.to_vec();
+
+        let invalid_multiaddr = {
+            let a = vec![255; 8];
+            assert!(Multiaddr::try_from(a.clone()).is_err());
+            a
+        };
+
+        let payload = proto::message::Peer {
+            id: PeerId::random().to_bytes(),
+            addrs: vec![valid_multiaddr_bytes, invalid_multiaddr],
+            connection: proto::message::ConnectionType::CanConnect.into(),
+        };
+
+        let peer = KadPeer::try_from(payload).expect("not to fail");
+
+        assert_eq!(peer.multiaddrs, vec![valid_multiaddr])
+    }
 
     /*// TODO: restore
     use self::libp2p_tcp::TcpTransport;