fix(dcutr): Skip unparsable multiaddr (libp2p#3280)

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 libp2p#3244 for details.
mxinden · Jan 2, 2023 · 4fa9822 · 4fa9822
1 parent 773c370
commit 4fa9822
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 8 deletions.
diff --git a/protocols/dcutr/CHANGELOG.md b/protocols/dcutr/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 0.8.1
+
+- Skip unparsable multiaddr in `InboundUpgrade::upgrade_inbound`. See [PR XXX].
+
+[PR XXX]: https://github.com/libp2p/rust-libp2p/pull/XXX
+
 # 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.")]