diff --git a/awc/CHANGES.md b/awc/CHANGES.md index a4948c827..8a3fea46a 100644 --- a/awc/CHANGES.md +++ b/awc/CHANGES.md @@ -1,6 +1,9 @@ # Changes ## Unreleased - 2021-xx-xx +* Add `ClientBuilder::add_default_header` and deprecate `ClientBuilder::header`. [#2510] + +[#2510]: https://github.com/actix/actix-web/pull/2510 ## 3.0.0-beta.13 - 2021-12-11 diff --git a/awc/src/builder.rs b/awc/src/builder.rs index e9c1319fd..09f8063bb 100644 --- a/awc/src/builder.rs +++ b/awc/src/builder.rs @@ -2,7 +2,7 @@ use std::{convert::TryFrom, fmt, net::IpAddr, rc::Rc, time::Duration}; use actix_http::{ error::HttpError, - header::{self, HeaderMap, HeaderName}, + header::{self, HeaderMap, HeaderName, TryIntoHeaderPair}, Uri, }; use actix_rt::net::{ActixStream, TcpStream}; @@ -21,11 +21,11 @@ use crate::{ /// This type can be used to construct an instance of `Client` through a /// builder-like pattern. pub struct ClientBuilder { - default_headers: bool, max_http_version: Option, stream_window_size: Option, conn_window_size: Option, - headers: HeaderMap, + fundamental_headers: bool, + default_headers: HeaderMap, timeout: Option, connector: Connector, middleware: M, @@ -44,15 +44,15 @@ impl ClientBuilder { (), > { ClientBuilder { - middleware: (), - default_headers: true, - headers: HeaderMap::new(), - timeout: Some(Duration::from_secs(5)), - local_address: None, - connector: Connector::new(), max_http_version: None, stream_window_size: None, conn_window_size: None, + fundamental_headers: true, + default_headers: HeaderMap::new(), + timeout: Some(Duration::from_secs(5)), + connector: Connector::new(), + middleware: (), + local_address: None, max_redirects: 10, } } @@ -78,8 +78,8 @@ where { ClientBuilder { middleware: self.middleware, + fundamental_headers: self.fundamental_headers, default_headers: self.default_headers, - headers: self.headers, timeout: self.timeout, local_address: self.local_address, connector, @@ -153,15 +153,31 @@ where self } - /// Do not add default request headers. + /// Do not add fundamental default request headers. + /// /// By default `Date` and `User-Agent` headers are set. pub fn no_default_headers(mut self) -> Self { - self.default_headers = false; + self.fundamental_headers = false; self } - /// Add default header. Headers added by this method - /// get added to every request. + /// Add default header. + /// + /// Headers added by this method get added to every request unless overriden by . + /// + /// # Panics + /// Panics if header name or value is invalid. + pub fn add_default_header(mut self, header: impl TryIntoHeaderPair) -> Self { + match header.try_into_header_pair() { + Ok((key, value)) => self.default_headers.append(key, value), + Err(err) => panic!("Header error: {:?}", err.into()), + } + + self + } + + #[doc(hidden)] + #[deprecated(since = "3.0.0", note = "Prefer `add_default_header((key, value))`.")] pub fn header(mut self, key: K, value: V) -> Self where HeaderName: TryFrom, @@ -172,11 +188,11 @@ where match HeaderName::try_from(key) { Ok(key) => match value.try_into_value() { Ok(value) => { - self.headers.append(key, value); + self.default_headers.append(key, value); } - Err(e) => log::error!("Header value error: {:?}", e), + Err(err) => log::error!("Header value error: {:?}", err), }, - Err(e) => log::error!("Header name error: {:?}", e), + Err(err) => log::error!("Header name error: {:?}", err), } self } @@ -190,10 +206,10 @@ where Some(password) => format!("{}:{}", username, password), None => format!("{}:", username), }; - self.header( + self.add_default_header(( header::AUTHORIZATION, format!("Basic {}", base64::encode(&auth)), - ) + )) } /// Set client wide HTTP bearer authentication header @@ -201,13 +217,12 @@ where where T: fmt::Display, { - self.header(header::AUTHORIZATION, format!("Bearer {}", token)) + self.add_default_header((header::AUTHORIZATION, format!("Bearer {}", token))) } - /// Registers middleware, in the form of a middleware component (type), - /// that runs during inbound and/or outbound processing in the request - /// life-cycle (request -> response), modifying request/response as - /// necessary, across all requests managed by the Client. + /// Registers middleware, in the form of a middleware component (type), that runs during inbound + /// and/or outbound processing in the request life-cycle (request -> response), + /// modifying request/response as necessary, across all requests managed by the `Client`. pub fn wrap( self, mw: M1, @@ -218,11 +233,11 @@ where { ClientBuilder { middleware: NestTransform::new(self.middleware, mw), - default_headers: self.default_headers, + fundamental_headers: self.fundamental_headers, max_http_version: self.max_http_version, stream_window_size: self.stream_window_size, conn_window_size: self.conn_window_size, - headers: self.headers, + default_headers: self.default_headers, timeout: self.timeout, connector: self.connector, local_address: self.local_address, @@ -237,10 +252,10 @@ where M::Transform: Service, { - let redirect_time = self.max_redirects; + let max_redirects = self.max_redirects; - if redirect_time > 0 { - self.wrap(Redirect::new().max_redirect_times(redirect_time)) + if max_redirects > 0 { + self.wrap(Redirect::new().max_redirect_times(max_redirects)) ._finish() } else { self._finish() @@ -272,7 +287,7 @@ where let connector = boxed::rc_service(self.middleware.new_transform(connector)); Client(ClientConfig { - headers: Rc::new(self.headers), + default_headers: Rc::new(self.default_headers), timeout: self.timeout, connector, }) @@ -288,7 +303,7 @@ mod tests { let client = ClientBuilder::new().basic_auth("username", Some("password")); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() @@ -299,7 +314,7 @@ mod tests { let client = ClientBuilder::new().basic_auth("username", None); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() @@ -313,7 +328,7 @@ mod tests { let client = ClientBuilder::new().bearer_auth("someS3cr3tAutht0k3n"); assert_eq!( client - .headers + .default_headers .get(header::AUTHORIZATION) .unwrap() .to_str() diff --git a/awc/src/lib.rs b/awc/src/lib.rs index 06fd33fac..00c559406 100644 --- a/awc/src/lib.rs +++ b/awc/src/lib.rs @@ -168,7 +168,7 @@ pub struct Client(ClientConfig); #[derive(Clone)] pub(crate) struct ClientConfig { pub(crate) connector: BoxConnectorService, - pub(crate) headers: Rc, + pub(crate) default_headers: Rc, pub(crate) timeout: Option, } @@ -204,7 +204,9 @@ impl Client { { let mut req = ClientRequest::new(method, url, self.0.clone()); - for header in self.0.headers.iter() { + for header in self.0.default_headers.iter() { + // header map is empty + // TODO: probably append instead req = req.insert_header_if_none(header); } req @@ -297,7 +299,7 @@ impl Client { >::Error: Into, { let mut req = ws::WebsocketsRequest::new(url, self.0.clone()); - for (key, value) in self.0.headers.iter() { + for (key, value) in self.0.default_headers.iter() { req.head.headers.insert(key.clone(), value.clone()); } req @@ -308,6 +310,6 @@ impl Client { /// Returns Some(&mut HeaderMap) when Client object is unique /// (No other clone of client exists at the same time). pub fn headers(&mut self) -> Option<&mut HeaderMap> { - Rc::get_mut(&mut self.0.headers) + Rc::get_mut(&mut self.0.default_headers) } } diff --git a/awc/src/middleware/redirect.rs b/awc/src/middleware/redirect.rs index 0fde48074..704d2d79d 100644 --- a/awc/src/middleware/redirect.rs +++ b/awc/src/middleware/redirect.rs @@ -442,13 +442,15 @@ mod tests { }); let client = ClientBuilder::new() - .header("custom", "value") + .add_default_header(("custom", "value")) .disable_redirects() .finish(); let res = client.get(srv.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 302); - let client = ClientBuilder::new().header("custom", "value").finish(); + let client = ClientBuilder::new() + .add_default_header(("custom", "value")) + .finish(); let res = client.get(srv.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 200); @@ -520,7 +522,7 @@ mod tests { // send a request to different origins, http://srv1/ then http://srv2/. So it should remove the header let client = ClientBuilder::new() - .header(header::AUTHORIZATION, "auth_key_value") + .add_default_header((header::AUTHORIZATION, "auth_key_value")) .finish(); let res = client.get(srv1.url("/")).send().await.unwrap(); assert_eq!(res.status().as_u16(), 200); diff --git a/awc/src/request.rs b/awc/src/request.rs index bfb72f1db..21f187e99 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -579,7 +579,7 @@ mod tests { #[actix_rt::test] async fn test_client_header() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .get("/"); @@ -597,7 +597,7 @@ mod tests { #[actix_rt::test] async fn test_client_header_override() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .get("/") .insert_header((header::CONTENT_TYPE, "222")); diff --git a/awc/src/ws.rs b/awc/src/ws.rs index e32a9affb..06d54aadb 100644 --- a/awc/src/ws.rs +++ b/awc/src/ws.rs @@ -445,7 +445,7 @@ mod tests { #[actix_rt::test] async fn test_header_override() { let req = Client::builder() - .header(header::CONTENT_TYPE, "111") + .add_default_header((header::CONTENT_TYPE, "111")) .finish() .ws("/") .set_header(header::CONTENT_TYPE, "222");