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

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 12, 2023 · 3cc8247 · 3cc8247
1 parent 7665e74
commit 3cc8247
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 12 deletions.
diff --git a/protocols/dcutr/CHANGELOG.md b/protocols/dcutr/CHANGELOG.md
@@ -16,6 +16,13 @@
 [issue 2217]: https://github.com/libp2p/rust-libp2p/issues/2217
 [PR 3214]: https://github.com/libp2p/rust-libp2p/pull/3214
 
+# 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/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.")]