Allow blocking transports underneath rustls::Stream

This corrects a bug where we'd accept new plaintext,
but transient errors from a non-blocking transport would
take precedence.  This gives the caller the impression
no plaintext has been written, and duplication results.

Now we don't report errors from the the speculative
write after new plaintext is accepted.

README.md

@@ -17,6 +17,10 @@ Rustls is currently in development and hence unstable.  [Here's what I'm working
 * Next release:
   - Move TLS1.3 support from draft 23 to 28.
   - Introduce client-side support for 0-RTT data in TLS1.3.
+  - Fix a bug in rustls::Stream for non-blocking transports.
+* 0.13.1 (2018-08-17):
+  - Fix a bug in rustls::Stream for non-blocking transports
+    (backport).
 * 0.13.0 (2018-07-15):
   - Move TLS1.3 support from draft 22 to 23.
   - Add support for `SSLKEYLOGFILE`; not enabled by default.

src/stream.rs

@@ -2,8 +2,7 @@ use std::io::{Read, Write, Result};
 use session::Session;
 
 /// This type implements `io::Read` and `io::Write`, encapsulating
-/// a Session `S` and an underlying blocking transport `T`, such as
-/// a socket.
+/// a Session `S` and an underlying transport `T`, such as a socket.
 ///
 /// This allows you to use a rustls Session like a normal stream.
 pub struct Stream<'a, S: 'a + Session + ?Sized, T: 'a + Read + Write + ?Sized> {
@@ -60,7 +59,12 @@ impl<'a, S, T> Write for Stream<'a, S, T> where S: 'a + Session, T: 'a + Read +
         self.complete_prior_io()?;
 
         let len = self.sess.write(buf)?;
-        self.sess.complete_io(self.sock)?;
+
+        // Try to write the underlying transport here, but don't let
+        // any errors mask the fact we've consumed `len` bytes.
+        // Callers will learn of permanent errors on the next call.
+        let _ = self.sess.complete_io(self.sock);
+
         Ok(len)
     }
 

tests/api.rs

@@ -1082,6 +1082,64 @@ fn server_streamowned_read() {
     }
 }
 
+struct FailsWrites {
+    errkind: io::ErrorKind,
+    after: usize,
+}
+
+impl io::Read for FailsWrites {
+    fn read(&mut self, _b: &mut [u8]) -> io::Result<usize> {
+        Ok(0)
+    }
+}
+
+impl io::Write for FailsWrites {
+    fn write(&mut self, b: &[u8]) -> io::Result<usize> {
+        if self.after > 0 {
+            self.after -= 1;
+            Ok(b.len())
+        } else {
+            Err(io::Error::new(self.errkind, "oops"))
+        }
+    }
+
+    fn flush(&mut self) -> io::Result<()> {
+        Ok(())
+    }
+}
+
+#[test]
+fn stream_write_reports_underlying_io_error_before_plaintext_processed() {
+    let (mut client, mut server) = make_pair(KeyType::RSA);
+    do_handshake(&mut client, &mut server);
+
+    let mut pipe = FailsWrites {
+        errkind: io::ErrorKind::WouldBlock,
+        after: 0,
+    };
+    client.write(b"hello").unwrap();
+    let mut client_stream = Stream::new(&mut client, &mut pipe);
+    let rc = client_stream.write(b"world");
+    assert!(rc.is_err());
+    assert_eq!(format!("{:?}", rc),
+               "Err(Custom { kind: WouldBlock, error: StringError(\"oops\") })");
+}
+
+#[test]
+fn stream_write_swallows_underlying_io_error_after_plaintext_processed() {
+    let (mut client, mut server) = make_pair(KeyType::RSA);
+    do_handshake(&mut client, &mut server);
+
+    let mut pipe = FailsWrites {
+        errkind: io::ErrorKind::WouldBlock,
+        after: 1,
+    };
+    client.write(b"hello").unwrap();
+    let mut client_stream = Stream::new(&mut client, &mut pipe);
+    let rc = client_stream.write(b"world");
+    assert_eq!(format!("{:?}", rc), "Ok(5)");
+}
+
 fn make_disjoint_suite_configs() -> (ClientConfig, ServerConfig) {
     let kt = KeyType::RSA;
     let mut server_config = make_server_config(kt);