Skip to content

Commit

Permalink
fix(kad): Skip invalid multiaddr (#3284)
Browse files Browse the repository at this point in the history
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 <thomas@eizinger.io>
  • Loading branch information
mxinden and thomaseizinger authored Dec 27, 2022
1 parent e4d67c5 commit 56b3b8f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
6 changes: 6 additions & 0 deletions protocols/kad/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@

[PR 3239]: https://github.com/libp2p/rust-libp2p/pull/3239

# 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`.
Expand Down
32 changes: 29 additions & 3 deletions protocols/kad/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))?
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 56b3b8f

Please sign in to comment.