diff --git a/examples/ipfs-private.rs b/examples/ipfs-private.rs index 133316e1164..f3c7afd496c 100644 --- a/examples/ipfs-private.rs +++ b/examples/ipfs-private.rs @@ -223,6 +223,12 @@ fn main() -> Result<(), Box> { } => { println!("ping: timeout to {}", peer.to_base58()); } + PingEvent { + peer, + result: Result::Err(PingFailure::Unsupported), + } => { + println!("ping: {} does not support ping protocol", peer.to_base58()); + } PingEvent { peer, result: Result::Err(PingFailure::Other { error }), diff --git a/protocols/ping/CHANGELOG.md b/protocols/ping/CHANGELOG.md index 68ff4c0f6c9..dad4298ddd9 100644 --- a/protocols/ping/CHANGELOG.md +++ b/protocols/ping/CHANGELOG.md @@ -1,3 +1,13 @@ +# Unreleased + +- Don't close connection if ping protocol is unsupported by remote. + Previously, a failed protocol negotation for ping caused a force close of the connection. + As a result, all nodes in a network had to support ping. + To allow networks where some nodes don't support ping, we now emit + `PingFailure::Unsupported` once for every connection on which ping is not supported. + + Fixes [#2109](https://github.com/libp2p/rust-libp2p/issues/2109). + # 0.30.0 [2021-07-12] - Update dependencies. diff --git a/protocols/ping/src/handler.rs b/protocols/ping/src/handler.rs index 9059bb85cae..d9bd6d65096 100644 --- a/protocols/ping/src/handler.rs +++ b/protocols/ping/src/handler.rs @@ -21,6 +21,8 @@ use crate::protocol; use futures::prelude::*; use futures::future::BoxFuture; + +use libp2p_core::{UpgradeError, upgrade::NegotiationError}; use libp2p_swarm::{ KeepAlive, NegotiatedSubstream, @@ -188,6 +190,22 @@ pub struct PingHandler { /// substream, this is always a future that waits for the /// next inbound ping to be answered. inbound: Option, + /// Tracks the state of our handler. + state: State +} + +/// Models our knowledge of the other peer's support of the ping protocol. +#[derive(Debug, Clone, Copy)] +enum State { + /// We are inactive because the other peer doesn't support ping. + Inactive { + /// Whether or not we've reported the missing support yet. + /// + /// This is used to avoid repeated events being emitted for a specific connection. + reported: bool + }, + /// We are actively pinging the other peer. + Active, } impl PingHandler { @@ -200,6 +218,7 @@ impl PingHandler { failures: 0, outbound: None, inbound: None, + state: State::Active, } } } @@ -230,12 +249,20 @@ impl ProtocolsHandler for PingHandler { fn inject_dial_upgrade_error(&mut self, _info: (), error: ProtocolsHandlerUpgrErr) { self.outbound = None; // Request a new substream on the next `poll`. - self.pending_errors.push_front( - match error { - // Note: This timeout only covers protocol negotiation. - ProtocolsHandlerUpgrErr::Timeout => PingFailure::Timeout, - e => PingFailure::Other { error: Box::new(e) }, - }) + + let error = match error { + ProtocolsHandlerUpgrErr::Upgrade(UpgradeError::Select(NegotiationError::Failed)) => { + self.state = State::Inactive { + reported: false + }; + return; + }, + // Note: This timeout only covers protocol negotiation. + ProtocolsHandlerUpgrErr::Timeout => PingFailure::Timeout, + e => PingFailure::Other { error: Box::new(e) }, + }; + + self.pending_errors.push_front(error); } fn connection_keep_alive(&self) -> KeepAlive { @@ -247,6 +274,17 @@ impl ProtocolsHandler for PingHandler { } fn poll(&mut self, cx: &mut Context<'_>) -> Poll> { + match self.state { + State::Inactive { reported: true } => { + return Poll::Pending // nothing to do on this connection + }, + State::Inactive { reported: false } => { + self.state = State::Inactive { reported: true }; + return Poll::Ready(ProtocolsHandlerEvent::Custom(Err(PingFailure::Unsupported))); + }, + State::Active => {} + } + // Respond to inbound pings. if let Some(fut) = self.inbound.as_mut() { match fut.poll_unpin(cx) {