Zero-length responses break connection reuse

[njsmith]

Here's the code for handling responses with fixed-length bodies:

ureq/src/response.rs

Lines 325 to 343 in 28d667a

	// Responses with a content-length header means we should limit the reading
	// of the body to the number of bytes in the header. Once done, we can
	// return the underlying stream to the connection pool.
	(false, Some(len)) => {
	let mut pooler =
	PoolReturnRead::new(&unit.agent, &unit.url, LimitedRead::new(stream, len));
	if len <= buffer_len {
	debug!("Body entirely buffered (length: {})", len);
	let mut buf = vec![0; len];
	pooler
	.read_exact(&mut buf)
	.expect("failed to read exact buffer length from stream");
	Box::new(Cursor::new(buf))
	} else {
	debug!("Streaming body until content-length: {}", len);
	Box::new(pooler)
	}
	}

We set up the PoolReturnRead, and then if the body is short enough, we call .read_exact on the PoolReturnRead to both fetch the body and, as a side-effect, force PoolReturnRead to notice that the body has been consumed and that it's free to return the connection to the pool.

However, if the response body is empty (e.g. on a 304 Not Modified), then len is 0, and we call read_exact on a zero-length buffer. In this case, the default implementation of std::io::Read::read_exact exits immediately. In particular, it does not call PoolReturnRead::read at all, so PoolReturnRead has no chance to notice that the stream has been exhausted. Then we drop the PoolReturnRead, and since it believes the body hasn't been exhausted, it drops the underlying connection and on our next request we'll have to set up a new connection from scratch.

(Ironically, this seems to make my program slower on a warm cache than a cold cache, because in many cases fetching the data is cheaper than restarting the connection.)