Repository: algesten/ureq
Pull request #101 (2020-06-30 18:31:41)

Update cookie requirement from 0.13 to 0.14
Updates the requirements on [cookie](https://github.com/SergioBenitez/cookie-rs) to permit the latest version.
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a href="https://github.com/SergioBenitez/cookie-rs/blob/master/CHANGELOG.md">cookie's changelog</a>.</em></p>
<blockquote>
<h2>Version 0.14.1 (Jun 5, 2020)</h2>
<h3>Changes and Fixes</h3>
<ul>
<li>Updated <code>base64</code> dependency to 0.12.</li>
<li>Updated minimum <code>time</code> dependency to correct version: 0.2.11.</li>
<li>Added <code>readme</code> key to <code>Cargo.toml</code>, updated <code>license</code> field.</li>
</ul>
<h2>Version 0.14.0 (May 29, 2020)</h2>
<h3>Breaking Changes</h3>
<ul>
<li>The <code>Key::from_master()</code> method was deprecated in favor of the more aptly
named <code>Key::derive_from()</code>.</li>
<li>The deprecated <code>CookieJar::clear()</code> method was removed.</li>
</ul>
<h3>New Features</h3>
<ul>
<li>Added <code>Key::from()</code> to create a <code>Key</code> structure from a full-length key.</li>
<li>Signed and private cookie jars can be individually enabled via the new
<code>signed</code> and <code>private</code> features, respectively.</li>
<li>Key derivation via key expansion can be individually enabled via the new
<code>key-expansion</code> feature.</li>
</ul>
<h3>General Changes and Fixes</h3>
<ul>
<li><code>ring</code> is no longer a dependency: <code>RustCrypto</code>-based cryptography is used in
lieu of <code>ring</code>. Prior to their inclusion here, the <code>hmac</code> and <code>hkdf</code> crates
were audited.</li>
<li>Quotes, if present, are stripped from cookie values when parsing.</li>
</ul>
<h1>Version 0.13</h1>
<h2>Version 0.13.3 (Feb 3, 2020)</h2>
<h3>Changes</h3>
<ul>
<li>The <code>time</code> dependency was unpinned from <code>0.2.4</code>, allowing any <code>0.2.x</code>
version of <code>time</code> where <code>x &gt;= 6</code>.</li>
</ul>
<h2>Version 0.13.2 (Jan 28, 2020)</h2>
<h3>Changes</h3>
<ul>
<li>The <code>time</code> dependency was pinned to <code>0.2.4</code> due to upstream breaking changes
in <code>0.2.5</code>.</li>
</ul>
<h2>Version 0.13.1 (Jan 23, 2020)</h2>
<h3>New Features</h3>
<!-- raw HTML omitted -->
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/b5a054c158c5d9b118d3bbfe1791209f86fd1c93"><code>b5a054c</code></a> New version: 0.14.1.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/5933366206a163e62a3d08840edffedef7ad73af"><code>5933366</code></a> Add 'readme' and update 'license' in 'Cargo.toml'.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/8d35e58fb511a459c721337a0bf29172233bad84"><code>8d35e58</code></a> Specify a correct 'time' minimum version.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/9b165d6fda6f707c9799c5d1794f782691ce6e4c"><code>9b165d6</code></a> Update 'base64' to '0.12'.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/0adf48e3b1221cc66c0951b47724fe3009cc159b"><code>0adf48e</code></a> New version: 0.14.0.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/451928eada489322e4c49f9c38dd2f8604e5ec50"><code>451928e</code></a> Remove use of deprecated 'time' items.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/bba0ccae71a9499a020f11757f8395493abec981"><code>bba0cca</code></a> Remove superfluous import of 'std::mem::replace'.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/69ad64ffbb09d4acf6f75e0c1639e7ab3d3a4968"><code>69ad64f</code></a> Reintroduce 'Key::from_master()', albeit deprecated.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/4bf34e5cb6d96dfe21ef65b152505c9600d70f13"><code>4bf34e5</code></a> Remove deprecated 'CookieJar::clear()' method.</li>
<li><a href="https://github.com/SergioBenitez/cookie-rs/commit/623ee27dd6f0d572adbda6c7aa9a13f0ac90f830"><code>623ee27</code></a> Re-enable commented out test.</li>
<li>Additional commits viewable in <a href="https://github.com/SergioBenitez/cookie-rs/compare/0.13.0...0.14.1">compare view</a></li>
</ul>
</details>
<br />


Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
- `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)



</details>

jsha: Test failure is in the "doc" phase:
```
error[E0554]: `#![feature]` may not be used on the stable release channel
##[error]  --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/cookie-0.14.1/src/lib.rs:77:37
   |
77 | #![cfg_attr(all(doc, not(doctest)), feature(doc_cfg))]
   |                                     ^^^^^^^^^^^^^^^^

error: Compilation failed, aborting rustdoc
##[error]Compilation failed, aborting rustdoc
error: aborting due to 2 previous errors
##[error]aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0554`.
error: Could not document `cookie`.
##[error]Could not document `cookie`.
```

I think we need to add --no-deps to our doc build.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index 408b9ce..abcb7b5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -26,14 +26,14 @@ socks-proxy = ["socks"]
 [dependencies]
 base64 = "0.12"
 chunked_transfer = "1.2.0"
-cookie = { version = "0.13", features = ["percent-encode"], optional = true}
+cookie = { version = "0.14", features = ["percent-encode"], optional = true}
 lazy_static = "1"
 qstring = "0.7"
 url = "2"
 socks = { version = "0.3.2", optional = true }
 rustls = { version = "0.17", optional = true, features = [] }
 webpki = { version = "0.21", optional = true }
-webpki-roots = { version = "0.19", optional = true }
+webpki-roots = { version = "0.20", optional = true }
 rustls-native-certs = { version = "0.3", optional = true }
 serde = { version = "1", optional = true }
 serde_json = { version = "1", optional = true }
diff --git a/src/header.rs b/src/header.rs
index 056a4b8..719ccb9 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -2,9 +2,15 @@ use crate::error::Error;
 use std::str::FromStr;
 
 #[derive(Clone)]
-/// Wrapper type for a header line.
+/// Wrapper type for a header field.
+/// https://tools.ietf.org/html/rfc7230#section-3.2
 pub struct Header {
+    // Line contains the unmodified bytes of single header field.
+    // It does not contain the final CRLF.
     line: String,
+    // Index is the position of the colon within the header field.
+    // Invariant: index > 0
+    // Invariant: index + 1 < line.len()
     index: usize,
 }
 
@@ -82,6 +88,29 @@ pub fn add_header(headers: &mut Vec<Header>, header: Header) {
     headers.push(header);
 }
 
+// https://tools.ietf.org/html/rfc7230#section-3.2.3
+// Each header field consists of a case-insensitive field name followed
+// by a colon (":"), optional leading whitespace, the field value, and
+// optional trailing whitespace.
+// field-name     = token
+// token = 1*tchar
+// tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" / "+" / "-" / "." /
+// "^" / "_" / "`" / "|" / "~" / DIGIT / ALPHA
+fn valid_name(name: &str) -> bool {
+    name.len() > 0 && name.bytes().all(is_tchar)
+}
+
+#[inline]
+fn is_tchar(b: u8) -> bool {
+    match b {
+        b'!' | b'#' | b'$' | b'%' | b'&' => true,
+        b'\'' | b'*' | b'+' | b'-' | b'.' => true,
+        b'^' | b'_' | b'`' | b'|' | b'~' => true,
+        b if b.is_ascii_alphanumeric() => true,
+        _ => false,
+    }
+}
+
 impl FromStr for Header {
     type Err = Error;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
@@ -94,16 +123,43 @@ impl FromStr for Header {
             return Err(Error::BadHeader);
         }
 
-        // https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html#sec4.2
-        // The field value MAY be preceded by any amount of LWS, though a single SP is preferred.
-        let value_from = &s[index..];
-        let voff = match value_from.find(|c: char| !c.is_whitespace()) {
-            Some(n) => n,
-            None => 0,
-        };
-
-        let index = index + voff;
+        if !valid_name(&line[0..index]) {
+            return Err(Error::BadHeader);
+        }
 
         Ok(Header { line, index })
     }
 }
