From c61307944d7f0d7f79054d3b9ff7cd2ef580c61d Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 28 Sep 2020 00:54:49 +0100 Subject: [PATCH] improve clarity in header setting method names --- actix-http/src/header/common/content_type.rs | 19 +++---- actix-http/src/header/common/if_range.rs | 5 ++ actix-http/src/header/map.rs | 6 --- actix-http/src/response.rs | 55 ++++++++++++++------ actix-http/src/service.rs | 4 +- actix-http/src/ws/mod.rs | 6 +-- awc/tests/test_client.rs | 12 ++--- src/middleware/defaultheaders.rs | 7 ++- src/middleware/logger.rs | 2 +- src/service.rs | 2 +- 10 files changed, 73 insertions(+), 45 deletions(-) diff --git a/actix-http/src/header/common/content_type.rs b/actix-http/src/header/common/content_type.rs index a0baa5637..4985040ff 100644 --- a/actix-http/src/header/common/content_type.rs +++ b/actix-http/src/header/common/content_type.rs @@ -1,6 +1,7 @@ -use crate::header::CONTENT_TYPE; use mime::Mime; +use crate::header::CONTENT_TYPE; + header! { /// `Content-Type` header, defined in /// [RFC7231](http://tools.ietf.org/html/rfc7231#section-3.1.1.5) @@ -67,51 +68,51 @@ header! { } impl ContentType { - /// A constructor to easily create a `Content-Type: application/json` + /// A constructor to easily create a `Content-Type: application/json` /// header. #[inline] pub fn json() -> ContentType { ContentType(mime::APPLICATION_JSON) } - /// A constructor to easily create a `Content-Type: text/plain; + /// A constructor to easily create a `Content-Type: text/plain; /// charset=utf-8` header. #[inline] pub fn plaintext() -> ContentType { ContentType(mime::TEXT_PLAIN_UTF_8) } - /// A constructor to easily create a `Content-Type: text/html` header. + /// A constructor to easily create a `Content-Type: text/html` header. #[inline] pub fn html() -> ContentType { ContentType(mime::TEXT_HTML) } - /// A constructor to easily create a `Content-Type: text/xml` header. + /// A constructor to easily create a `Content-Type: text/xml` header. #[inline] pub fn xml() -> ContentType { ContentType(mime::TEXT_XML) } - /// A constructor to easily create a `Content-Type: + /// A constructor to easily create a `Content-Type: /// application/www-form-url-encoded` header. #[inline] pub fn form_url_encoded() -> ContentType { ContentType(mime::APPLICATION_WWW_FORM_URLENCODED) } - /// A constructor to easily create a `Content-Type: image/jpeg` header. + /// A constructor to easily create a `Content-Type: image/jpeg` header. #[inline] pub fn jpeg() -> ContentType { ContentType(mime::IMAGE_JPEG) } - /// A constructor to easily create a `Content-Type: image/png` header. + /// A constructor to easily create a `Content-Type: image/png` header. #[inline] pub fn png() -> ContentType { ContentType(mime::IMAGE_PNG) } - /// A constructor to easily create a `Content-Type: + /// A constructor to easily create a `Content-Type: /// application/octet-stream` header. #[inline] pub fn octet_stream() -> ContentType { diff --git a/actix-http/src/header/common/if_range.rs b/actix-http/src/header/common/if_range.rs index b14ad0391..4b6b721b5 100644 --- a/actix-http/src/header/common/if_range.rs +++ b/actix-http/src/header/common/if_range.rs @@ -67,6 +67,7 @@ impl Header for IfRange { fn name() -> HeaderName { header::IF_RANGE } + #[inline] fn parse(msg: &T) -> Result where @@ -74,14 +75,18 @@ impl Header for IfRange { { let etag: Result = from_one_raw_str(msg.headers().get(&header::IF_RANGE)); + if let Ok(etag) = etag { return Ok(IfRange::EntityTag(etag)); } + let date: Result = from_one_raw_str(msg.headers().get(&header::IF_RANGE)); + if let Ok(date) = date { return Ok(IfRange::Date(date)); } + Err(ParseError::Header) } } diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 36c050b8f..869c54716 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -202,9 +202,6 @@ impl HeaderMap { /// Inserts a key-value pair into the map. /// - /// If the map did not previously have this key present, then `None` is - /// returned. - /// /// If the map did have this key present, the new value is associated with /// the key and all previous values are removed. **Note** that only a single /// one of the previous values is returned. If there are multiple values @@ -220,9 +217,6 @@ impl HeaderMap { /// Inserts a key-value pair into the map. /// - /// If the map did not previously have this key present, then `false` is - /// returned. - /// /// If the map did have this key present, the new value is pushed to the end /// of the list of values currently associated with the key. The key is not /// updated, though; this matters for types that can be `==` without being diff --git a/actix-http/src/response.rs b/actix-http/src/response.rs index 2def67168..c9320b53a 100644 --- a/actix-http/src/response.rs +++ b/actix-http/src/response.rs @@ -354,7 +354,6 @@ impl ResponseBuilder { /// .finish()) /// } /// ``` - #[doc(hidden)] pub fn set(&mut self, hdr: H) -> &mut Self { if let Some(parts) = parts(&mut self.head, &self.err) { match hdr.try_into() { @@ -367,7 +366,7 @@ impl ResponseBuilder { self } - /// Append a header to existing headers. + /// Append a header to response. /// /// ```rust /// use actix_http::{http, Request, Response}; @@ -379,7 +378,7 @@ impl ResponseBuilder { /// .finish() /// } /// ``` - pub fn header(&mut self, key: K, value: V) -> &mut Self + pub fn append_header(&mut self, key: K, value: V) -> &mut Self where HeaderName: TryFrom, >::Error: Into, @@ -399,7 +398,19 @@ impl ResponseBuilder { self } - /// Set a header. + /// Append a header to response. + #[doc(hidden)] + #[deprecated = "Renamed to `append_header`."] + pub fn header(&mut self, key: K, value: V) -> &mut Self + where + HeaderName: TryFrom, + >::Error: Into, + V: IntoHeaderValue, + { + self.append_header(key, value) + } + + /// Set a header, overwriting any with the same key. /// /// ```rust /// use actix_http::{http, Request, Response}; @@ -411,7 +422,7 @@ impl ResponseBuilder { /// .finish() /// } /// ``` - pub fn set_header(&mut self, key: K, value: V) -> &mut Self + pub fn insert_header(&mut self, key: K, value: V) -> &mut Self where HeaderName: TryFrom, >::Error: Into, @@ -431,6 +442,18 @@ impl ResponseBuilder { self } + /// Set a header, overwriting any with the same key. + #[doc(hidden)] + #[deprecated = "Renamed to `insert_header`."] + pub fn set_header(&mut self, key: K, value: V) -> &mut Self + where + HeaderName: TryFrom, + >::Error: Into, + V: IntoHeaderValue, + { + self.insert_header(key, value) + } + /// Set the custom reason for the response. #[inline] pub fn reason(&mut self, reason: &'static str) -> &mut Self { @@ -458,7 +481,7 @@ impl ResponseBuilder { if let Some(parts) = parts(&mut self.head, &self.err) { parts.set_connection_type(ConnectionType::Upgrade); } - self.set_header(header::UPGRADE, value) + self.insert_header(header::UPGRADE, value) } /// Force close connection, even if it is marked as keep-alive @@ -473,7 +496,7 @@ impl ResponseBuilder { /// Disable chunked transfer encoding for HTTP/1.1 streaming responses. #[inline] pub fn no_chunking(&mut self, len: u64) -> &mut Self { - self.header(header::CONTENT_LENGTH, len); + self.insert_header(header::CONTENT_LENGTH, len); if let Some(parts) = parts(&mut self.head, &self.err) { parts.no_chunking(true); @@ -556,6 +579,7 @@ impl ResponseBuilder { /// This method calls provided closure with builder reference if value is /// true. + #[deprecated = "Conditionally assign headers with standard if statement."] pub fn if_true(&mut self, value: bool, f: F) -> &mut Self where F: FnOnce(&mut ResponseBuilder), @@ -568,6 +592,7 @@ impl ResponseBuilder { /// This method calls provided closure with builder reference if value is /// Some. + #[deprecated = "Conditionally assign headers with standard if-let statement."] pub fn if_some(&mut self, value: Option, f: F) -> &mut Self where F: FnOnce(T, &mut ResponseBuilder), @@ -592,10 +617,10 @@ impl ResponseBuilder { head.extensions.borrow_mut() } - #[inline] /// Set a body and generate `Response`. /// /// `ResponseBuilder` can not be used after this call. + #[inline] pub fn body>(&mut self, body: B) -> Response { self.message_body(body.into()) } @@ -626,10 +651,10 @@ impl ResponseBuilder { } } - #[inline] /// Set a streaming body and generate `Response`. /// /// `ResponseBuilder` can not be used after this call. + #[inline] pub fn streaming(&mut self, stream: S) -> Response where S: Stream> + Unpin + 'static, @@ -638,10 +663,10 @@ impl ResponseBuilder { self.body(Body::from_message(BodyStream::new(stream))) } - #[inline] /// Set a json body and generate `Response` /// /// `ResponseBuilder` can not be used after this call. + #[inline] pub fn json(&mut self, value: T) -> Response { self.json2(&value) } @@ -658,7 +683,7 @@ impl ResponseBuilder { true }; if !contains { - self.header(header::CONTENT_TYPE, "application/json"); + self.insert_header(header::CONTENT_TYPE, "application/json"); } self.body(Body::from(body)) @@ -667,10 +692,10 @@ impl ResponseBuilder { } } - #[inline] /// Set an empty body and generate `Response` /// /// `ResponseBuilder` can not be used after this call. + #[inline] pub fn finish(&mut self) -> Response { self.body(Body::Empty) } @@ -854,8 +879,8 @@ mod tests { #[test] fn test_debug() { let resp = Response::Ok() - .header(COOKIE, HeaderValue::from_static("cookie1=value1; ")) - .header(COOKIE, HeaderValue::from_static("cookie2=value2; ")) + .append_header(COOKIE, HeaderValue::from_static("cookie1=value1; ")) + .append_header(COOKIE, HeaderValue::from_static("cookie2=value2; ")) .finish(); let dbg = format!("{:?}", resp); assert!(dbg.contains("Response")); @@ -921,7 +946,7 @@ mod tests { #[test] fn test_basic_builder() { - let resp = Response::Ok().header("X-TEST", "value").finish(); + let resp = Response::Ok().insert_header("X-TEST", "value").finish(); assert_eq!(resp.status(), StatusCode::OK); } diff --git a/actix-http/src/service.rs b/actix-http/src/service.rs index 9ee579702..074edd8e2 100644 --- a/actix-http/src/service.rs +++ b/actix-http/src/service.rs @@ -218,7 +218,7 @@ mod openssl { U::InitError: fmt::Debug, ::Future: 'static, { - /// Create openssl based service + /// Create OpenSSL based service pub fn openssl( self, acceptor: SslAcceptor, @@ -280,7 +280,7 @@ mod rustls { U::InitError: fmt::Debug, ::Future: 'static, { - /// Create openssl based service + /// Create Rustls based service pub fn rustls( self, mut config: ServerConfig, diff --git a/actix-http/src/ws/mod.rs b/actix-http/src/ws/mod.rs index 6ffdecc35..2da325bbc 100644 --- a/actix-http/src/ws/mod.rs +++ b/actix-http/src/ws/mod.rs @@ -89,7 +89,7 @@ impl ResponseError for HandshakeError { fn error_response(&self) -> Response { match *self { HandshakeError::GetMethodRequired => Response::MethodNotAllowed() - .header(header::ALLOW, "GET") + .insert_header(header::ALLOW, "GET") .finish(), HandshakeError::NoWebsocketUpgrade => Response::BadRequest() .reason("No WebSocket UPGRADE header found") @@ -181,8 +181,8 @@ pub fn handshake_response(req: &RequestHead) -> ResponseBuilder { Response::build(StatusCode::SWITCHING_PROTOCOLS) .upgrade("websocket") - .header(header::TRANSFER_ENCODING, "chunked") - .header(header::SEC_WEBSOCKET_ACCEPT, key.as_str()) + .insert_header(header::TRANSFER_ENCODING, "chunked") + .insert_header(header::SEC_WEBSOCKET_ACCEPT, key.as_str()) .take() } diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index a9552d0d5..8a034420d 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -12,7 +12,7 @@ use flate2::Compression; use futures_util::future::ok; use rand::Rng; -use actix_http::HttpService; +use actix_http::{http::ContentEncoding, HttpService}; use actix_http_test::test_server; use actix_service::{map_config, pipeline_factory}; use actix_web::dev::{AppConfig, BodyEncoding}; @@ -438,7 +438,7 @@ async fn test_client_gzip_encoding() { let data = e.finish().unwrap(); HttpResponse::Ok() - .header("content-encoding", "gzip") + .insert_header(header::CONTENT_ENCODING, ContentEncoding::Gzip.as_str()) .body(data) }))) }); @@ -461,7 +461,7 @@ async fn test_client_gzip_encoding_large() { let data = e.finish().unwrap(); HttpResponse::Ok() - .header("content-encoding", "gzip") + .insert_header(header::CONTENT_ENCODING, ContentEncoding::Gzip.as_str()) .body(data) }))) }); @@ -488,7 +488,7 @@ async fn test_client_gzip_encoding_large_random() { e.write_all(&data).unwrap(); let data = e.finish().unwrap(); HttpResponse::Ok() - .header("content-encoding", "gzip") + .insert_header(header::CONTENT_ENCODING, ContentEncoding::Gzip.as_str()) .body(data) }))) }); @@ -510,7 +510,7 @@ async fn test_client_brotli_encoding() { e.write_all(&data).unwrap(); let data = e.finish().unwrap(); HttpResponse::Ok() - .header("content-encoding", "br") + .insert_header(header::CONTENT_ENCODING, ContentEncoding::Br.as_str()) .body(data) }))) }); @@ -537,7 +537,7 @@ async fn test_client_brotli_encoding_large_random() { e.write_all(&data).unwrap(); let data = e.finish().unwrap(); HttpResponse::Ok() - .header("content-encoding", "br") + .insert_header(header::CONTENT_ENCODING, ContentEncoding::Br.as_str()) .body(data) }))) }); diff --git a/src/middleware/defaultheaders.rs b/src/middleware/defaultheaders.rs index 6d43aba95..3f95c5c74 100644 --- a/src/middleware/defaultheaders.rs +++ b/src/middleware/defaultheaders.rs @@ -180,8 +180,11 @@ mod tests { let req = TestRequest::default().to_srv_request(); let srv = |req: ServiceRequest| { - ok(req - .into_response(HttpResponse::Ok().header(CONTENT_TYPE, "0002").finish())) + ok(req.into_response( + HttpResponse::Ok() + .insert_header(CONTENT_TYPE, "0002") + .finish(), + )) }; let mut mw = DefaultHeaders::new() .header(CONTENT_TYPE, "0001") diff --git a/src/middleware/logger.rs b/src/middleware/logger.rs index 51d4722d7..8286cd996 100644 --- a/src/middleware/logger.rs +++ b/src/middleware/logger.rs @@ -522,7 +522,7 @@ mod tests { let srv = |req: ServiceRequest| { ok(req.into_response( HttpResponse::build(StatusCode::OK) - .header("X-Test", "ttt") + .insert_header("X-Test", "ttt") .finish(), )) }; diff --git a/src/service.rs b/src/service.rs index a861ba38c..f0aebfac5 100644 --- a/src/service.rs +++ b/src/service.rs @@ -626,7 +626,7 @@ mod tests { assert!(s.contains("test=1")); assert!(s.contains("x-test")); - let res = HttpResponse::Ok().header("x-test", "111").finish(); + let res = HttpResponse::Ok().insert_header("x-test", "111").finish(); let res = TestRequest::post() .uri("/index.html?test=1") .to_srv_response(res);