bugfix: Don't re-pool streams on drop.

We only want streams in the pool that are ready for the next request,
and don't have any remaining bytes of the previous request. That's the
case when we return a stream due to EOF on the underlying `Read`.

However, there was also an `impl Drop` that returned the stream to the
pool on drop of a PoolReturnRead. That resulted in a bug: If a user
called `response.into_reader()`, but did not read the entire body,
a dirty stream would be returned to the pool. The next time a request
was made to the same host, that stream would be reused, and instead of
reading off "HTTP/1.1 200 OK", ureq would read a remnant of the previous
request, resulting in a BadStatus error.

src/pool.rs

@@ -415,9 +415,3 @@ impl<R: Read + Sized + Into<Stream>> Read for PoolReturnRead<R> {
         Ok(amount)
     }
 }
-
-impl<R: Read + Sized + Into<Stream>> Drop for PoolReturnRead<R> {
-    fn drop(&mut self) {
-        self.return_connection();
-    }
-}

src/test/agent_test.rs

@@ -2,7 +2,7 @@
 
 use crate::test;
 use crate::test::testserver::{read_headers, TestServer};
-use std::io::{self, Write};
+use std::io::{self, Read, Write};
 use std::net::TcpStream;
 use std::time::Duration;
 
@@ -183,3 +183,49 @@ fn test_cookies_on_redirect() {
     assert!(agent.cookie("second").is_some());
     assert!(agent.cookie("third").is_some());
 }
+
+#[test]
+fn dirty_streams_not_returned() -> io::Result<()> {
+    let testserver = TestServer::new(|mut stream: TcpStream| -> io::Result<()> {
+        read_headers(&stream);
+        stream.write_all(b"HTTP/1.1 200 OK\r\n")?;
+        stream.write_all(b"Transfer-Encoding: chunked\r\n")?;
+        stream.write_all(b"\r\n")?;
+        stream.write_all(b"5\r\n")?;
+        stream.write_all(b"corgi\r\n")?;
+        stream.write_all(b"8\r\n")?;
+        stream.write_all(b"dachsund\r\n")?;
+        stream.write_all(b"0\r\n")?;
+        stream.write_all(b"\r\n")?;
+        Ok(())
+    });
+    let url = format!("http://localhost:{}/", testserver.port);
+    let agent = Agent::default().build();
+    let resp = agent.get(&url).call();
+    assert!(resp.ok(), format!("error: {}", resp.status()));
+    let resp_str = resp.into_string()?;
+    assert_eq!(resp_str, "corgidachsund");
+
+    // Now fetch it again, but only read part of the body.
+    let resp_to_be_dropped = agent.get(&url).call();
+    assert!(
+        resp_to_be_dropped.ok(),
+        format!("error: {}", resp_to_be_dropped.status())
+    );
+    let mut reader = resp_to_be_dropped.into_reader();
+
+    // Read 9 bytes of the response and then drop the reader.
+    let mut buf = [0_u8; 4];
+    let n = reader.read(&mut buf)?;
+    assert_ne!(n, 0, "early EOF");
+    assert_eq!(&buf, b"corg");
+    drop(reader);
+
+    let resp_to_succeed = agent.get(&url).call();
+    assert!(
+        resp_to_succeed.ok(),
+        format!("error: {}", resp_to_succeed.status())
+    );
+
+    Ok(())
+}