+
+#[test]
+fn test_valid_name() {
+    assert!(valid_name("example"));
+    assert!(valid_name("Content-Type"));
+    assert!(valid_name("h-123456789"));
+    assert!(!valid_name("Content-Type:"));
+    assert!(!valid_name("Content-Type "));
+    assert!(!valid_name(" some-header"));
+    assert!(!valid_name("\"invalid\""));
+    assert!(!valid_name("Gödel"));
+}
+
+#[test]
+fn test_parse_invalid_name() {
+    let h = "Content-Type   :".parse::<Header>();
+    match h {
+        Err(Error::BadHeader) => {}
+        h => assert!(false, "expected BadHeader error, got {:?}", h),
+    }
+}
+
+#[test]
+fn empty_value() {
+    let h = "foo:".parse::<Header>().unwrap();
+    assert_eq!(h.value(), "");
+}
+
+#[test]
+fn value_with_whitespace() {
+    let h = "foo:      bar    ".parse::<Header>().unwrap();
+    assert_eq!(h.value(), "bar");
+}
diff --git a/src/request.rs b/src/request.rs
index 59e60b3..715fb36 100644
--- a/src/request.rs
+++ b/src/request.rs
@@ -39,7 +39,7 @@ pub struct Request {
 
     // via agent
     pub(crate) method: String,
-    path: String,
+    url: String,
 
     // from request itself
     pub(crate) headers: Vec<Header>,
@@ -72,11 +72,11 @@ impl ::std::fmt::Debug for Request {
 }
 
 impl Request {
-    pub(crate) fn new(agent: &Agent, method: String, path: String) -> Request {
+    pub(crate) fn new(agent: &Agent, method: String, url: String) -> Request {
         Request {
             agent: Arc::clone(&agent.state),
             method,
-            path,
+            url,
             headers: agent.headers.clone(),
             redirects: 5,
             ..Default::default()
@@ -260,7 +260,7 @@ impl Request {
     ///     .build();
     /// assert_eq!("foobar", req.header("x-api-Key").unwrap());
     /// ```
-    pub fn header<'a>(&self, name: &'a str) -> Option<&str> {
+    pub fn header(&self, name: &str) -> Option<&str> {
         header::get_header(&self.headers, name)
     }
 
@@ -288,7 +288,7 @@ impl Request {
     ///     .build();
     /// assert_eq!(true, req.has("x-api-Key"));
     /// ```
-    pub fn has<'a>(&self, name: &'a str) -> bool {
+    pub fn has(&self, name: &str) -> bool {
         header::has_header(&self.headers, name)
     }
 
@@ -304,7 +304,7 @@ impl Request {
     ///     "2.3.4.5",
     /// ]);
     /// ```
-    pub fn all<'a>(&self, name: &'a str) -> Vec<&str> {
+    pub fn all(&self, name: &str) -> Vec<&str> {
         header::get_all_headers(&self.headers, name)
     }
 
@@ -512,7 +512,7 @@ impl Request {
     /// assert_eq!(req.get_url(), "https://cool.server/innit");
     /// ```
     pub fn get_url(&self) -> &str {
-        &self.path
+        &self.url
     }
 
     /// Normalizes and returns the host that will be used for this request.
@@ -558,7 +558,7 @@ impl Request {
             .map(|u| unit::combine_query(&u, &self.query, true))
     }
 
-    /// The normalized path of this request.
+    /// The normalized url of this request.
     ///
     /// Example:
     /// ```
@@ -572,7 +572,7 @@ impl Request {
 
     fn to_url(&self) -> Result<Url, Error> {
         URL_BASE
-            .join(&self.path)
+            .join(&self.url)
             .map_err(|e| Error::BadUrl(format!("{}", e)))
     }
 
diff --git a/src/response.rs b/src/response.rs
index 3e52ead..571bba2 100644
--- a/src/response.rs
+++ b/src/response.rs
@@ -122,7 +122,7 @@ impl Response {
     }
 
     /// The header corresponding header value for the give name, if any.
-    pub fn header<'a>(&self, name: &'a str) -> Option<&str> {
+    pub fn header(&self, name: &str) -> Option<&str> {
         self.headers
             .iter()
             .find(|h| h.is_name(name))
@@ -139,12 +139,12 @@ impl Response {
     }
 
     /// Tells if the response has the named header.
-    pub fn has<'a>(&self, name: &'a str) -> bool {
+    pub fn has(&self, name: &str) -> bool {
         self.header(name).is_some()
     }
 
     /// All headers corresponding values for the give name, or empty vector.
-    pub fn all<'a>(&self, name: &'a str) -> Vec<&str> {
+    pub fn all(&self, name: &str) -> Vec<&str> {
         self.headers
             .iter()
             .filter(|h| h.is_name(name))
diff --git a/src/unit.rs b/src/unit.rs
index b6fd18b..e28c404 100644
--- a/src/unit.rs
+++ b/src/unit.rs
@@ -116,15 +116,15 @@ impl Unit {
     }
 
     #[cfg(test)]
-    pub fn header<'a>(&self, name: &'a str) -> Option<&str> {
+    pub fn header(&self, name: &str) -> Option<&str> {
         header::get_header(&self.headers, name)
     }
     #[cfg(test)]
-    pub fn has<'a>(&self, name: &'a str) -> bool {
+    pub fn has(&self, name: &str) -> bool {
         header::has_header(&self.headers, name)
     }
     #[cfg(test)]
-    pub fn all<'a>(&self, name: &'a str) -> Vec<&str> {
+    pub fn all(&self, name: &str) -> Vec<&str> {
         header::get_all_headers(&self.headers, name)
     }
 }
@@ -250,7 +250,7 @@ fn extract_cookies(_state: &std::sync::Mutex<Option<AgentState>>, _url: &Url) ->
 
 // TODO check so cookies can't be set for tld:s
 #[cfg(feature = "cookie")]
-fn match_cookies<'a>(jar: &'a CookieJar, domain: &str, path: &str, is_secure: bool) -> Vec<Header> {
+fn match_cookies(jar: &CookieJar, domain: &str, path: &str, is_secure: bool) -> Vec<Header> {
     jar.iter()
         .filter(|c| {
             // if there is a domain, it must be matched.


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index fce37c2..94379ff 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -31,4 +31,4 @@ jobs:
         with:
           command: doc
           # Keep in sync with Cargo.toml's [package.metadata.docs.rs]
-          args: --no-default-features --features "tls json charset cookies socks-proxy"
+          args: --no-default-features --no-deps --features "tls json charset cookies socks-proxy"
diff --git a/src/test/agent_test.rs b/src/test/agent_test.rs
index 55934b9..66356cd 100644
--- a/src/test/agent_test.rs
+++ b/src/test/agent_test.rs
@@ -1,6 +1,7 @@
 use crate::test;
-use std::io::{BufRead, BufReader, Read, Write};
-use std::thread;
+use crate::test::testserver::{read_headers, TestServer};
+use std::io::{self, Write};
+use std::net::TcpStream;
 use std::time::Duration;
 
 use super::super::*;
@@ -56,47 +57,25 @@ fn agent_cookies() {
     agent.get("test://host/agent_cookies").call();
 }
 
-// Start a test server on an available port, that times out idle connections at 2 seconds.
-// Return the port this server is listening on.
-fn start_idle_timeout_server() -> u16 {
-    let listener = std::net::TcpListener::bind("localhost:0").unwrap();
-    let port = listener.local_addr().unwrap().port();
-    thread::spawn(move || {
-        for stream in listener.incoming() {
-            thread::spawn(move || {
-                let stream = stream.unwrap();
-                stream
-                    .set_read_timeout(Some(Duration::from_secs(2)))
-                    .unwrap();
-                let mut write_stream = stream.try_clone().unwrap();
-                for line in BufReader::new(stream).lines() {
-                    let line = match line {
-                        Ok(x) => x,
-                        Err(_) => return,
-                    };
-                    if line == "" {
-                        write_stream
-                            .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\nresponse")
-                            .unwrap();
-                    }
-                }
-            });
-        }
-    });
-    port
+// Handler that answers with a simple HTTP response, and times
+// out idle connections after 2 seconds.
+fn idle_timeout_handler(mut stream: TcpStream) -> io::Result<()> {
+    read_headers(&stream);
+    stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 8\r\n\r\nresponse")?;
+    stream.set_read_timeout(Some(Duration::from_secs(2)))?;
+    Ok(())
 }
 
 #[test]
 fn connection_reuse() {
-    let port = start_idle_timeout_server();
-    let url = format!("http://localhost:{}", port);
+    let testserver = TestServer::new(idle_timeout_handler);
+    let url = format!("http://localhost:{}", testserver.port);
     let agent = Agent::default().build();
     let resp = agent.get(&url).call();
 
     // use up the connection so it gets returned to the pool
     assert_eq!(resp.status(), 200);
-    let mut buf = vec![];
-    resp.into_reader().read_to_end(&mut buf).unwrap();
+    resp.into_string().unwrap();
 
     {
         let mut guard_state = agent.state.lock().unwrap();
diff --git a/src/test/mod.rs b/src/test/mod.rs
index 2baa08f..801b7c9 100644
--- a/src/test/mod.rs
+++ b/src/test/mod.rs
@@ -14,6 +14,7 @@ mod query_string;
 mod range;
 mod redirect;
 mod simple;
+mod testserver;
 mod timeout;
 
 type RequestHandler = dyn Fn(&Unit) -> Result<Stream, Error> + Send + 'static;
diff --git a/src/test/testserver.rs b/src/test/testserver.rs
new file mode 100644
index 0000000..7c563a1
--- /dev/null
+++ b/src/test/testserver.rs
@@ -0,0 +1,50 @@
+use std::io::{self, BufRead, BufReader};
+use std::net::{TcpListener, TcpStream};
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::Arc;
+use std::thread;
+
+pub struct TestServer {
+    pub port: u16,
+    pub done: Arc<AtomicBool>,
+}
+
+// Read a stream until reaching a blank line, in order to consume
+// request headers.
+pub fn read_headers(stream: &TcpStream) {
+    for line in BufReader::new(stream).lines() {
+        if line.unwrap() == "" {
+            break;
+        }
+    }
+}
+
+impl TestServer {
+    pub fn new(handler: fn(TcpStream) -> io::Result<()>) -> Self {
+        let listener = TcpListener::bind("localhost:0").unwrap();
+        let port = listener.local_addr().unwrap().port();
+        let done = Arc::new(AtomicBool::new(false));
+        let done_clone = done.clone();
+        thread::spawn(move || {
+            for stream in listener.incoming() {
+                thread::spawn(move || handler(stream.unwrap()));
+                if done.load(Ordering::Relaxed) {
+                    break;
+                }
+            }
+            println!("testserver on {} exiting", port);
+        });
+        TestServer {
+            port,
+            done: done_clone,
+        }
+    }
+}
+
+impl Drop for TestServer {
+    fn drop(&mut self) {
+        self.done.store(true, Ordering::Relaxed);
+        // Connect once to unblock the listen loop.
+        TcpStream::connect(format!("localhost:{}", self.port)).unwrap();
+    }
+}
diff --git a/src/test/timeout.rs b/src/test/timeout.rs
index 4e14df5..0ccafca 100644
--- a/src/test/timeout.rs
+++ b/src/test/timeout.rs
@@ -1,4 +1,5 @@
-use std::io::{self, BufRead, BufReader, Read, Write};
+use crate::test::testserver::*;
+use std::io::{self, Write};
 use std::net::TcpStream;
 use std::thread;
 use std::time::Duration;
@@ -7,7 +8,8 @@ use super::super::*;
 
 // Send an HTTP response on the TcpStream at a rate of two bytes every 10
 // milliseconds, for a total of 600 bytes.
-fn dribble_body_respond(stream: &mut TcpStream) -> io::Result<()> {
+fn dribble_body_respond(mut stream: TcpStream) -> io::Result<()> {
+    read_headers(&stream);
     let contents = [b'a'; 300];
     let headers = format!(
         "HTTP/1.1 200 OK\r\nContent-Length: {}\r\n\r\n",
@@ -23,49 +25,12 @@ fn dribble_body_respond(stream: &mut TcpStream) -> io::Result<()> {
     Ok(())
 }
 
-// Read a stream until reaching a blank line, in order to consume
-// request headers.
-fn read_headers(stream: &TcpStream) {
-    for line in BufReader::new(stream).lines() {
-        let line = match line {
-            Ok(x) => x,
-            Err(_) => return,
-        };
-        if line == "" {
-            break;
-        }
-    }
-}
-
-// Start a test server on an available port, that dribbles out a response at 1 write per 10ms.
-// Return the port this server is listening on.
-fn start_dribble_body_server() -> u16 {
-    let listener = std::net::TcpListener::bind("localhost:0").unwrap();
-    let port = listener.local_addr().unwrap().port();
-    let dribble_handler = |mut stream: TcpStream| {
-        read_headers(&stream);
-        if let Err(e) = dribble_body_respond(&mut stream) {
-            eprintln!("sending dribble repsonse: {}", e);
-        }
-    };
-    thread::spawn(move || {
-        for stream in listener.incoming() {
-            thread::spawn(move || dribble_handler(stream.unwrap()));
-        }
-    });
-    port
-}
-
 fn get_and_expect_timeout(url: String) {
     let agent = Agent::default().build();
     let timeout = Duration::from_millis(500);
     let resp = agent.get(&url).timeout(timeout).call();
 
-    let mut reader = resp.into_reader();
-    let mut bytes = vec![];
-    let result = reader.read_to_end(&mut bytes);
-
-    match result {
+    match resp.into_string() {
         Err(io_error) => match io_error.kind() {
             io::ErrorKind::WouldBlock => Ok(()),
             io::ErrorKind::TimedOut => Ok(()),
@@ -78,15 +43,15 @@ fn get_and_expect_timeout(url: String) {
 
 #[test]
 fn overall_timeout_during_body() {
-    let port = start_dribble_body_server();
-    let url = format!("http://localhost:{}/", port);
-
+    // Start a test server on an available port, that dribbles out a response at 1 write per 10ms.
+    let server = TestServer::new(dribble_body_respond);
+    let url = format!("http://localhost:{}/", server.port);
     get_and_expect_timeout(url);
 }
 
 // Send HTTP headers on the TcpStream at a rate of one header every 100
 // milliseconds, for a total of 30 headers.
-fn dribble_headers_respond(stream: &mut TcpStream) -> io::Result<()> {
+fn dribble_headers_respond(mut stream: TcpStream) -> io::Result<()> {
     stream.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n")?;
     for _ in 0..30 {
         stream.write_all(b"a: b\n")?;
@@ -96,28 +61,10 @@ fn dribble_headers_respond(stream: &mut TcpStream) -> io::Result<()> {
     Ok(())
 }
 
-// Start a test server on an available port, that dribbles out response *headers* at 1 write per 10ms.
-// Return the port this server is listening on.
-fn start_dribble_headers_server() -> u16 {
-    let listener = std::net::TcpListener::bind("localhost:0").unwrap();
-    let port = listener.local_addr().unwrap().port();
-    let dribble_handler = |mut stream: TcpStream| {
-        read_headers(&stream);
-        if let Err(e) = dribble_headers_respond(&mut stream) {
-            eprintln!("sending dribble repsonse: {}", e);
-        }
-    };
-    thread::spawn(move || {
-        for stream in listener.incoming() {
-            thread::spawn(move || dribble_handler(stream.unwrap()));
-        }
-    });
-    port
-}
-
 #[test]
 fn overall_timeout_during_headers() {
-    let port = start_dribble_headers_server();
-    let url = format!("http://localhost:{}/", port);
+    // Start a test server on an available port, that dribbles out a response at 1 write per 10ms.
+    let server = TestServer::new(dribble_headers_respond);
+    let url = format!("http://localhost:{}/", server.port);
     get_and_expect_timeout(url);
 }