Previously, we treated HTTP/1.0 responses as always being
close-delimited. However, that's not quite right. HTTP/1.0 responses
_default_ to Connection: close, but the server may send Connection:
keep-alive, along with a Content-Length header. Update body_type to
understand this.
Also, slightly reorganize body_type. has_no_body was being checked
redundantly when we could just early-return when has_no_body is true. And
regardless of whether we see Connection: close (on any HTTP version),
we must honor Transfer-Encoding or Content-Length if they are present.
Change the handling of `Connection: close` to more directly affect
whether a connection is pooled, regardless of what method was used
to delimit the body.
Fixes#600
These test were failing because they made live requests to httpbin.org,
which is serving intermittent 500s.
In general we strive to make our test and doctests interact with a local
test server rather than a live website. For doctests we do this with
some sleight of hand in a hidden `ureq::is_test(true)`, which swaps out
the default agent returned by ureq::agent. However, since the middleware
doctests rely on constructing a custom agent with the middleware, they
can't rely on this.
I suspect we can get around this problem but it may take some thinking.
Similarly disable the AgentBuilder::user_agent and redirects doctests
and the agent_set_header unittest.
This removes the cookie header on redirect, otherwise if there are
multiple redirects, a new cookie header gets appended each time, while
the existing one remains. This could result in the cookies being leaked
to a third party.
Previously, `ureq` used `HTTP CONNECT` for *all* requests, including
plain-text HTTP requests. However, some proxy servers like Squid
only allow tunneling via the `HTTP CONNECT` method on port 443 - since
it is usually only used to proxy HTTPS requests. As a result,
it was not possible to use `ureq` with a Squid server in its default configuration.
With the changes from this commit, ureq will interact with HTTP proxies
in a more standard-conforming way, where `CONNECT` is only used for
HTTPS requests. HTTP request paths are transformed in such a way that they
comply with RFC 7230 [1].
Tested against Squid 4.13 in standard configuration, with and without basic authentication,
for HTTP and HTTPS requests.
[1] https://www.rfc-editor.org/rfc/rfc7230#section-5.3.2
This breaks a reference cycle between PoolReturner and Agent that was
causing Agents (and their contained ConnectionPool) to never be dropped
so long as there was any stream in the ConnectionPool. This cause
sockets to leak over time, particularly when the convenience functions
ureq::get(), ureq::post(), etc were used, since those functions create
a new Agent each time.
Introduce PoolReturner, a handle on an agent and a PoolKey that is
capable of returning a Stream to a Pool. Make Streams keep track of
their own PoolReturner, instead of having PoolReturnRead keep track of
that information.
For the LimitedRead code path, get rid of PoolReturnRead. Instead,
LimitedRead is responsible for returning its Stream to the Pool after
its second-to-last read. In other words, LimitedRead will return the
stream if the next read is guaranteed to return Ok(0).
Constructing a LimitedRead of size 0 is always wrong, because we could
always just return the stream immediately. Change the size argument to
NonZeroUsize to enforce that.
Remove the Done trait, which was only used for LimitedRead. It was used
to try and make sure we returned the stream to the pool on exact reads,
but was not reliable.
This does not yet move the ChunkDecoder code path away from
PoolReturnRead. That requires a little more work.
Part 1 of #559. Fixes#555.
This allows removing the hack where we create a Response with an empty `reader`,
then immediately mutate it to set the real reader. It also happens to allow us
to get rid of 3 fields of Response that were only used to pass information to
`stream_to_reader`.
I've tried to keep the structure and logic of the body calculation as close to
the same as possible, for ease of review and to avoid introducing bugs. I think
there are some followup fixes we can make to the logic, which will be made
easier by having it in a self contained function.
This is more correct, since all gzip streams can consist of multiple members. It
has the happy side effect that it causes gzipped responses to reliably return
their streams to the pool.
The mbedtls example has caused problem in the main build a number of
times. By making it a standalone `cargo new --bin`, we can keep it in
the source tree as a good example but avoid having it break the main
build.
Also, fix some clippy lints.
