diff --git a/core/Cargo.toml b/core/Cargo.toml index 1b54773e803..999122bcc23 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -22,7 +22,7 @@ libsecp256k1 = { version = "0.3.1", optional = true } log = "0.4" multiaddr = { package = "parity-multiaddr", version = "0.9.2", path = "../misc/multiaddr" } multihash = "0.11.3" -multistream-select = { version = "0.8.4", path = "../misc/multistream-select" } +multistream-select = { version = "0.8.5", path = "../misc/multistream-select" } parking_lot = "0.11.0" pin-project = "1.0.0" prost = "0.6.1" diff --git a/misc/multistream-select/CHANGELOG.md b/misc/multistream-select/CHANGELOG.md index 8e054427ac4..d81960dfc6d 100644 --- a/misc/multistream-select/CHANGELOG.md +++ b/misc/multistream-select/CHANGELOG.md @@ -1,3 +1,8 @@ +# 0.8.5 [unreleased] + +- During negotiation do not interpret EOF error as an IO error, but instead as a + negotiation error. See https://github.com/libp2p/rust-libp2p/pull/1823. + # 0.8.4 [2020-10-20] - Temporarily disable the internal selection of "parallel" protocol diff --git a/misc/multistream-select/Cargo.toml b/misc/multistream-select/Cargo.toml index 67efeade0e3..d8e3fa167af 100644 --- a/misc/multistream-select/Cargo.toml +++ b/misc/multistream-select/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "multistream-select" description = "Multistream-select negotiation protocol for libp2p" -version = "0.8.4" +version = "0.8.5" authors = ["Parity Technologies "] license = "MIT" repository = "https://github.com/libp2p/rust-libp2p" diff --git a/misc/multistream-select/src/dialer_select.rs b/misc/multistream-select/src/dialer_select.rs index 18509e942b7..38111d99c25 100644 --- a/misc/multistream-select/src/dialer_select.rs +++ b/misc/multistream-select/src/dialer_select.rs @@ -24,7 +24,7 @@ use crate::{Negotiated, NegotiationError}; use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, Version}; use futures::{future::Either, prelude::*}; -use std::{convert::TryFrom as _, io, iter, mem, pin::Pin, task::{Context, Poll}}; +use std::{convert::TryFrom as _, iter, mem, pin::Pin, task::{Context, Poll}}; /// Returns a `Future` that negotiates a protocol on the given I/O stream /// for a peer acting as the _dialer_ (or _initiator_). @@ -235,9 +235,10 @@ where *this.state = SeqState::AwaitProtocol { io, protocol }; return Poll::Pending } - Poll::Ready(None) => - return Poll::Ready(Err(NegotiationError::from( - io::Error::from(io::ErrorKind::UnexpectedEof)))), + // Treat EOF error as [`NegotiationError::Failed`], not as + // [`NegotiationError::ProtocolError`], allowing dropping or closing an I/O + // stream as a permissible way to "gracefully" fail a negotiation. + Poll::Ready(None) => return Poll::Ready(Err(NegotiationError::Failed)), }; match msg { @@ -358,9 +359,10 @@ where *this.state = ParState::RecvProtocols { io }; return Poll::Pending } - Poll::Ready(None) => - return Poll::Ready(Err(NegotiationError::from( - io::Error::from(io::ErrorKind::UnexpectedEof)))), + // Treat EOF error as [`NegotiationError::Failed`], not as + // [`NegotiationError::ProtocolError`], allowing dropping or closing an I/O + // stream as a permissible way to "gracefully" fail a negotiation. + Poll::Ready(None) => return Poll::Ready(Err(NegotiationError::Failed)), }; match &msg { diff --git a/misc/multistream-select/src/listener_select.rs b/misc/multistream-select/src/listener_select.rs index 2d66d666b7b..04f23228369 100644 --- a/misc/multistream-select/src/listener_select.rs +++ b/misc/multistream-select/src/listener_select.rs @@ -26,7 +26,7 @@ use crate::protocol::{Protocol, ProtocolError, MessageIO, Message, Version}; use futures::prelude::*; use smallvec::SmallVec; -use std::{convert::TryFrom as _, io, iter::FromIterator, mem, pin::Pin, task::{Context, Poll}}; +use std::{convert::TryFrom as _, iter::FromIterator, mem, pin::Pin, task::{Context, Poll}}; /// Returns a `Future` that negotiates a protocol on the given I/O stream /// for a peer acting as the _listener_ (or _responder_). @@ -118,10 +118,10 @@ where return Poll::Ready(Err(ProtocolError::InvalidMessage.into())) }, Poll::Ready(Some(Err(err))) => return Poll::Ready(Err(From::from(err))), - Poll::Ready(None) => - return Poll::Ready(Err(NegotiationError::from( - ProtocolError::IoError( - io::ErrorKind::UnexpectedEof.into())))), + // Treat EOF error as [`NegotiationError::Failed`], not as + // [`NegotiationError::ProtocolError`], allowing dropping or closing an I/O + // stream as a permissible way to "gracefully" fail a negotiation. + Poll::Ready(None) => return Poll::Ready(Err(NegotiationError::Failed)), Poll::Pending => { *this.state = State::RecvHeader { io }; return Poll::Pending @@ -152,10 +152,16 @@ where State::RecvMessage { mut io } => { let msg = match Pin::new(&mut io).poll_next(cx) { Poll::Ready(Some(Ok(msg))) => msg, - Poll::Ready(None) => - return Poll::Ready(Err(NegotiationError::from( - ProtocolError::IoError( - io::ErrorKind::UnexpectedEof.into())))), + // Treat EOF error as [`NegotiationError::Failed`], not as + // [`NegotiationError::ProtocolError`], allowing dropping or closing an I/O + // stream as a permissible way to "gracefully" fail a negotiation. + // + // This is e.g. important when a listener rejects a protocol with + // [`Message::NotAvailable`] and the dialer does not have alternative + // protocols to propose. Then the dialer will stop the negotiation and drop + // the corresponding stream. As a listener this EOF should be interpreted as + // a failed negotiation. + Poll::Ready(None) => return Poll::Ready(Err(NegotiationError::Failed)), Poll::Pending => { *this.state = State::RecvMessage { io }; return Poll::Pending; diff --git a/misc/multistream-select/src/tests.rs b/misc/multistream-select/src/tests.rs index c5ddd43ea3a..50ab7ded016 100644 --- a/misc/multistream-select/src/tests.rs +++ b/misc/multistream-select/src/tests.rs @@ -85,9 +85,8 @@ fn no_protocol_found() { let protos = vec![b"/proto1", b"/proto2"]; let io = match listener_select_proto(connec, protos).await { Ok((_, io)) => io, - // We don't explicitly check for `Failed` because the client might close the connection when it - // realizes that we have no protocol in common. - Err(_) => return, + Err(NegotiationError::Failed) => return, + Err(NegotiationError::ProtocolError(e)) => panic!("Unexpected protocol error {}", e), }; match io.complete().await { Err(NegotiationError::Failed) => {},