misc/multistream-select: Fail negotiation for all Poll::Ready(None)

libp2p · Nov 9, 2020 · adf31c8 · adf31c8
1 parent b332e03
commit adf31c8
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 17 deletions.
diff --git 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
@@ -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
@@ -153,11 +153,14 @@ where
                     let msg = match Pin::new(&mut io).poll_next(cx) {
                         Poll::Ready(Some(Ok(msg))) => msg,
                         // Treat EOF error as [`NegotiationError::Failed`], not as
-                        // [`NegotiationError::ProtocolError`]. 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.
+                        // [`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 };