fix(dcutr): Skip unparsable multiaddr (#3300)

With this commit `libp2p-dcutr` no longer discards the whole remote payload in case an addr is unparsable, but instead logs the failure and skips the unparsable multiaddr. See #3244 for details.
libp2p · Jan 11, 2023 · 92a1b95 · 92a1b95
1 parent 773c370
commit 92a1b95
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 14 deletions.
diff --git a/protocols/dcutr/CHANGELOG.md b/protocols/dcutr/CHANGELOG.md
@@ -1,3 +1,10 @@
+# 0.8.1
+
+- Skip unparsable multiaddr in `InboundUpgrade::upgrade_inbound` and
+  `OutboundUpgrade::upgrade_outbound`. See [PR 3300].
+
+[PR 3300]: https://github.com/libp2p/rust-libp2p/pull/3300
+
 # 0.8.0
 
 - Update to `prost-codec` `v0.3.0`.

diff --git a/protocols/dcutr/Cargo.toml b/protocols/dcutr/Cargo.toml
@@ -3,7 +3,7 @@ name = "libp2p-dcutr"
 edition = "2021"
 rust-version = "1.62.0"
 description = "Direct connection upgrade through relay"
-version = "0.8.0"
+version = "0.8.1"
 authors = ["Max Inden <mail@max-inden.de>"]
 license = "MIT"
 repository = "https://github.com/libp2p/rust-libp2p"
@@ -34,7 +34,7 @@ libp2p = { path = "../..", features = ["full"] }
 rand = "0.8"
 clap = { version = "4.0.13", features = ["derive"] }
 
-# 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/dcutr/src/protocol/inbound.rs b/protocols/dcutr/src/protocol/inbound.rs
@@ -58,14 +58,23 @@ impl upgrade::InboundUpgrade<NegotiatedSubstream> for Upgrade {
             } else {
                 obs_addrs
                     .into_iter()
-                    .map(Multiaddr::try_from)
+                    .filter_map(|a| match Multiaddr::try_from(a) {
+                        Ok(a) => Some(a),
+                        Err(e) => {
+                            log::debug!("Unable to parse multiaddr: {e}");
+                            None
+                        }
+                    })
                     // Filter out relayed addresses.
-                    .filter(|a| match a {
-                        Ok(a) => !a.iter().any(|p| p == Protocol::P2pCircuit),
-                        Err(_) => true,
+                    .filter(|a| {
+                        if a.iter().any(|p| p == Protocol::P2pCircuit) {
+                            log::debug!("Dropping relayed address {a}");
+                            false
+                        } else {
+                            true
+                        }
                     })
-                    .collect::<Result<Vec<Multiaddr>, _>>()
-                    .map_err(|_| UpgradeError::InvalidAddrs)?
+                    .collect::<Vec<Multiaddr>>()
             };
 
             let r#type = hole_punch::Type::from_i32(r#type).ok_or(UpgradeError::ParseTypeField)?;
@@ -124,6 +133,7 @@ pub enum UpgradeError {
     StreamClosed,
     #[error("Expected at least one address in reservation.")]
     NoAddresses,
+    #[deprecated(since = "0.8.1", note = "Error is no longer constructed.")]
     #[error("Invalid addresses.")]
     InvalidAddrs,
     #[error("Failed to parse response type field.")]

diff --git a/protocols/dcutr/src/protocol/outbound.rs b/protocols/dcutr/src/protocol/outbound.rs
@@ -85,14 +85,23 @@ impl upgrade::OutboundUpgrade<NegotiatedSubstream> for Upgrade {
             } else {
                 obs_addrs
                     .into_iter()
-                    .map(Multiaddr::try_from)
+                    .filter_map(|a| match Multiaddr::try_from(a) {
+                        Ok(a) => Some(a),
+                        Err(e) => {
+                            log::debug!("Unable to parse multiaddr: {e}");
+                            None
+                        }
+                    })
                     // Filter out relayed addresses.
-                    .filter(|a| match a {
-                        Ok(a) => !a.iter().any(|p| p == Protocol::P2pCircuit),
-                        Err(_) => true,
+                    .filter(|a| {
+                        if a.iter().any(|p| p == Protocol::P2pCircuit) {
+                            log::debug!("Dropping relayed address {a}");
+                            false
+                        } else {
+                            true
+                        }
                     })
-                    .collect::<Result<Vec<Multiaddr>, _>>()
-                    .map_err(|_| UpgradeError::InvalidAddrs)?
+                    .collect::<Vec<Multiaddr>>()
             };
 
             let msg = HolePunch {
@@ -128,6 +137,7 @@ pub enum UpgradeError {
     NoAddresses,
     #[error("Invalid expiration timestamp in reservation.")]
     InvalidReservationExpiration,
+    #[deprecated(since = "0.8.1", note = "Error is no longer constructed.")]
     #[error("Invalid addresses in reservation.")]
     InvalidAddrs,
     #[error("Failed to parse response type field.")]