From bd2761e28015c713fc77f55e2bfa933afa5627d1 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 19 Oct 2020 06:43:27 +0000 Subject: [PATCH 01/11] Update env_logger requirement from 0.7.1 to 0.8.1 Updates the requirements on [env_logger](https://github.com/env-logger-rs/env_logger) to permit the latest version. - [Release notes](https://github.com/env-logger-rs/env_logger/releases) - [Changelog](https://github.com/env-logger-rs/env_logger/blob/master/CHANGELOG.md) - [Commits](https://github.com/env-logger-rs/env_logger/compare/v0.7.1...v0.8.1) Signed-off-by: dependabot-preview[bot] --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index ac9318a..e449064 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ serde = { version = "1", features = ["derive"] } rayon = "1.3.0" rayon-core = "1.7.0" chrono = "0.4.11" -env_logger = "0.7.1" +env_logger = "0.8.1" [[example]] name = "smoke-test" From 67c28d28a352c832b7357e0eb0a015cc3c3d2155 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 20 Oct 2020 23:46:54 -0700 Subject: [PATCH 02/11] Check for synthetic_error early in unit::connect --- src/response.rs | 5 +++++ src/unit.rs | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/response.rs b/src/response.rs index 38f44f1..a768535 100644 --- a/src/response.rs +++ b/src/response.rs @@ -221,6 +221,11 @@ impl Response { &self.error } + // Internal-only API, to allow unit::connect to return early on errors. + pub(crate) fn into_error(self) -> Option { + self.error + } + /// The content type part of the "Content-Type" header without /// the charset. /// diff --git a/src/unit.rs b/src/unit.rs index 3ec4794..0431009 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -181,6 +181,9 @@ pub(crate) fn connect( // start reading the response to process cookies and redirects. let mut stream = stream::DeadlineStream::new(stream, unit.deadline); let mut resp = Response::from_read(&mut stream); + if resp.synthetic_error().is_some() { + return Err(resp.into_error().unwrap()); + } // https://tools.ietf.org/html/rfc7230#section-6.3.1 // When an inbound connection is closed prematurely, a client MAY From 14475cb5c797714a4158d98deaf3e5c1f8947a5d Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 21 Oct 2020 00:19:30 -0700 Subject: [PATCH 03/11] Add test for read_timeout during headers. --- src/test/timeout.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/timeout.rs b/src/test/timeout.rs index cb69055..2553dd9 100644 --- a/src/test/timeout.rs +++ b/src/test/timeout.rs @@ -59,6 +59,18 @@ fn dribble_headers_respond(mut stream: TcpStream) -> io::Result<()> { Ok(()) } +#[test] +fn read_timeout_during_headers() { + let server = TestServer::new(dribble_headers_respond); + let url = format!("http://localhost:{}/", server.port); + let resp = crate::get(&url).timeout_read(10).call(); + assert!(!resp.ok()); + assert_eq!( + resp.into_string().unwrap(), + "Network Error: timed out reading response\n" + ); +} + #[test] fn overall_timeout_during_headers() { // Start a test server on an available port, that dribbles out a response at 1 write per 10ms. From 1f5f65877ad2911c599a12e79ef794b9e16f2a39 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 21 Oct 2020 10:08:47 -0700 Subject: [PATCH 04/11] Update overall_timeout_during_headers test. --- src/test/timeout.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/timeout.rs b/src/test/timeout.rs index 2553dd9..be9654a 100644 --- a/src/test/timeout.rs +++ b/src/test/timeout.rs @@ -28,15 +28,15 @@ 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(); - - match resp.into_string() { - Err(io_error) => match io_error.kind() { - io::ErrorKind::TimedOut => Ok(()), - _ => Err(format!("{:?}", io_error)), - }, - Ok(_) => Err("successful response".to_string()), - } - .expect("expected timeout but got something else"); + assert!( + matches!(resp.synthetic_error(), Some(Error::Io(_))), + "expected timeout error, got {:?}", + resp.synthetic_error() + ); + assert_eq!( + resp.synthetic_error().as_ref().unwrap().body_text(), + "Network Error: timed out reading response" + ); } #[test] From 32f9ebc04ad147a199c6ae753a6b7f7a878f7d8a Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 21 Oct 2020 10:26:00 -0700 Subject: [PATCH 05/11] Separate overall_timeout checks. --- src/test/timeout.rs | 32 +++++++++++++++++++++----------- src/unit.rs | 5 ++--- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/test/timeout.rs b/src/test/timeout.rs index be9654a..7a296e4 100644 --- a/src/test/timeout.rs +++ b/src/test/timeout.rs @@ -28,15 +28,15 @@ 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(); - assert!( - matches!(resp.synthetic_error(), Some(Error::Io(_))), - "expected timeout error, got {:?}", - resp.synthetic_error() - ); - assert_eq!( - resp.synthetic_error().as_ref().unwrap().body_text(), - "Network Error: timed out reading response" - ); + + match resp.into_string() { + Err(io_error) => match io_error.kind() { + io::ErrorKind::TimedOut => Ok(()), + _ => Err(format!("{:?}", io_error)), + }, + Ok(_) => Err("successful response".to_string()), + } + .expect("expected timeout but got something else"); } #[test] @@ -50,7 +50,6 @@ fn overall_timeout_during_body() { // 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(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")?; stream.flush()?; @@ -76,7 +75,18 @@ fn overall_timeout_during_headers() { // 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); + let agent = Agent::default().build(); + let timeout = Duration::from_millis(500); + let resp = agent.get(&url).timeout(timeout).call(); + assert!( + matches!(resp.synthetic_error(), Some(Error::Io(_))), + "expected timeout error, got {:?}", + resp.synthetic_error() + ); + assert_eq!( + resp.synthetic_error().as_ref().unwrap().body_text(), + "Network Error: timed out reading response" + ); } #[test] diff --git a/src/unit.rs b/src/unit.rs index 0431009..2957af1 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -181,9 +181,6 @@ pub(crate) fn connect( // start reading the response to process cookies and redirects. let mut stream = stream::DeadlineStream::new(stream, unit.deadline); let mut resp = Response::from_read(&mut stream); - if resp.synthetic_error().is_some() { - return Err(resp.into_error().unwrap()); - } // https://tools.ietf.org/html/rfc7230#section-6.3.1 // When an inbound connection is closed prematurely, a client MAY @@ -201,6 +198,8 @@ pub(crate) fn connect( let empty = Payload::Empty.into_read(); return connect(req, unit, false, redirect_count, empty, redir); } + // Non-retryable errors return early. + return Err(resp.into_error().unwrap()); } // squirrel away cookies From 2bf9362eff0e77aac7f182effe13844e6171d3c2 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 21 Oct 2020 20:39:34 -0700 Subject: [PATCH 06/11] Reinstate read timeouts on body. This feature was broken in #67, which reset timeouts on the stream before passing it to set_stream. As part of this change, refactor the internal storage of timeouts on the Request object to use Option. Remove the deadline field on Response. It wasn't used. The deadline field on unit was used instead. Add a unittest. --- src/request.rs | 16 +++++++++++----- src/response.rs | 26 ++++++++++++++++++-------- src/stream.rs | 20 ++++++++++---------- src/test/timeout.rs | 18 +++++++++++++++++- 4 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/request.rs b/src/request.rs index daee345..7d469fb 100644 --- a/src/request.rs +++ b/src/request.rs @@ -1,8 +1,8 @@ -use std::fmt; use std::io::Read; use std::result::Result; use std::sync::{Arc, Mutex}; use std::time; +use std::{fmt, time::Duration}; use qstring::QString; use url::{form_urlencoded, Url}; @@ -40,8 +40,8 @@ pub struct Request { pub(crate) headers: Vec
, pub(crate) query: QString, pub(crate) timeout_connect: u64, - pub(crate) timeout_read: u64, - pub(crate) timeout_write: u64, + pub(crate) timeout_read: Option, + pub(crate) timeout_write: Option, pub(crate) timeout: Option, pub(crate) redirects: u32, pub(crate) proxy: Option, @@ -368,7 +368,10 @@ impl Request { /// println!("{:?}", r); /// ``` pub fn timeout_read(&mut self, millis: u64) -> &mut Request { - self.timeout_read = millis; + match millis { + 0 => self.timeout_read = None, + m => self.timeout_read = Some(Duration::from_millis(m)), + } self } @@ -385,7 +388,10 @@ impl Request { /// println!("{:?}", r); /// ``` pub fn timeout_write(&mut self, millis: u64) -> &mut Request { - self.timeout_write = millis; + match millis { + 0 => self.timeout_write = None, + m => self.timeout_write = Some(Duration::from_millis(m)), + } self } diff --git a/src/response.rs b/src/response.rs index a768535..75ba391 100644 --- a/src/response.rs +++ b/src/response.rs @@ -1,7 +1,6 @@ use std::fmt; use std::io::{self, Cursor, ErrorKind, Read}; use std::str::FromStr; -use std::time::Instant; use chunked_transfer::Decoder as ChunkDecoder; @@ -56,7 +55,6 @@ pub struct Response { headers: Vec
, unit: Option, stream: Option, - deadline: Option, } /// index into status_line where we split: HTTP/1.1 200 OK @@ -327,12 +325,17 @@ impl Response { let stream = self.stream.expect("No reader in response?!"); let unit = self.unit; + if let Some(unit) = &unit { + let result = stream.set_read_timeout(unit.req.timeout_read); + if let Err(e) = result { + return Box::new(ErrorReader(e)) as Box; + } + } let deadline = unit.as_ref().and_then(|u| u.deadline); let stream = DeadlineStream::new(stream, deadline); match (use_chunked, limit_bytes) { - (true, _) => Box::new(PoolReturnRead::new(unit, ChunkDecoder::new(stream))) - as Box, + (true, _) => Box::new(PoolReturnRead::new(unit, ChunkDecoder::new(stream))), (false, Some(len)) => { Box::new(PoolReturnRead::new(unit, LimitedRead::new(stream, len))) } @@ -505,7 +508,6 @@ impl Response { headers, unit: None, stream: None, - deadline: None, }) } @@ -585,9 +587,6 @@ impl Into for Error { /// *Internal API* pub(crate) fn set_stream(resp: &mut Response, url: String, unit: Option, stream: Stream) { resp.url = Some(url); - if let Some(unit) = &unit { - resp.deadline = unit.deadline; - } resp.unit = unit; resp.stream = Some(stream); } @@ -813,3 +812,14 @@ mod tests { assert_eq!(v, "Bad Status\n"); } } + +// ErrorReader returns an error for every read. +// The error is as close to a clone of the underlying +// io::Error as we can get. +struct ErrorReader(io::Error); + +impl Read for ErrorReader { + fn read(&mut self, _buf: &mut [u8]) -> io::Result { + Err(io::Error::new(self.0.kind(), self.0.to_string())) + } +} diff --git a/src/stream.rs b/src/stream.rs index f14a3ae..cf53c4d 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -172,6 +172,14 @@ impl Stream { } } + pub(crate) fn set_read_timeout(&self, timeout: Option) -> io::Result<()> { + if let Some(socket) = self.socket() { + socket.set_read_timeout(timeout) + } else { + Ok(()) + } + } + #[cfg(test)] pub fn to_write_vec(&self) -> Vec { match self { @@ -453,24 +461,16 @@ pub(crate) fn connect_host(unit: &Unit, hostname: &str, port: u16) -> Result 0 { - stream - .set_read_timeout(Some(Duration::from_millis(unit.req.timeout_read as u64))) - .ok(); } else { - stream.set_read_timeout(None).ok(); + stream.set_read_timeout(unit.req.timeout_read)?; } if let Some(deadline) = deadline { stream .set_write_timeout(Some(time_until_deadline(deadline)?)) .ok(); - } else if unit.req.timeout_write > 0 { - stream - .set_write_timeout(Some(Duration::from_millis(unit.req.timeout_write as u64))) - .ok(); } else { - stream.set_write_timeout(None).ok(); + stream.set_read_timeout(unit.req.timeout_read)?; } if proto == Some(Proto::HTTPConnect) { diff --git a/src/test/timeout.rs b/src/test/timeout.rs index 7a296e4..c9a0821 100644 --- a/src/test/timeout.rs +++ b/src/test/timeout.rs @@ -19,7 +19,7 @@ fn dribble_body_respond(mut stream: TcpStream, contents: &[u8]) -> io::Result<() stream.write_all(&contents[i..i + 1])?; stream.write_all(&[b'\n'; 1])?; stream.flush()?; - thread::sleep(Duration::from_millis(10)); + thread::sleep(Duration::from_millis(100)); } Ok(()) } @@ -47,6 +47,22 @@ fn overall_timeout_during_body() { get_and_expect_timeout(url); } +#[test] +fn read_timeout_during_body() { + let server = TestServer::new(|stream| dribble_body_respond(stream, &[b'a'; 300])); + let url = format!("http://localhost:{}/", server.port); + let agent = Agent::default().build(); + let resp = agent.get(&url).timeout_read(5).call(); + match resp.into_string() { + Err(io_error) => match io_error.kind() { + io::ErrorKind::TimedOut => Ok(()), + _ => Err(format!("{:?}", io_error)), + }, + Ok(_) => Err("successful response".to_string()), + } + .expect("expected timeout but got something else"); +} + // 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(mut stream: TcpStream) -> io::Result<()> { From 22e38393403570131221fc9fa8e1f20540130801 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Thu, 22 Oct 2020 00:10:30 -0700 Subject: [PATCH 07/11] Review feedback. --- src/stream.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index cf53c4d..d057e53 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -455,20 +455,14 @@ pub(crate) fn connect_host(unit: &Unit, hostname: &str, port: u16) -> Result Date: Sun, 25 Oct 2020 11:08:50 +0100 Subject: [PATCH 08/11] API changes for 2.0 * Remove Request::build * All mutations on Request follow builder pattern The previous `build()` on request was necessary because mutating functions did not follow a proper builder pattern (taking `&mut self` instead of `mut self`). With a proper builder pattern, the need for `.build()` goes away. * All Request body and call methods consume self Anything which "executes" the request will now consume the `Request` to produce a `Result`. * Move all config from request to agent builder Timeouts, redirect config, proxy settings and TLS config are now on `AgentBuilder`. * Rename max_pool_connections -> max_idle_connections * Rename max_pool_connections_per_host -> max_idle_connections_per_host Consistent internal and external naming. * Introduce new AgentConfig for static config created by builder. `Agent` can be seen as having two parts. Static config and a mutable shared state between all states. The static config goes into `AgentConfig` and the mutable shared state into `AgentState`. * Replace all use of `Default` for `new`. Deriving or implementing `Default` makes for a secondary instantiation API. It is useful in some cases, but gets very confusing when there is both `new` _and_ a `Default`. It's especially devious for derived values where a reasonable default is not `0`, `false` or `None`. * Remove feature native_tls, we want only native rustls. This feature made for very clunky handling throughout the code. From a security point of view, it's better to stick with one single TLS API. Rustls recently got an official audit (very positive). https://github.com/ctz/rustls/tree/master/audit Rustls deliberately omits support for older, insecure TLS such as TLS 1.1 or RC4. This might be a problem for a user of ureq, but on balance not considered important enough to keep native_tls. * Remove auth and support for basic auth. The API just wasn't enough. A future reintroduction should at least also provide a `Bearer` mechanism and possibly more. * Rename jar -> cookie_store * Rename jar -> cookie_tin Just make some field names sync up with the type. * Drop "cookies" as default feature The need for handling cookies is probably rare, let's not enable it by default. * Change all feature checks for "cookie" to "cookies" The outward facing feature is "cookies" and I think it's better form that the code uses the official feature name instead of the optional library "cookies". * Keep `set` on Agent level as well as AgentBuilder. The idea is that an auth exchange might result in a header that need to be set _after_ the agent has been built. --- .github/workflows/test.yml | 1 - Cargo.toml | 3 +- README.md | 4 +- examples/smoke-test/main.rs | 11 +- src/agent.rs | 332 ++++++++++++++++++++++++------------ src/cookies.rs | 12 +- src/error.rs | 5 - src/lib.rs | 21 ++- src/pool.rs | 56 +++--- src/request.rs | 316 +++++----------------------------- src/resolve.rs | 6 - src/response.rs | 2 +- src/stream.rs | 99 +++-------- src/test/agent_test.rs | 22 ++- src/test/auth.rs | 64 ------- src/test/mod.rs | 1 - src/test/range.rs | 6 +- src/test/redirect.rs | 14 +- src/test/simple.rs | 6 +- src/test/testserver.rs | 12 +- src/test/timeout.rs | 8 +- src/unit.rs | 28 +-- test.sh | 2 +- tests/https-agent.rs | 14 +- 24 files changed, 398 insertions(+), 647 deletions(-) delete mode 100644 src/test/auth.rs diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index be7342b..90a02f4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -27,7 +27,6 @@ jobs: tls: - "" - tls - - native-tls feature: - "" - json diff --git a/Cargo.toml b/Cargo.toml index 703b26e..277a7a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ edition = "2018" features = [ "tls", "json", "charset", "cookies", "socks-proxy" ] [features] -default = ["tls", "cookies"] +default = ["tls"] json = ["serde", "serde_json"] charset = ["encoding"] tls = ["rustls", "webpki", "webpki-roots"] @@ -38,7 +38,6 @@ rustls-native-certs = { version = "0.4", optional = true } serde = { version = "1", optional = true } serde_json = { version = "1", optional = true } encoding = { version = "0.2", optional = true } -native-tls = { version = "0.2", optional = true } cookie_store = { version = "0.12.0", optional = true } log = "0.4.11" diff --git a/README.md b/README.md index 6323277..396acac 100644 --- a/README.md +++ b/README.md @@ -61,9 +61,7 @@ You can control them when including `ureq` as a dependency. ``` * `tls` enables https. This is enabled by default. -* `native-tls` enables https using the [`native-tls`](https://crates.io/crates/native-tls) crate. - NB: To make this work you currently need to use `default-features: false` to disable `tls`. - We plan on fixing that. +* `cookies` enables handling cookies between requests in an agent. * `json` enables `response.into_json()` and `request.send_json()` via serde_json. * `charset` enables interpreting the charset part of `Content-Type: text/plain; charset=iso-8859-1`. Without this, the library diff --git a/examples/smoke-test/main.rs b/examples/smoke-test/main.rs index 6f0e368..3d9018d 100644 --- a/examples/smoke-test/main.rs +++ b/examples/smoke-test/main.rs @@ -40,11 +40,7 @@ impl fmt::Display for Oops { type Result = result::Result; fn get(agent: &ureq::Agent, url: &String) -> Result> { - let response = agent - .get(url) - .timeout_connect(std::time::Duration::from_secs(5)) - .timeout(Duration::from_secs(20)) - .call()?; + let response = agent.get(url).call()?; let mut reader = response.into_reader(); let mut bytes = vec![]; reader.read_to_end(&mut bytes)?; @@ -61,7 +57,10 @@ fn get_and_write(agent: &ureq::Agent, url: &String) -> Result<()> { } fn get_many(urls: Vec, simultaneous_fetches: usize) -> Result<()> { - let agent = ureq::Agent::default(); + let agent = ureq::builder() + .timeout_connect(std::time::Duration::from_secs(5)) + .timeout(Duration::from_secs(20)) + .build(); let pool = rayon::ThreadPoolBuilder::new() .num_threads(simultaneous_fetches) .build()?; diff --git a/src/agent.rs b/src/agent.rs index 38dfb99..7efc798 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -1,36 +1,45 @@ -#[cfg(feature = "cookie")] -use cookie::Cookie; -#[cfg(feature = "cookie")] -use cookie_store::CookieStore; use std::sync::Arc; -#[cfg(feature = "cookie")] -use url::Url; -#[cfg(feature = "cookie")] -use crate::cookies::CookieTin; use crate::header::{self, Header}; use crate::pool::ConnectionPool; use crate::proxy::Proxy; use crate::request::Request; -use crate::resolve::ArcResolver; +use crate::resolve::{ArcResolver, StdResolver}; +use std::time; -#[derive(Debug, Default)] +#[cfg(feature = "cookies")] +use crate::cookies::CookieTin; +#[cfg(feature = "cookies")] +use cookie::Cookie; +#[cfg(feature = "cookies")] +use cookie_store::CookieStore; +#[cfg(feature = "cookies")] +use url::Url; + +#[derive(Debug)] pub struct AgentBuilder { headers: Vec
, - proxy: Option, - max_idle_connections: usize, - max_idle_connections_per_host: usize, + config: AgentConfig, /// Cookies saved between requests. /// Invariant: All cookies must have a nonempty domain and path. - #[cfg(feature = "cookie")] - jar: CookieStore, + #[cfg(feature = "cookies")] + cookie_store: Option, resolver: ArcResolver, } -impl Default for Agent { - fn default() -> Self { - AgentBuilder::new().build() - } +/// Config as built by AgentBuilder and then static for the lifetime of the Agent. +#[derive(Debug, Clone)] +pub(crate) struct AgentConfig { + pub max_idle_connections: usize, + pub max_idle_connections_per_host: usize, + pub proxy: Option, + pub timeout_connect: Option, + pub timeout_read: Option, + pub timeout_write: Option, + pub timeout: Option, + pub redirects: u32, + #[cfg(feature = "tls")] + pub tls_config: Option, } /// Agents keep state between requests. @@ -40,12 +49,13 @@ impl Default for Agent { /// can keep a state. /// /// ``` -/// let agent = ureq::agent(); +/// let mut agent = ureq::agent(); +/// +/// agent.set("x-my-secret-header", "very secret"); /// /// let auth = agent /// .post("/login") -/// .auth("martin", "rubbermashgum") -/// .call(); // blocks. puts auth cookies in agent. +/// .call(); // blocks. /// /// if auth.is_err() { /// println!("Noes!"); @@ -66,6 +76,7 @@ impl Default for Agent { /// that shares the same underlying connection pool and other state. #[derive(Debug, Clone)] pub struct Agent { + pub(crate) config: Arc, /// Copied into each request of this agent. pub(crate) headers: Vec
, /// Reused agent state for repeated requests from this agent. @@ -75,19 +86,47 @@ pub struct Agent { /// Container of the state /// /// *Internal API*. -#[derive(Debug, Default)] +#[derive(Debug)] pub(crate) struct AgentState { /// Reused connections between requests. pub(crate) pool: ConnectionPool, pub(crate) proxy: Option, /// Cookies saved between requests. /// Invariant: All cookies must have a nonempty domain and path. - #[cfg(feature = "cookie")] - pub(crate) jar: CookieTin, + #[cfg(feature = "cookies")] + pub(crate) cookie_tin: CookieTin, pub(crate) resolver: ArcResolver, } impl Agent { + /// Creates an Agent with default settings. + /// + /// Same as `AgentBuilder::new().build()`. + pub fn new() -> Self { + AgentBuilder::new().build() + } + + /// Set a extra header field that will be present in all following requests using the agent. + /// + /// This is useful for cases like auth, where we do a number of requests before getting + /// some credential that later must be presented in a header. + /// + /// Notice that fixed headers can also be set in the `AgentBuilder`. + /// + /// ``` + /// let mut agent = ureq::agent(); + /// + /// agent.set("X-API-Key", "foobar"); + /// agent.set("Accept", "text/plain"); + /// + /// let r = agent + /// .get("/my-page") + /// .call(); + /// ``` + pub fn set(&mut self, header: &str, value: &str) { + header::add_header(&mut self.headers, Header::new(header, value)); + } + /// Request by providing the HTTP verb such as `GET`, `POST`... /// /// ``` @@ -99,7 +138,7 @@ impl Agent { /// println!("{:?}", r); /// ``` pub fn request(&self, method: &str, path: &str) -> Request { - Request::new(&self, method.into(), path.into()) + Request::new(self.clone(), method.into(), path.into()) } /// Store a cookie in this agent. @@ -112,10 +151,10 @@ impl Agent { /// .finish(); /// agent.set_cookie(cookie, &"https://example.com/".parse().unwrap()); /// ``` - #[cfg(feature = "cookie")] + #[cfg(feature = "cookies")] pub fn set_cookie(&self, cookie: Cookie<'static>, url: &Url) { self.state - .jar + .cookie_tin .store_response_cookies(Some(cookie).into_iter(), url); } @@ -161,11 +200,24 @@ impl Agent { } impl AgentBuilder { - pub fn new() -> AgentBuilder { + pub fn new() -> Self { AgentBuilder { - max_idle_connections: 100, - max_idle_connections_per_host: 1, - ..Default::default() + headers: vec![], + config: AgentConfig { + max_idle_connections: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS, + max_idle_connections_per_host: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST, + proxy: None, + timeout_connect: Some(time::Duration::from_secs(30)), + timeout_read: None, + timeout_write: None, + timeout: None, + redirects: 5, + #[cfg(feature = "tls")] + tls_config: None, + }, + resolver: StdResolver.into(), + #[cfg(feature = "cookies")] + cookie_store: None, } } @@ -175,25 +227,29 @@ impl AgentBuilder { // not implement clone, so we have to give ownership to the newly // built Agent. pub fn build(self) -> Agent { + let config = Arc::new(self.config); Agent { - headers: self.headers.clone(), + headers: self.headers, state: Arc::new(AgentState { - pool: ConnectionPool::new( - self.max_idle_connections, - self.max_idle_connections_per_host, + pool: ConnectionPool::new_with_limits( + config.max_idle_connections, + config.max_idle_connections_per_host, + ), + proxy: config.proxy.clone(), + #[cfg(feature = "cookies")] + cookie_tin: CookieTin::new( + self.cookie_store.unwrap_or_else(|| CookieStore::default()), ), - proxy: self.proxy.clone(), - #[cfg(feature = "cookie")] - jar: CookieTin::new(self.jar), resolver: self.resolver, }), + config, } } /// Set a header field that will be present in all requests using the agent. /// /// ``` - /// let agent = ureq::AgentBuilder::new() + /// let agent = ureq::builder() /// .set("X-API-Key", "foobar") /// .set("Accept", "text/plain") /// .build(); @@ -213,39 +269,18 @@ impl AgentBuilder { self } - /// Basic auth that will be present in all requests using the agent. + /// Set the proxy server to use for all connections from this Agent. /// + /// Example: /// ``` + /// let proxy = ureq::Proxy::new("user:password@cool.proxy:9090").unwrap(); /// let agent = ureq::AgentBuilder::new() - /// .auth("martin", "rubbermashgum") + /// .proxy(proxy) /// .build(); - /// - /// let r = agent - /// .get("/my_page") - /// .call(); - /// println!("{:?}", r); /// ``` - pub fn auth(self, user: &str, pass: &str) -> Self { - let pass = basic_auth(user, pass); - self.auth_kind("Basic", &pass) - } - - /// Auth of other kinds such as `Digest`, `Token` etc, that will be present - /// in all requests using the agent. - /// - /// ``` - /// // sets a header "Authorization: token secret" - /// let agent = ureq::AgentBuilder::new() - /// .auth_kind("token", "secret") - /// .build(); - /// - /// let r = agent - /// .get("/my_page") - /// .call(); - /// ``` - pub fn auth_kind(self, kind: &str, pass: &str) -> Self { - let value = format!("{} {}", kind, pass); - self.set("Authorization", &value) + pub fn proxy(mut self, proxy: Proxy) -> Self { + self.config.proxy = Some(proxy); + self } /// Sets the maximum number of connections allowed in the connection pool. @@ -253,10 +288,10 @@ impl AgentBuilder { /// connection pooling. /// /// ``` - /// let agent = ureq::AgentBuilder::new().max_pool_connections(200).build(); + /// let agent = ureq::AgentBuilder::new().max_idle_connections(200).build(); /// ``` - pub fn max_pool_connections(mut self, max: usize) -> Self { - self.max_idle_connections = max; + pub fn max_idle_connections(mut self, max: usize) -> Self { + self.config.max_idle_connections = max; self } @@ -265,10 +300,10 @@ impl AgentBuilder { /// would disable connection pooling. /// /// ``` - /// let agent = ureq::AgentBuilder::new().max_pool_connections_per_host(200).build(); + /// let agent = ureq::AgentBuilder::new().max_idle_connections_per_host(200).build(); /// ``` - pub fn max_pool_connections_per_host(mut self, max: usize) -> Self { - self.max_idle_connections_per_host = max; + pub fn max_idle_connections_per_host(mut self, max: usize) -> Self { + self.config.max_idle_connections_per_host = max; self } @@ -296,27 +331,125 @@ impl AgentBuilder { self } - /// Set the proxy server to use for all connections from this Agent. + /// Timeout for the socket connection to be successful. + /// If both this and `.timeout()` are both set, `.timeout_connect()` + /// takes precedence. + /// + /// The default is 30 seconds. + /// + /// ``` + /// let agent = ureq::builder() + /// .timeout_connect(std::time::Duration::from_secs(1)) + /// .build(); + /// let r = agent.get("/my_page").call(); + /// ``` + pub fn timeout_connect(mut self, timeout: time::Duration) -> Self { + self.config.timeout_connect = Some(timeout); + self + } + + /// Timeout for the individual reads of the socket. + /// If both this and `.timeout()` are both set, `.timeout()` + /// takes precedence. + /// + /// The default is `0`, which means it can block forever. + /// + /// ``` + /// let agent = ureq::builder() + /// .timeout_read(std::time::Duration::from_secs(1)) + /// .build(); + /// let r = agent.get("/my_page").call(); + /// ``` + pub fn timeout_read(mut self, timeout: time::Duration) -> Self { + self.config.timeout_read = Some(timeout); + self + } + + /// Timeout for the individual writes to the socket. + /// If both this and `.timeout()` are both set, `.timeout()` + /// takes precedence. + /// + /// The default is `0`, which means it can block forever. + /// + /// ``` + /// let agent = ureq::builder() + /// .timeout_write(std::time::Duration::from_secs(1)) + /// .build(); + /// let r = agent.get("/my_page").call(); + /// ``` + pub fn timeout_write(mut self, timeout: time::Duration) -> Self { + self.config.timeout_write = Some(timeout); + self + } + + /// Timeout for the overall request, including DNS resolution, connection + /// time, redirects, and reading the response body. Slow DNS resolution + /// may cause a request to exceed the timeout, because the DNS request + /// cannot be interrupted with the available APIs. + /// + /// This takes precedence over `.timeout_read()` and `.timeout_write()`, but + /// not `.timeout_connect()`. + /// + /// ``` + /// // wait max 1 second for whole request to complete. + /// let agent = ureq::builder() + /// .timeout(std::time::Duration::from_secs(1)) + /// .build(); + /// let r = agent.get("/my_page").call(); + /// ``` + pub fn timeout(mut self, timeout: time::Duration) -> Self { + self.config.timeout = Some(timeout); + self + } + + /// How many redirects to follow. + /// + /// Defaults to `5`. Set to `0` to avoid redirects and instead + /// get a response object with the 3xx status code. + /// + /// If the redirect count hits this limit (and it's > 0), TooManyRedirects is returned. + /// + /// ``` + /// let r = ureq::builder() + /// .redirects(10) + /// .build() + /// .get("/my_page") + /// .call(); + /// println!("{:?}", r); + /// ``` + pub fn redirects(mut self, n: u32) -> Self { + self.config.redirects = n; + self + } + + /// Set the TLS client config to use for the connection. See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). + /// + /// See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). /// /// Example: /// ``` - /// let proxy = ureq::Proxy::new("user:password@cool.proxy:9090").unwrap(); - /// let agent = ureq::AgentBuilder::new() - /// .proxy(proxy) + /// let tls_config = std::sync::Arc::new(rustls::ClientConfig::new()); + /// let agent = ureq::builder() + /// .set_tls_config(tls_config.clone()) /// .build(); + /// let req = agent.post("https://cool.server"); /// ``` - pub fn proxy(mut self, proxy: Proxy) -> Self { - self.proxy = Some(proxy); + #[cfg(feature = "tls")] + pub fn set_tls_config(mut self, tls_config: Arc) -> Self { + self.config.tls_config = Some(TLSClientConfig(tls_config)); self } } -pub(crate) fn basic_auth(user: &str, pass: &str) -> String { - let safe = match user.find(':') { - Some(idx) => &user[..idx], - None => user, - }; - base64::encode(&format!("{}:{}", safe, pass)) +#[cfg(feature = "tls")] +#[derive(Clone)] +pub(crate) struct TLSClientConfig(pub(crate) Arc); + +#[cfg(feature = "tls")] +impl std::fmt::Debug for TLSClientConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("TLSClientConfig").finish() + } } #[cfg(test)] @@ -330,31 +463,4 @@ mod tests { let _agent: Box = Box::new(AgentBuilder::new().build()); let _agent: Box = Box::new(AgentBuilder::new().build()); } - - #[test] - #[cfg(any(feature = "tls", feature = "native-tls"))] - fn agent_pool() { - use std::io::Read; - - let agent = crate::agent(); - let url = "http://example.com"; - // req 1 - let resp = agent.get(url).call().unwrap(); - let mut reader = resp.into_reader(); - let mut buf = vec![]; - // reading the entire content will return the connection to the pool - reader.read_to_end(&mut buf).unwrap(); - - fn poolsize(agent: &Agent) -> usize { - agent.state.pool.len() - } - assert_eq!(poolsize(&agent), 1); - - // req 2 should be done with a reused connection - let resp = agent.get(url).call().unwrap(); - assert_eq!(poolsize(&agent), 0); - let mut reader = resp.into_reader(); - let mut buf = vec![]; - reader.read_to_end(&mut buf).unwrap(); - } } diff --git a/src/cookies.rs b/src/cookies.rs index be6d10b..13982d9 100644 --- a/src/cookies.rs +++ b/src/cookies.rs @@ -1,18 +1,18 @@ -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] use std::sync::RwLock; -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] use cookie_store::CookieStore; -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] use url::Url; -#[cfg(feature = "cookie")] -#[derive(Default, Debug)] +#[cfg(feature = "cookies")] +#[derive(Debug)] pub(crate) struct CookieTin { inner: RwLock, } -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] impl CookieTin { pub(crate) fn new(store: CookieStore) -> Self { CookieTin { diff --git a/src/error.rs b/src/error.rs index c47f399..e47dba6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -32,9 +32,6 @@ pub enum Error { /// Read the inner response body for details and to return /// the connection to the pool. HTTP(Box), - /// TLS Error - #[cfg(feature = "native-tls")] - TlsError(native_tls::Error), } impl Error { @@ -70,8 +67,6 @@ impl fmt::Display for Error { Error::ProxyConnect => write!(f, "Proxy failed to connect"), Error::InvalidProxyCreds => write!(f, "Provided proxy credentials are incorrect"), Error::HTTP(response) => write!(f, "HTTP status {}", response.status()), - #[cfg(feature = "native-tls")] - Error::TlsError(err) => write!(f, "TLS Error: {}", err), } } } diff --git a/src/lib.rs b/src/lib.rs index 7bcd231..d1ebc78 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -130,15 +130,20 @@ pub use crate::resolve::Resolver; pub use crate::response::Response; // re-export -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] pub use cookie::Cookie; #[cfg(feature = "json")] pub use serde_json::{to_value as serde_to_value, Map as SerdeMap, Value as SerdeValue}; +/// Creates an agent builder. +pub fn builder() -> AgentBuilder { + AgentBuilder::new() +} + /// Agents are used to keep state between requests. pub fn agent() -> Agent { #[cfg(not(test))] - return Agent::default(); + return AgentBuilder::new().build(); #[cfg(test)] return test::test_agent(); } @@ -198,7 +203,9 @@ mod tests { #[test] fn connect_http_google() { - let resp = get("http://www.google.com/").call().unwrap(); + let agent = Agent::new(); + + let resp = agent.get("http://www.google.com/").call().unwrap(); assert_eq!( "text/html; charset=ISO-8859-1", resp.header("content-type").unwrap() @@ -207,9 +214,11 @@ mod tests { } #[test] - #[cfg(any(feature = "tls", feature = "native-tls"))] + #[cfg(feature = "tls")] fn connect_https_google() { - let resp = get("https://www.google.com/").call().unwrap(); + let agent = Agent::new(); + + let resp = agent.get("https://www.google.com/").call().unwrap(); assert_eq!( "text/html; charset=ISO-8859-1", resp.header("content-type").unwrap() @@ -218,7 +227,7 @@ mod tests { } #[test] - #[cfg(any(feature = "tls", feature = "native-tls"))] + #[cfg(feature = "tls")] fn connect_https_invalid_name() { let result = get("https://example.com{REQUEST_URI}/").call(); assert!(matches!(result.unwrap_err(), Error::DnsFailed(_))); diff --git a/src/pool.rs b/src/pool.rs index 3023e2b..8d9b62c 100644 --- a/src/pool.rs +++ b/src/pool.rs @@ -9,8 +9,8 @@ use crate::Proxy; use url::Url; -const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 100; -const DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST: usize = 1; +pub const DEFAULT_MAX_IDLE_CONNECTIONS: usize = 100; +pub const DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST: usize = 1; /// Holder of recycled connections. /// @@ -74,25 +74,23 @@ fn remove_last_match(list: &mut VecDeque, key: &PoolKey) -> Option Self { - Self { - max_idle_connections: DEFAULT_MAX_IDLE_CONNECTIONS, - max_idle_connections_per_host: DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST, - inner: Mutex::new(Inner { - recycle: HashMap::default(), - lru: VecDeque::default(), - }), - } - } -} - impl ConnectionPool { - pub(crate) fn new(max_idle_connections: usize, max_idle_connections_per_host: usize) -> Self { + #[cfg(test)] + pub(crate) fn new() -> Self { + Self::new_with_limits( + DEFAULT_MAX_IDLE_CONNECTIONS, + DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST, + ) + } + + pub(crate) fn new_with_limits( + max_idle_connections: usize, + max_idle_connections_per_host: usize, + ) -> Self { ConnectionPool { inner: Mutex::new(Inner { - recycle: HashMap::default(), - lru: VecDeque::default(), + recycle: HashMap::new(), + lru: VecDeque::new(), }), max_idle_connections, max_idle_connections_per_host, @@ -105,7 +103,7 @@ impl ConnectionPool { } /// How the unit::connect tries to get a pooled connection. - pub fn try_get_connection(&self, url: &Url, proxy: &Option) -> Option { + pub fn try_get_connection(&self, url: &Url, proxy: Option) -> Option { let key = PoolKey::new(url, proxy); self.remove(&key) } @@ -211,13 +209,13 @@ impl fmt::Debug for PoolKey { } impl PoolKey { - fn new(url: &Url, proxy: &Option) -> Self { + fn new(url: &Url, proxy: Option) -> Self { let port = url.port_or_known_default(); PoolKey { scheme: url.scheme().to_string(), hostname: url.host_str().unwrap_or("").to_string(), port, - proxy: proxy.clone(), + proxy: proxy, } } } @@ -225,7 +223,7 @@ impl PoolKey { #[test] fn poolkey_new() { // Test that PoolKey::new() does not panic on unrecognized schemes. - PoolKey::new(&Url::parse("zzz:///example.com").unwrap(), &None); + PoolKey::new(&Url::parse("zzz:///example.com").unwrap(), None); } #[test] @@ -233,7 +231,7 @@ fn pool_connections_limit() { // Test inserting connections with different keys into the pool, // filling and draining it. The pool should evict earlier connections // when the connection limit is reached. - let pool = ConnectionPool::default(); + let pool = ConnectionPool::new(); let hostnames = (0..DEFAULT_MAX_IDLE_CONNECTIONS * 2).map(|i| format!("{}.example", i)); let poolkeys = hostnames.map(|hostname| PoolKey { scheme: "https".to_string(), @@ -258,7 +256,7 @@ fn pool_per_host_connections_limit() { // Test inserting connections with the same key into the pool, // filling and draining it. The pool should evict earlier connections // when the per-host connection limit is reached. - let pool = ConnectionPool::default(); + let pool = ConnectionPool::new(); let poolkey = PoolKey { scheme: "https".to_string(), hostname: "example.com".to_string(), @@ -285,17 +283,17 @@ fn pool_per_host_connections_limit() { fn pool_checks_proxy() { // Test inserting different poolkeys with same address but different proxies. // Each insertion should result in an additional entry in the pool. - let pool = ConnectionPool::default(); + let pool = ConnectionPool::new(); let url = Url::parse("zzz:///example.com").unwrap(); pool.add( - PoolKey::new(&url, &None), + PoolKey::new(&url, None), Stream::Cursor(std::io::Cursor::new(vec![])), ); assert_eq!(pool.len(), 1); pool.add( - PoolKey::new(&url, &Some(Proxy::new("localhost:9999").unwrap())), + PoolKey::new(&url, Some(Proxy::new("localhost:9999").unwrap())), Stream::Cursor(std::io::Cursor::new(vec![])), ); assert_eq!(pool.len(), 2); @@ -303,7 +301,7 @@ fn pool_checks_proxy() { pool.add( PoolKey::new( &url, - &Some(Proxy::new("user:password@localhost:9999").unwrap()), + Some(Proxy::new("user:password@localhost:9999").unwrap()), ), Stream::Cursor(std::io::Cursor::new(vec![])), ); @@ -343,7 +341,7 @@ impl> PoolReturnRead { stream.reset()?; // insert back into pool - let key = PoolKey::new(&unit.url, &unit.req.proxy); + let key = PoolKey::new(&unit.url, unit.req.proxy()); unit.req.agent.state.pool.add(key, stream); } diff --git a/src/request.rs b/src/request.rs index a534dea..1812418 100644 --- a/src/request.rs +++ b/src/request.rs @@ -1,13 +1,10 @@ use std::fmt; use std::io::Read; -#[cfg(any(feature = "native-tls", feature = "tls"))] -use std::sync::Arc; -use std::time; use qstring::QString; use url::{form_urlencoded, Url}; -use crate::agent::{self, Agent}; +use crate::agent::Agent; use crate::body::BodySize; use crate::body::{Payload, SizedReader}; use crate::error::Error; @@ -30,28 +27,14 @@ pub type Result = std::result::Result; /// .query("foo", "bar baz") // add ?foo=bar%20baz /// .call(); // run the request /// ``` -#[derive(Clone, Default)] +#[derive(Clone)] pub struct Request { pub(crate) agent: Agent, - - // via agent pub(crate) method: String, url: String, - - // from request itself return_error_for_status: bool, pub(crate) headers: Vec
, pub(crate) query: QString, - pub(crate) timeout_connect: Option, - pub(crate) timeout_read: Option, - pub(crate) timeout_write: Option, - pub(crate) timeout: Option, - pub(crate) redirects: u32, - pub(crate) proxy: Option, - #[cfg(feature = "tls")] - pub(crate) tls_config: Option, - #[cfg(all(feature = "native-tls", not(feature = "tls")))] - pub(crate) tls_connector: Option, } impl fmt::Debug for Request { @@ -72,43 +55,32 @@ impl fmt::Debug for Request { } impl Request { - pub(crate) fn new(agent: &Agent, method: String, url: String) -> Request { + pub(crate) fn new(agent: Agent, method: String, url: String) -> Request { + let headers = agent.headers.clone(); Request { - agent: agent.clone(), + agent, method, url, - headers: agent.headers.clone(), - redirects: 5, + headers, return_error_for_status: true, - ..Default::default() + query: QString::default(), } } - /// "Builds" this request which is effectively the same as cloning. - /// This is needed when we use a chain of request builders, but - /// don't want to send the request at the end of the chain. - /// - /// ``` - /// let r = ureq::get("/my_page") - /// .set("X-Foo-Bar", "Baz") - /// .build(); - /// ``` - pub fn build(&self) -> Request { - self.clone() - } - /// Executes the request and blocks the caller until done. /// /// Use `.timeout_connect()` and `.timeout_read()` to avoid blocking forever. /// /// ``` - /// let r = ureq::get("/my_page") + /// let r = ureq::builder() /// .timeout_connect(std::time::Duration::from_secs(10)) // max 10 seconds + /// .build() + /// .get("/my_page") /// .call(); /// /// println!("{:?}", r); /// ``` - pub fn call(&mut self) -> Result { + pub fn call(self) -> Result { self.do_call(Payload::Empty) } @@ -146,11 +118,12 @@ impl Request { /// } /// ``` #[cfg(feature = "json")] - pub fn send_json(&mut self, data: SerdeValue) -> Result { - if self.header("Content-Type").is_none() { - self.set("Content-Type", "application/json"); + pub fn send_json(self, data: SerdeValue) -> Result { + let mut this = self; + if this.header("Content-Type").is_none() { + this = this.set("Content-Type", "application/json"); } - self.do_call(Payload::JSON(data)) + this.do_call(Payload::JSON(data)) } /// Send data as bytes. @@ -163,7 +136,7 @@ impl Request { /// .send_bytes(body); /// println!("{:?}", r); /// ``` - pub fn send_bytes(&mut self, data: &[u8]) -> Result { + pub fn send_bytes(self, data: &[u8]) -> Result { self.do_call(Payload::Bytes(data.to_owned())) } @@ -188,7 +161,7 @@ impl Request { /// .send_string("Hällo Wörld!"); /// println!("{:?}", r); /// ``` - pub fn send_string(&mut self, data: &str) -> Result { + pub fn send_string(self, data: &str) -> Result { let text = data.into(); let charset = crate::response::charset_from_content_type(self.header("content-type")).to_string(); @@ -210,14 +183,15 @@ impl Request { /// println!("{:?}", r); /// } /// ``` - pub fn send_form(&mut self, data: &[(&str, &str)]) -> Result { - if self.header("Content-Type").is_none() { - self.set("Content-Type", "application/x-www-form-urlencoded"); + pub fn send_form(self, data: &[(&str, &str)]) -> Result { + let mut this = self; + if this.header("Content-Type").is_none() { + this = this.set("Content-Type", "application/x-www-form-urlencoded"); } let encoded = form_urlencoded::Serializer::new(String::new()) .extend_pairs(data) .finish(); - self.do_call(Payload::Bytes(encoded.into_bytes())) + this.do_call(Payload::Bytes(encoded.into_bytes())) } /// Send data from a reader. @@ -238,7 +212,7 @@ impl Request { /// .set("Content-Type", "text/plain") /// .send(read); /// ``` - pub fn send(&mut self, reader: impl Read + 'static) -> Result { + pub fn send(self, reader: impl Read + 'static) -> Result { self.do_call(Payload::Reader(Box::new(reader))) } @@ -256,7 +230,7 @@ impl Request { /// println!("Oh no error!"); /// } /// ``` - pub fn set(&mut self, header: &str, value: &str) -> &mut Request { + pub fn set(mut self, header: &str, value: &str) -> Self { header::add_header(&mut self.headers, Header::new(header, value)); self } @@ -265,8 +239,7 @@ impl Request { /// /// ``` /// let req = ureq::get("/my_page") - /// .set("X-API-Key", "foobar") - /// .build(); + /// .set("X-API-Key", "foobar"); /// assert_eq!("foobar", req.header("x-api-Key").unwrap()); /// ``` pub fn header(&self, name: &str) -> Option<&str> { @@ -278,8 +251,7 @@ impl Request { /// ``` /// let req = ureq::get("/my_page") /// .set("X-API-Key", "foobar") - /// .set("Content-Type", "application/json") - /// .build(); + /// .set("Content-Type", "application/json"); /// assert_eq!(req.header_names(), vec!["x-api-key", "content-type"]); /// ``` pub fn header_names(&self) -> Vec { @@ -293,8 +265,7 @@ impl Request { /// /// ``` /// let req = ureq::get("/my_page") - /// .set("X-API-Key", "foobar") - /// .build(); + /// .set("X-API-Key", "foobar"); /// assert_eq!(true, req.has("x-api-Key")); /// ``` pub fn has(&self, name: &str) -> bool { @@ -306,8 +277,8 @@ impl Request { /// ``` /// let req = ureq::get("/my_page") /// .set("X-Forwarded-For", "1.2.3.4") - /// .set("X-Forwarded-For", "2.3.4.5") - /// .build(); + /// .set("X-Forwarded-For", "2.3.4.5"); + /// /// assert_eq!(req.all("x-forwarded-for"), vec![ /// "1.2.3.4", /// "2.3.4.5", @@ -329,7 +300,7 @@ impl Request { /// /// println!("{:?}", r); /// ``` - pub fn query(&mut self, param: &str, value: &str) -> &mut Request { + pub fn query(mut self, param: &str, value: &str) -> Self { self.query.add_pair((param, value)); self } @@ -344,132 +315,11 @@ impl Request { /// .call(); /// println!("{:?}", r); /// ``` - pub fn query_str(&mut self, query: &str) -> &mut Request { + pub fn query_str(mut self, query: &str) -> Self { self.query.add_str(query); self } - /// Timeout for the socket connection to be successful. - /// If both this and .timeout() are both set, .timeout_connect() - /// takes precedence. - /// - /// The default is `0`, which means a request can block forever. - /// - /// ``` - /// let r = ureq::get("/my_page") - /// .timeout(std::time::Duration::from_secs(1)) - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn timeout_connect(&mut self, timeout: time::Duration) -> &mut Request { - self.timeout_connect = Some(timeout); - self - } - - /// Timeout for the individual reads of the socket. - /// If both this and .timeout() are both set, .timeout() - /// takes precedence. - /// - /// The default is `0`, which means it can block forever. - /// - /// ``` - /// let r = ureq::get("/my_page") - /// .timeout(std::time::Duration::from_secs(1)) - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn timeout_read(&mut self, timeout: time::Duration) -> &mut Request { - self.timeout_read = Some(timeout); - self - } - - /// Timeout for the individual writes to the socket. - /// If both this and .timeout() are both set, .timeout() - /// takes precedence. - /// - /// The default is `0`, which means it can block forever. - /// - /// ``` - /// let r = ureq::get("/my_page") - /// .timeout(std::time::Duration::from_secs(1)) - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn timeout_write(&mut self, timeout: time::Duration) -> &mut Request { - self.timeout_write = Some(timeout); - self - } - - /// Timeout for the overall request, including DNS resolution, connection - /// time, redirects, and reading the response body. Slow DNS resolution - /// may cause a request to exceed the timeout, because the DNS request - /// cannot be interrupted with the available APIs. - /// - /// This takes precedence over .timeout_read() and .timeout_write(), but - /// not .timeout_connect(). - /// - /// ``` - /// // wait max 1 second for whole request to complete. - /// let r = ureq::get("/my_page") - /// .timeout(std::time::Duration::from_secs(1)) - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn timeout(&mut self, timeout: time::Duration) -> &mut Request { - self.timeout = Some(timeout); - self - } - - /// Basic auth. - /// - /// These are the same - /// - /// ``` - /// let r1 = ureq::get("http://localhost/my_page") - /// .auth("martin", "rubbermashgum") - /// .call(); - /// println!("{:?}", r1); - /// - /// let r2 = ureq::get("http://martin:rubbermashgum@localhost/my_page").call(); - /// println!("{:?}", r2); - /// ``` - pub fn auth(&mut self, user: &str, pass: &str) -> &mut Request { - let pass = agent::basic_auth(user, pass); - self.auth_kind("Basic", &pass) - } - - /// Auth of other kinds such as `Digest`, `Token` etc. - /// - /// ``` - /// let r = ureq::get("http://localhost/my_page") - /// .auth_kind("token", "secret") - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn auth_kind(&mut self, kind: &str, pass: &str) -> &mut Request { - let value = format!("{} {}", kind, pass); - self.set("Authorization", &value); - self - } - - /// How many redirects to follow. - /// - /// Defaults to `5`. Set to `0` to avoid redirects and instead - /// get a response object with the 3xx status code. - /// - /// If the redirect count hits this limit (and it's > 0), TooManyRedirects is returned. - /// - /// ``` - /// let r = ureq::get("/my_page") - /// .redirects(10) - /// .call(); - /// println!("{:?}", r); - /// ``` - pub fn redirects(&mut self, n: u32) -> &mut Request { - self.redirects = n; - self - } - /// By default, if a response's status is anything but a 2xx or 3xx, /// send() and related methods will return an Error. If you want /// to handle such responses as non-errors, set this to false. @@ -484,7 +334,7 @@ impl Request { /// # Ok(()) /// # } /// ``` - pub fn error_for_status(&mut self, value: bool) -> &mut Request { + pub fn error_for_status(mut self, value: bool) -> Self { self.return_error_for_status = value; self } @@ -493,8 +343,7 @@ impl Request { /// /// Example: /// ``` - /// let req = ureq::post("/somewhere") - /// .build(); + /// let req = ureq::post("/somewhere"); /// assert_eq!(req.get_method(), "POST"); /// ``` pub fn get_method(&self) -> &str { @@ -508,8 +357,7 @@ impl Request { /// /// Example: /// ``` - /// let req = ureq::post("https://cool.server/innit") - /// .build(); + /// let req = ureq::post("https://cool.server/innit"); /// assert_eq!(req.get_url(), "https://cool.server/innit"); /// ``` pub fn get_url(&self) -> &str { @@ -520,12 +368,10 @@ impl Request { /// /// Example: /// ``` - /// let req1 = ureq::post("https://cool.server/innit") - /// .build(); + /// let req1 = ureq::post("https://cool.server/innit"); /// assert_eq!(req1.get_host().unwrap(), "cool.server"); /// - /// let req2 = ureq::post("http://localhost/some/path") - /// .build(); + /// let req2 = ureq::post("http://localhost/some/path"); /// assert_eq!(req2.get_host().unwrap(), "localhost"); /// ``` pub fn get_host(&self) -> Result { @@ -542,8 +388,7 @@ impl Request { /// /// Example: /// ``` - /// let req = ureq::post("https://cool.server/innit") - /// .build(); + /// let req = ureq::post("https://cool.server/innit"); /// assert_eq!(req.get_scheme().unwrap(), "https"); /// ``` pub fn get_scheme(&self) -> Result { @@ -555,8 +400,7 @@ impl Request { /// Example: /// ``` /// let req = ureq::post("https://cool.server/innit?foo=bar") - /// .query("format", "json") - /// .build(); + /// .query("format", "json"); /// assert_eq!(req.get_query().unwrap(), "?foo=bar&format=json"); /// ``` pub fn get_query(&self) -> Result { @@ -568,8 +412,7 @@ impl Request { /// /// Example: /// ``` - /// let req = ureq::post("https://cool.server/innit") - /// .build(); + /// let req = ureq::post("https://cool.server/innit"); /// assert_eq!(req.get_path().unwrap(), "/innit"); /// ``` pub fn get_path(&self) -> Result { @@ -580,63 +423,14 @@ impl Request { Url::parse(&self.url).map_err(|e| Error::BadUrl(format!("{}", e))) } - /// Set the proxy server to use for the connection. - /// - /// Example: - /// ``` - /// let proxy = ureq::Proxy::new("user:password@cool.proxy:9090").unwrap(); - /// let req = ureq::post("https://cool.server") - /// .set_proxy(proxy) - /// .build(); - /// ``` - pub fn set_proxy(&mut self, proxy: Proxy) -> &mut Request { - self.proxy = Some(proxy); - self - } - pub(crate) fn proxy(&self) -> Option { - if let Some(proxy) = &self.proxy { - Some(proxy.clone()) - } else if let Some(proxy) = &self.agent.state.proxy { + if let Some(proxy) = &self.agent.state.proxy { Some(proxy.clone()) } else { None } } - /// Set the TLS client config to use for the connection. See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). - /// - /// See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). - /// - /// Example: - /// ``` - /// let tls_config = std::sync::Arc::new(rustls::ClientConfig::new()); - /// let req = ureq::post("https://cool.server") - /// .set_tls_config(tls_config.clone()); - /// ``` - #[cfg(feature = "tls")] - pub fn set_tls_config(&mut self, tls_config: Arc) -> &mut Request { - self.tls_config = Some(TLSClientConfig(tls_config)); - self - } - - /// Sets the TLS connector that will be used for the connection. - /// - /// Example: - /// ``` - /// let tls_connector = std::sync::Arc::new(native_tls::TlsConnector::new().unwrap()); - /// let req = ureq::post("https://cool.server") - /// .set_tls_connector(tls_connector.clone()); - /// ``` - #[cfg(all(feature = "native-tls", not(feature = "tls")))] - pub fn set_tls_connector( - &mut self, - tls_connector: Arc, - ) -> &mut Request { - self.tls_connector = Some(TLSConnector(tls_connector)); - self - } - // Returns true if this request, with the provided body, is retryable. pub(crate) fn is_retryable(&self, body: &SizedReader) -> bool { // Per https://tools.ietf.org/html/rfc7231#section-8.1.3 @@ -660,32 +454,10 @@ impl Request { } } -#[cfg(feature = "tls")] -#[derive(Clone)] -pub(crate) struct TLSClientConfig(pub(crate) Arc); - -#[cfg(feature = "tls")] -impl fmt::Debug for TLSClientConfig { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("TLSClientConfig").finish() - } -} - -#[cfg(all(feature = "native-tls", not(feature = "tls")))] -#[derive(Clone)] -pub(crate) struct TLSConnector(pub(crate) Arc); - -#[cfg(all(feature = "native-tls", not(feature = "tls")))] -impl fmt::Debug for TLSConnector { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("TLSConnector").finish() - } -} - #[test] fn no_hostname() { let req = Request::new( - &Agent::default(), + Agent::new(), "GET".to_string(), "unix:/run/foo.socket".to_string(), ); @@ -695,12 +467,12 @@ fn no_hostname() { #[test] fn request_implements_send_and_sync() { let _request: Box = Box::new(Request::new( - &Agent::default(), + Agent::new(), "GET".to_string(), "https://example.com/".to_string(), )); let _request: Box = Box::new(Request::new( - &Agent::default(), + Agent::new(), "GET".to_string(), "https://example.com/".to_string(), )); diff --git a/src/resolve.rs b/src/resolve.rs index ca6497d..fc32f99 100644 --- a/src/resolve.rs +++ b/src/resolve.rs @@ -51,9 +51,3 @@ impl std::ops::Deref for ArcResolver { self.0.as_ref() } } - -impl Default for ArcResolver { - fn default() -> Self { - StdResolver.into() - } -} diff --git a/src/response.rs b/src/response.rs index 1101c17..53c8ed6 100644 --- a/src/response.rs +++ b/src/response.rs @@ -189,7 +189,7 @@ impl Response { .unwrap_or(DEFAULT_CONTENT_TYPE) } - /// The character set part of the "Content-Type" header.native_tls + /// The character set part of the "Content-Type". /// /// Example: /// diff --git a/src/stream.rs b/src/stream.rs index 3c5e4d8..dc775f6 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -15,9 +15,6 @@ use rustls::StreamOwned; #[cfg(feature = "socks-proxy")] use socks::{TargetAddr, ToTargetAddr}; -#[cfg(feature = "native-tls")] -use native_tls::{HandshakeError, TlsStream}; - use crate::proxy::Proto; use crate::proxy::Proxy; @@ -27,10 +24,8 @@ use crate::unit::Unit; #[allow(clippy::large_enum_variant)] pub enum Stream { Http(BufReader), - #[cfg(all(feature = "tls", not(feature = "native-tls")))] + #[cfg(feature = "tls")] Https(BufReader>), - #[cfg(all(feature = "native-tls", not(feature = "tls")))] - Https(BufReader>), Cursor(Cursor>), #[cfg(test)] Test(Box, Vec), @@ -101,10 +96,7 @@ impl fmt::Debug for Stream { "Stream[{}]", match self { Stream::Http(_) => "http", - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(_) => "https", Stream::Cursor(_) => "cursor", #[cfg(test)] @@ -144,10 +136,7 @@ impl Stream { pub fn is_poolable(&self) -> bool { match self { Stream::Http(_) => true, - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(_) => true, _ => false, } @@ -186,10 +175,7 @@ impl Read for Stream { fn read(&mut self, buf: &mut [u8]) -> io::Result { match self { Stream::Http(sock) => sock.read(buf), - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(stream) => read_https(stream, buf), Stream::Cursor(read) => read.read(buf), #[cfg(test)] @@ -202,10 +188,7 @@ impl BufRead for Stream { fn fill_buf(&mut self) -> io::Result<&[u8]> { match self { Stream::Http(r) => r.fill_buf(), - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(r) => r.fill_buf(), Stream::Cursor(r) => r.fill_buf(), #[cfg(test)] @@ -216,10 +199,7 @@ impl BufRead for Stream { fn consume(&mut self, amt: usize) { match self { Stream::Http(r) => r.consume(amt), - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(r) => r.consume(amt), Stream::Cursor(r) => r.consume(amt), #[cfg(test)] @@ -238,7 +218,7 @@ where } } -#[cfg(all(feature = "tls", not(feature = "native-tls")))] +#[cfg(feature = "tls")] fn read_https( stream: &mut BufReader>, buf: &mut [u8], @@ -250,17 +230,8 @@ fn read_https( } } -#[cfg(all(feature = "native-tls", not(feature = "tls")))] -fn read_https(stream: &mut BufReader>, buf: &mut [u8]) -> io::Result { - match stream.read(buf) { - Ok(size) => Ok(size), - Err(ref e) if is_close_notify(e) => Ok(0), - Err(e) => Err(e), - } -} - #[allow(deprecated)] -#[cfg(any(feature = "tls", feature = "native-tls"))] +#[cfg(feature = "tls")] fn is_close_notify(e: &std::io::Error) -> bool { if e.kind() != ErrorKind::ConnectionAborted { return false; @@ -279,10 +250,7 @@ impl Write for Stream { fn write(&mut self, buf: &[u8]) -> io::Result { match self { Stream::Http(sock) => sock.get_mut().write(buf), - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(stream) => stream.get_mut().write(buf), Stream::Cursor(_) => panic!("Write to read only stream"), #[cfg(test)] @@ -292,10 +260,7 @@ impl Write for Stream { fn flush(&mut self) -> io::Result<()> { match self { Stream::Http(sock) => sock.get_mut().flush(), - #[cfg(any( - all(feature = "tls", not(feature = "native-tls")), - all(feature = "native-tls", not(feature = "tls")), - ))] + #[cfg(feature = "tls")] Stream::Https(stream) => stream.get_mut().flush(), Stream::Cursor(_) => panic!("Flush read only stream"), #[cfg(test)] @@ -326,7 +291,7 @@ fn configure_certs(config: &mut rustls::ClientConfig) { .add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS); } -#[cfg(all(feature = "tls", not(feature = "native-tls")))] +#[cfg(feature = "tls")] pub(crate) fn connect_https(unit: &Unit, hostname: &str) -> Result { use once_cell::sync::Lazy; use std::sync::Arc; @@ -343,6 +308,8 @@ pub(crate) fn connect_https(unit: &Unit, hostname: &str) -> Result = unit .req + .agent + .config .tls_config .as_ref() .map(|c| &c.0) @@ -356,35 +323,13 @@ pub(crate) fn connect_https(unit: &Unit, hostname: &str) -> Result Result { - use std::sync::Arc; - - let port = unit.url.port().unwrap_or(443); - let sock = connect_host(unit, hostname, port)?; - - let tls_connector: Arc = match &unit.req.tls_connector { - Some(connector) => connector.0.clone(), - None => Arc::new(native_tls::TlsConnector::new().map_err(|e| Error::TlsError(e))?), - }; - let stream = tls_connector - .connect(&hostname.trim_matches(|c| c == '[' || c == ']'), sock) - .map_err(|e| match e { - HandshakeError::Failure(err) => Error::TlsError(err), - // The only other possibility is WouldBlock. Since we don't - // handle retries of WouldBlock, turn it into a generic error. - _ => Error::ConnectionFailed("TLS handshake unexpected error".to_string()), - })?; - - Ok(Stream::Https(BufReader::new(stream))) -} - pub(crate) fn connect_host(unit: &Unit, hostname: &str, port: u16) -> Result { - let deadline: Option = if let Some(timeout_connect) = unit.req.timeout_connect { - Instant::now().checked_add(timeout_connect) - } else { - unit.deadline - }; + let deadline: Option = + if let Some(timeout_connect) = unit.req.agent.config.timeout_connect { + Instant::now().checked_add(timeout_connect) + } else { + unit.deadline + }; let proxy: Option = unit.req.proxy(); let netloc = match proxy { Some(ref proxy) => format!("{}:{}", proxy.server, proxy.port), @@ -451,7 +396,7 @@ pub(crate) fn connect_host(unit: &Unit, hostname: &str, port: u16) -> Result Result Result { Err(Error::UnknownScheme(unit.url.scheme().to_string())) } -#[cfg(not(any(feature = "tls", feature = "native-tls")))] +#[cfg(not(feature = "tls"))] pub(crate) fn connect_https(unit: &Unit, _hostname: &str) -> Result { Err(Error::UnknownScheme(unit.url.scheme().to_string())) } diff --git a/src/test/agent_test.rs b/src/test/agent_test.rs index 3a510c5..c98faad 100644 --- a/src/test/agent_test.rs +++ b/src/test/agent_test.rs @@ -10,9 +10,7 @@ use super::super::*; #[test] fn agent_reuse_headers() { - let agent = AgentBuilder::new() - .set("Authorization", "Foo 12345") - .build(); + let agent = builder().set("Authorization", "Foo 12345").build(); test::set_handler("/agent_reuse_headers", |unit| { assert!(unit.has("Authorization")); @@ -46,7 +44,7 @@ fn idle_timeout_handler(mut stream: TcpStream) -> io::Result<()> { fn connection_reuse() { let testserver = TestServer::new(idle_timeout_handler); let url = format!("http://localhost:{}", testserver.port); - let agent = Agent::default(); + let agent = Agent::new(); let resp = agent.get(&url).call().unwrap(); // use up the connection so it gets returned to the pool @@ -96,7 +94,7 @@ fn custom_resolver() { assert_eq!(&server.join().unwrap(), b"GET / HTTP/1.1\r\n"); } -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] #[cfg(test)] fn cookie_and_redirect(mut stream: TcpStream) -> io::Result<()> { let headers = read_headers(&stream); @@ -139,14 +137,14 @@ fn cookie_and_redirect(mut stream: TcpStream) -> io::Result<()> { Ok(()) } -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] #[test] fn test_cookies_on_redirect() -> Result<(), Error> { let testserver = TestServer::new(cookie_and_redirect); let url = format!("http://localhost:{}/first", testserver.port); - let agent = Agent::default(); + let agent = Agent::new(); agent.post(&url).call()?; - let cookies = agent.state.jar.get_request_cookies( + let cookies = agent.state.cookie_tin.get_request_cookies( &format!("https://localhost:{}/", testserver.port) .parse() .unwrap(), @@ -166,17 +164,17 @@ fn dirty_streams_not_returned() -> Result<(), Error> { 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"9\r\n")?; + stream.write_all(b"dachshund\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(); + let agent = Agent::new(); let resp = agent.get(&url).call()?; let resp_str = resp.into_string()?; - assert_eq!(resp_str, "corgidachsund"); + assert_eq!(resp_str, "corgidachshund"); // Now fetch it again, but only read part of the body. let resp_to_be_dropped = agent.get(&url).call()?; diff --git a/src/test/auth.rs b/src/test/auth.rs deleted file mode 100644 index 677e165..0000000 --- a/src/test/auth.rs +++ /dev/null @@ -1,64 +0,0 @@ -use crate::test; - -use super::super::*; - -#[test] -fn basic_auth() { - test::set_handler("/basic_auth", |unit| { - assert_eq!( - unit.header("Authorization").unwrap(), - "Basic bWFydGluOnJ1YmJlcm1hc2hndW0=" - ); - test::make_response(200, "OK", vec![], vec![]) - }); - let resp = get("test://host/basic_auth") - .auth("martin", "rubbermashgum") - .call() - .unwrap(); - assert_eq!(resp.status(), 200); -} - -#[test] -fn kind_auth() { - test::set_handler("/kind_auth", |unit| { - assert_eq!(unit.header("Authorization").unwrap(), "Digest abcdefgh123"); - test::make_response(200, "OK", vec![], vec![]) - }); - let resp = get("test://host/kind_auth") - .auth_kind("Digest", "abcdefgh123") - .call() - .unwrap(); - assert_eq!(resp.status(), 200); -} - -#[test] -fn url_auth() { - test::set_handler("/url_auth", |unit| { - assert_eq!( - unit.header("Authorization").unwrap(), - "Basic QWxhZGRpbjpPcGVuU2VzYW1l" - ); - test::make_response(200, "OK", vec![], vec![]) - }); - let resp = get("test://Aladdin:OpenSesame@host/url_auth") - .call() - .unwrap(); - assert_eq!(resp.status(), 200); -} - -#[test] -fn url_auth_overridden() { - test::set_handler("/url_auth_overridden", |unit| { - assert_eq!( - unit.header("Authorization").unwrap(), - "Basic bWFydGluOnJ1YmJlcm1hc2hndW0=" - ); - test::make_response(200, "OK", vec![], vec![]) - }); - let agent = AgentBuilder::new().auth("martin", "rubbermashgum").build(); - let resp = agent - .get("test://Aladdin:OpenSesame@host/url_auth_overridden") - .call() - .unwrap(); - assert_eq!(resp.status(), 200); -} diff --git a/src/test/mod.rs b/src/test/mod.rs index 98f507f..72506a9 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -7,7 +7,6 @@ use std::sync::{Arc, Mutex}; use std::{collections::HashMap, net::ToSocketAddrs}; mod agent_test; -mod auth; mod body_read; mod body_send; mod query_string; diff --git a/src/test/range.rs b/src/test/range.rs index 3d4a4c9..982b770 100644 --- a/src/test/range.rs +++ b/src/test/range.rs @@ -1,11 +1,11 @@ -#[cfg(any(feature = "tls", feature = "native-tls"))] +#[cfg(feature = "tls")] use std::io::Read; -#[cfg(any(feature = "tls", feature = "native-tls"))] +#[cfg(feature = "tls")] use super::super::*; #[test] -#[cfg(any(feature = "tls", feature = "native-tls"))] +#[cfg(feature = "tls")] fn read_range() { let resp = get("https://ureq.s3.eu-central-1.amazonaws.com/sherlock.txt") .set("Range", "bytes=1000-1999") diff --git a/src/test/redirect.rs b/src/test/redirect.rs index 649625c..0c4e4f5 100644 --- a/src/test/redirect.rs +++ b/src/test/redirect.rs @@ -29,7 +29,11 @@ fn redirect_many() { test::set_handler("/redirect_many2", |_| { test::make_response(302, "Go here", vec!["Location: /redirect_many3"], vec![]) }); - let result = get("test://host/redirect_many1").redirects(1).call(); + let result = builder() + .redirects(1) + .build() + .get("test://host/redirect_many1") + .call(); assert!(matches!(result, Err(Error::TooManyRedirects))); } @@ -38,7 +42,11 @@ fn redirect_off() -> Result<(), Error> { test::set_handler("/redirect_off", |_| { test::make_response(302, "Go here", vec!["Location: somewhere.else"], vec![]) }); - let resp = get("test://host/redirect_off").redirects(0).call()?; + let resp = builder() + .redirects(0) + .build() + .get("test://host/redirect_off") + .call()?; assert_eq!(resp.status(), 302); assert!(resp.has("Location")); assert_eq!(resp.header("Location").unwrap(), "somewhere.else"); @@ -96,7 +104,7 @@ fn redirect_host() { Ok(()) }); let url = format!("http://localhost:{}/", srv.port); - let resp = crate::Agent::default().get(&url).call(); + let resp = crate::Agent::new().get(&url).call(); let err = resp.err(); assert!( matches!(err, Some(Error::DnsFailed(_))), diff --git a/src/test/simple.rs b/src/test/simple.rs index 7be9d1b..2e1dda1 100644 --- a/src/test/simple.rs +++ b/src/test/simple.rs @@ -130,8 +130,7 @@ fn request_debug() { let req = get("http://localhost/my/page") .set("Authorization", "abcdef") .set("Content-Length", "1234") - .set("Content-Type", "application/json") - .build(); + .set("Content-Type", "application/json"); let s = format!("{:?}", req); @@ -143,8 +142,7 @@ fn request_debug() { let req = get("http://localhost/my/page?q=z") .query("foo", "bar baz") - .set("Authorization", "abcdef") - .build(); + .set("Authorization", "abcdef"); let s = format!("{:?}", req); diff --git a/src/test/testserver.rs b/src/test/testserver.rs index 2b764d6..720bec6 100644 --- a/src/test/testserver.rs +++ b/src/test/testserver.rs @@ -13,7 +13,7 @@ pub struct TestHeaders(Vec); impl TestHeaders { // Return the path for a request, e.g. /foo from "GET /foo HTTP/1.1" - #[cfg(feature = "cookie")] + #[cfg(feature = "cookies")] pub fn path(&self) -> &str { if self.0.len() == 0 { "" @@ -22,7 +22,7 @@ impl TestHeaders { } } - #[cfg(feature = "cookie")] + #[cfg(feature = "cookies")] pub fn headers(&self) -> &[String] { &self.0[1..] } @@ -57,12 +57,12 @@ impl TestServer { eprintln!("testserver: handling just-accepted stream: {}", e); break; } - thread::spawn(move || handler(stream.unwrap())); - if done.load(Ordering::Relaxed) { + if done.load(Ordering::SeqCst) { break; + } else { + thread::spawn(move || handler(stream.unwrap())); } } - println!("testserver on {} exiting", port); }); TestServer { port, @@ -73,7 +73,7 @@ impl TestServer { impl Drop for TestServer { fn drop(&mut self) { - self.done.store(true, Ordering::Relaxed); + self.done.store(true, Ordering::SeqCst); // 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 edd129f..6edd0bf 100644 --- a/src/test/timeout.rs +++ b/src/test/timeout.rs @@ -25,9 +25,9 @@ fn dribble_body_respond(mut stream: TcpStream, contents: &[u8]) -> io::Result<() } fn get_and_expect_timeout(url: String) { - let agent = Agent::default(); let timeout = Duration::from_millis(500); - let resp = agent.get(&url).timeout(timeout).call().unwrap(); + let agent = builder().timeout(timeout).build(); + let resp = agent.get(&url).call().unwrap(); match resp.into_string() { Err(io_error) => match io_error.kind() { @@ -86,9 +86,9 @@ fn overall_timeout_reading_json() { }); let url = format!("http://localhost:{}/", server.port); - let agent = Agent::default(); let timeout = Duration::from_millis(500); - let resp = agent.get(&url).timeout(timeout).call().unwrap(); + let agent = builder().timeout(timeout).build(); + let resp = agent.get(&url).call().unwrap(); match resp.into_json() { Ok(_) => Err("successful response".to_string()), diff --git a/src/unit.rs b/src/unit.rs index 8e95a8a..207fcd1 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -5,14 +5,14 @@ use log::{debug, info}; use qstring::QString; use url::Url; -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] use cookie::Cookie; use crate::body::{self, BodySize, Payload, SizedReader}; use crate::header; use crate::resolve::ArcResolver; use crate::stream::{self, connect_test, Stream}; -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] use crate::Agent; use crate::{Error, Header, Request, Response}; @@ -81,7 +81,7 @@ impl Unit { extra.push(Header::new("Authorization", &format!("Basic {}", encoded))); } - #[cfg(feature = "cookie")] + #[cfg(feature = "cookies")] extra.extend(extract_cookies(&req.agent, &url).into_iter()); extra @@ -94,7 +94,7 @@ impl Unit { .cloned() .collect(); - let deadline = match req.timeout { + let deadline = match req.agent.config.timeout { None => None, Some(timeout) => { let now = time::Instant::now(); @@ -203,12 +203,12 @@ pub(crate) fn connect( }; // squirrel away cookies - #[cfg(feature = "cookie")] + #[cfg(feature = "cookies")] save_cookies(&unit, &resp); // handle redirects - if resp.redirect() && req.redirects > 0 { - if redirect_count == req.redirects { + if resp.redirect() && req.agent.config.redirects > 0 { + if redirect_count == req.agent.config.redirects { return Err(Error::TooManyRedirects); } @@ -255,11 +255,11 @@ pub(crate) fn connect( Ok(resp) } -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] fn extract_cookies(agent: &Agent, url: &Url) -> Option
{ let header_value = agent .state - .jar + .cookie_tin .get_request_cookies(url) .iter() .map(|c| c.encoded().to_string()) @@ -295,7 +295,7 @@ fn connect_socket(unit: &Unit, hostname: &str, use_pooled: bool) -> Result<(Stre while let Some(stream) = agent .state .pool - .try_get_connection(&unit.url, &unit.req.proxy) + .try_get_connection(&unit.url, unit.req.agent.config.proxy.clone()) { let server_closed = stream.server_closed()?; if !server_closed { @@ -379,7 +379,7 @@ fn send_prelude(unit: &Unit, stream: &mut Stream, redir: bool) -> io::Result<()> } /// Investigate a response for "Set-Cookie" headers. -#[cfg(feature = "cookie")] +#[cfg(feature = "cookies")] fn save_cookies(unit: &Unit, resp: &Response) { // @@ -397,7 +397,7 @@ fn save_cookies(unit: &Unit, resp: &Response) { unit.req .agent .state - .jar + .cookie_tin .store_response_cookies(cookies, &unit.url.clone()); } @@ -411,13 +411,13 @@ mod tests { #[test] fn match_cookies_returns_one_header() { - let agent = Agent::default(); + let agent = Agent::new(); let url: Url = "https://crates.io/".parse().unwrap(); let cookie1: Cookie = "cookie1=value1; Domain=crates.io; Path=/".parse().unwrap(); let cookie2: Cookie = "cookie2=value2; Domain=crates.io; Path=/".parse().unwrap(); agent .state - .jar + .cookie_tin .store_response_cookies(vec![cookie1, cookie2].into_iter(), &url); // There's no guarantee to the order in which cookies are defined. diff --git a/test.sh b/test.sh index 8f1645d..ed7db80 100755 --- a/test.sh +++ b/test.sh @@ -4,7 +4,7 @@ set -eu export RUST_BACKTRACE=1 export RUSTFLAGS="-D dead_code -D unused-variables -D unused" -for tls in "" tls native-tls ; do +for tls in "" tls ; do for feature in "" json charset cookies socks-proxy ; do for what in --doc --tests ; do if ! cargo test "${what}" --no-default-features --features "${tls} ${feature}" ; then diff --git a/tests/https-agent.rs b/tests/https-agent.rs index e23f3c9..081df17 100644 --- a/tests/https-agent.rs +++ b/tests/https-agent.rs @@ -11,12 +11,12 @@ fn agent_set_cookie() { headers: HashMap, } - let agent = ureq::Agent::default().build(); + let agent = ureq::Agent::new(); let cookie = ureq::Cookie::build("name", "value") .domain("httpbin.org") .secure(true) .finish(); - agent.set_cookie(cookie); + agent.set_cookie(cookie, &"https://httpbin.org/".parse().unwrap()); let resp = agent .get("https://httpbin.org/get") .set("Connection", "close") @@ -102,8 +102,6 @@ m0Wqhhi8/24Sy934t5Txgkfoltg8ahkx934WjP6WWRnSAu+cf+vW #[cfg(feature = "tls")] #[test] fn tls_client_certificate() { - let agent = ureq::Agent::default(); - let mut tls_config = rustls::ClientConfig::new(); let certs = rustls::internal::pemfile::certs(&mut BADSSL_CLIENT_CERT_PEM.as_bytes()).unwrap(); @@ -116,11 +114,11 @@ fn tls_client_certificate() { .root_store .add_server_trust_anchors(&webpki_roots::TLS_SERVER_ROOTS); - let resp = agent - .get("https://client.badssl.com/") + let agent = ureq::builder() .set_tls_config(std::sync::Arc::new(tls_config)) - .call() - .unwrap(); + .build(); + + let resp = agent.get("https://client.badssl.com/").call().unwrap(); assert_eq!(resp.status(), 200); } From c8cd130770493e292ea3e06410666a7cca79320c Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Sun, 25 Oct 2020 21:51:04 +0100 Subject: [PATCH 09/11] Make Request::send more general. Removes `+ 'static` constraint from the `impl Read` parameter. For this, lifetime parameters are added to `Payload` and `SizedReader`. --- src/body.rs | 24 ++++++++++++------------ src/request.rs | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/body.rs b/src/body.rs index 1a99792..474977d 100644 --- a/src/body.rs +++ b/src/body.rs @@ -15,16 +15,16 @@ use super::SerdeValue; /// The different kinds of bodies to send. /// /// *Internal API* -pub(crate) enum Payload { +pub(crate) enum Payload<'a> { Empty, Text(String, String), #[cfg(feature = "json")] JSON(SerdeValue), - Reader(Box), + Reader(Box), Bytes(Vec), } -impl fmt::Debug for Payload { +impl fmt::Debug for Payload<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { Payload::Empty => write!(f, "Empty"), @@ -37,8 +37,8 @@ impl fmt::Debug for Payload { } } -impl Default for Payload { - fn default() -> Payload { +impl Default for Payload<'_> { + fn default() -> Self { Payload::Empty } } @@ -56,25 +56,25 @@ pub(crate) enum BodySize { /// Payloads are turned into this type where we can hold both a size and the reader. /// /// *Internal API* -pub(crate) struct SizedReader { +pub(crate) struct SizedReader<'a> { pub size: BodySize, - pub reader: Box, + pub reader: Box, } -impl fmt::Debug for SizedReader { +impl fmt::Debug for SizedReader<'_> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "SizedReader[size={:?},reader]", self.size) } } -impl SizedReader { - fn new(size: BodySize, reader: Box) -> Self { +impl<'a> SizedReader<'a> { + fn new(size: BodySize, reader: Box) -> Self { SizedReader { size, reader } } } -impl Payload { - pub fn into_read(self) -> SizedReader { +impl<'a> Payload<'a> { + pub fn into_read(self) -> SizedReader<'a> { match self { Payload::Empty => SizedReader::new(BodySize::Empty, Box::new(empty())), Payload::Text(text, _charset) => { diff --git a/src/request.rs b/src/request.rs index 7d469fb..152687e 100644 --- a/src/request.rs +++ b/src/request.rs @@ -227,7 +227,7 @@ impl Request { /// .set("Content-Type", "text/plain") /// .send(read); /// ``` - pub fn send(&mut self, reader: impl Read + 'static) -> Response { + pub fn send(&mut self, reader: impl Read) -> Response { self.do_call(Payload::Reader(Box::new(reader))) } From 72e7e06334627710329e5bbdf9fe3d5c99507656 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 25 Oct 2020 15:02:57 -0700 Subject: [PATCH 10/11] Tweak import paths Group together all cookie imports in agent.rs; consistently use `Duration`. --- examples/smoke-test/main.rs | 2 +- src/agent.rs | 30 +++++++++++------------------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/examples/smoke-test/main.rs b/examples/smoke-test/main.rs index 3d9018d..b2082f9 100644 --- a/examples/smoke-test/main.rs +++ b/examples/smoke-test/main.rs @@ -58,7 +58,7 @@ fn get_and_write(agent: &ureq::Agent, url: &String) -> Result<()> { fn get_many(urls: Vec, simultaneous_fetches: usize) -> Result<()> { let agent = ureq::builder() - .timeout_connect(std::time::Duration::from_secs(5)) + .timeout_connect(Duration::from_secs(5)) .timeout(Duration::from_secs(20)) .build(); let pool = rayon::ThreadPoolBuilder::new() diff --git a/src/agent.rs b/src/agent.rs index 7efc798..ad1ea0c 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -5,16 +5,10 @@ use crate::pool::ConnectionPool; use crate::proxy::Proxy; use crate::request::Request; use crate::resolve::{ArcResolver, StdResolver}; -use std::time; +use std::time::Duration; #[cfg(feature = "cookies")] -use crate::cookies::CookieTin; -#[cfg(feature = "cookies")] -use cookie::Cookie; -#[cfg(feature = "cookies")] -use cookie_store::CookieStore; -#[cfg(feature = "cookies")] -use url::Url; +use {crate::cookies::CookieTin, cookie::Cookie, cookie_store::CookieStore, url::Url}; #[derive(Debug)] pub struct AgentBuilder { @@ -33,10 +27,10 @@ pub(crate) struct AgentConfig { pub max_idle_connections: usize, pub max_idle_connections_per_host: usize, pub proxy: Option, - pub timeout_connect: Option, - pub timeout_read: Option, - pub timeout_write: Option, - pub timeout: Option, + pub timeout_connect: Option, + pub timeout_read: Option, + pub timeout_write: Option, + pub timeout: Option, pub redirects: u32, #[cfg(feature = "tls")] pub tls_config: Option, @@ -207,7 +201,7 @@ impl AgentBuilder { max_idle_connections: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS, max_idle_connections_per_host: crate::pool::DEFAULT_MAX_IDLE_CONNECTIONS_PER_HOST, proxy: None, - timeout_connect: Some(time::Duration::from_secs(30)), + timeout_connect: Some(Duration::from_secs(30)), timeout_read: None, timeout_write: None, timeout: None, @@ -343,7 +337,7 @@ impl AgentBuilder { /// .build(); /// let r = agent.get("/my_page").call(); /// ``` - pub fn timeout_connect(mut self, timeout: time::Duration) -> Self { + pub fn timeout_connect(mut self, timeout: Duration) -> Self { self.config.timeout_connect = Some(timeout); self } @@ -360,7 +354,7 @@ impl AgentBuilder { /// .build(); /// let r = agent.get("/my_page").call(); /// ``` - pub fn timeout_read(mut self, timeout: time::Duration) -> Self { + pub fn timeout_read(mut self, timeout: Duration) -> Self { self.config.timeout_read = Some(timeout); self } @@ -377,7 +371,7 @@ impl AgentBuilder { /// .build(); /// let r = agent.get("/my_page").call(); /// ``` - pub fn timeout_write(mut self, timeout: time::Duration) -> Self { + pub fn timeout_write(mut self, timeout: Duration) -> Self { self.config.timeout_write = Some(timeout); self } @@ -397,7 +391,7 @@ impl AgentBuilder { /// .build(); /// let r = agent.get("/my_page").call(); /// ``` - pub fn timeout(mut self, timeout: time::Duration) -> Self { + pub fn timeout(mut self, timeout: Duration) -> Self { self.config.timeout = Some(timeout); self } @@ -424,8 +418,6 @@ impl AgentBuilder { /// Set the TLS client config to use for the connection. See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). /// - /// See [`ClientConfig`](https://docs.rs/rustls/latest/rustls/struct.ClientConfig.html). - /// /// Example: /// ``` /// let tls_config = std::sync::Arc::new(rustls::ClientConfig::new()); From 17ab5110a34ccc8add0bd6a1cfd7f137187c8da9 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sun, 25 Oct 2020 15:52:02 -0700 Subject: [PATCH 11/11] Use lifetimes for more elements of Payload Text and Bytes can both have their lifetimes parameterized. --- src/body.rs | 6 +++--- src/request.rs | 13 +++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/body.rs b/src/body.rs index 474977d..8a48d32 100644 --- a/src/body.rs +++ b/src/body.rs @@ -17,11 +17,11 @@ use super::SerdeValue; /// *Internal API* pub(crate) enum Payload<'a> { Empty, - Text(String, String), + Text(&'a str, String), #[cfg(feature = "json")] JSON(SerdeValue), Reader(Box), - Bytes(Vec), + Bytes(&'a [u8]), } impl fmt::Debug for Payload<'_> { @@ -86,7 +86,7 @@ impl<'a> Payload<'a> { encoding.encode(&text, EncoderTrap::Replace).unwrap() }; #[cfg(not(feature = "charset"))] - let bytes = text.into_bytes(); + let bytes = text.as_bytes(); let len = bytes.len(); let cursor = Cursor::new(bytes); SizedReader::new(BodySize::Known(len as u64), Box::new(cursor)) diff --git a/src/request.rs b/src/request.rs index 152687e..a8976ae 100644 --- a/src/request.rs +++ b/src/request.rs @@ -153,7 +153,7 @@ impl Request { /// println!("{:?}", r); /// ``` pub fn send_bytes(&mut self, data: &[u8]) -> Response { - self.do_call(Payload::Bytes(data.to_owned())) + self.do_call(Payload::Bytes(data)) } /// Send data as a string. @@ -178,10 +178,9 @@ impl Request { /// println!("{:?}", r); /// ``` pub fn send_string(&mut self, data: &str) -> Response { - let text = data.into(); let charset = crate::response::charset_from_content_type(self.header("content-type")).to_string(); - self.do_call(Payload::Text(text, charset)) + self.do_call(Payload::Text(data, charset)) } /// Send a sequence of (key, value) pairs as form-urlencoded data. @@ -206,7 +205,7 @@ impl Request { let encoded = form_urlencoded::Serializer::new(String::new()) .extend_pairs(data) .finish(); - self.do_call(Payload::Bytes(encoded.into_bytes())) + self.do_call(Payload::Bytes(&encoded.into_bytes())) } /// Send data from a reader. @@ -668,3 +667,9 @@ fn no_hostname() { ); assert!(req.get_host().is_err()); } + +#[test] +fn send_byte_slice() { + let bytes = vec![1, 2, 3]; + crate::agent().post("http://example.com").send(&bytes[1..2]); +}