From d121432b3a1e5d5bfb95f8f71149d74c7753593a Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 19 Jan 2022 22:42:40 +0000 Subject: [PATCH] remove ambiguous `HttpResponseBuilder::del_cookie` --- CHANGES.md | 1 + src/response/builder.rs | 111 +++++++++++++++------------------------- 2 files changed, 43 insertions(+), 69 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index fae671072..757a15a3b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ ### Removed - `HttpRequest::req_data[_mut]()`; request-local data is still available through `.extensions()`. [#2585] +- `HttpRequestBuilder::del_cookie [#2585]: https://github.com/actix/actix-web/pull/2585 [#2586]: https://github.com/actix/actix-web/pull/2586 diff --git a/src/response/builder.rs b/src/response/builder.rs index c8e44729a..5062da1f6 100644 --- a/src/response/builder.rs +++ b/src/response/builder.rs @@ -6,18 +6,17 @@ use std::{ task::{Context, Poll}, }; -use actix_http::{ - error::HttpError, - header::{self, HeaderName, TryIntoHeaderPair, TryIntoHeaderValue}, - ConnectionType, Extensions, Response, ResponseHead, StatusCode, -}; +use actix_http::{error::HttpError, Response, ResponseHead}; use bytes::Bytes; use futures_core::Stream; use serde::Serialize; use crate::{ body::{BodyStream, BoxBody, MessageBody}, + dev::Extensions, error::{Error, JsonPayloadError}, + http::header::{self, HeaderName, TryIntoHeaderPair, TryIntoHeaderValue}, + http::{ConnectionType, StatusCode}, BoxError, HttpRequest, HttpResponse, Responder, }; @@ -26,9 +25,7 @@ use crate::{ /// This type can be used to construct an instance of `Response` through a builder-like pattern. pub struct HttpResponseBuilder { res: Option>, - err: Option, - #[cfg(feature = "cookies")] - cookies: Option, + error: Option, } impl HttpResponseBuilder { @@ -37,9 +34,7 @@ impl HttpResponseBuilder { pub fn new(status: StatusCode) -> Self { Self { res: Some(Response::with_body(status, BoxBody::new(()))), - err: None, - #[cfg(feature = "cookies")] - cookies: None, + error: None, } } @@ -68,7 +63,7 @@ impl HttpResponseBuilder { Ok((key, value)) => { parts.headers.insert(key, value); } - Err(e) => self.err = Some(e.into()), + Err(e) => self.error = Some(e.into()), }; } @@ -90,7 +85,7 @@ impl HttpResponseBuilder { if let Some(parts) = self.inner() { match header.try_into_pair() { Ok((key, value)) => parts.headers.append(key, value), - Err(e) => self.err = Some(e.into()), + Err(e) => self.error = Some(e.into()), }; } @@ -109,14 +104,14 @@ impl HttpResponseBuilder { K::Error: Into, V: TryIntoHeaderValue, { - if self.err.is_some() { + if self.error.is_some() { return self; } match (key.try_into(), value.try_into_value()) { (Ok(name), Ok(value)) => return self.insert_header((name, value)), - (Err(err), _) => self.err = Some(err.into()), - (_, Err(err)) => self.err = Some(err.into()), + (Err(err), _) => self.error = Some(err.into()), + (_, Err(err)) => self.error = Some(err.into()), } self @@ -134,14 +129,14 @@ impl HttpResponseBuilder { K::Error: Into, V: TryIntoHeaderValue, { - if self.err.is_some() { + if self.error.is_some() { return self; } match (key.try_into(), value.try_into_value()) { (Ok(name), Ok(value)) => return self.append_header((name, value)), - (Err(err), _) => self.err = Some(err.into()), - (_, Err(err)) => self.err = Some(err.into()), + (Err(err), _) => self.error = Some(err.into()), + (_, Err(err)) => self.error = Some(err.into()), } self @@ -214,18 +209,23 @@ impl HttpResponseBuilder { Ok(value) => { parts.headers.insert(header::CONTENT_TYPE, value); } - Err(e) => self.err = Some(e.into()), + Err(e) => self.error = Some(e.into()), }; } self } - /// Set a cookie. + /// Add a cookie to the response. /// + /// To send a "removal" cookie, call [`.make_removal()`](cookie::Cookie::make_removal) on the + /// given cookie. See [`HttpResponse::add_removal_cookie()`] to learn more. + /// + /// # Examples + /// Send a new cookie: /// ``` /// use actix_web::{HttpResponse, cookie::Cookie}; /// - /// HttpResponse::Ok() + /// let res = HttpResponse::Ok() /// .cookie( /// Cookie::build("name", "value") /// .domain("www.rust-lang.org") @@ -236,45 +236,30 @@ impl HttpResponseBuilder { /// ) /// .finish(); /// ``` - #[cfg(feature = "cookies")] - pub fn cookie<'c>(&mut self, cookie: cookie::Cookie<'c>) -> &mut Self { - if self.cookies.is_none() { - let mut jar = cookie::CookieJar::new(); - jar.add(cookie.into_owned()); - self.cookies = Some(jar) - } else { - self.cookies.as_mut().unwrap().add(cookie.into_owned()); - } - self - } - - /// Remove cookie. - /// - /// A `Set-Cookie` header is added that will delete a cookie with the same name from the client. /// + /// Send a removal cookie: /// ``` - /// use actix_web::{HttpRequest, HttpResponse, Responder}; + /// use actix_web::{HttpResponse, cookie::Cookie}; /// - /// async fn handler(req: HttpRequest) -> impl Responder { - /// let mut builder = HttpResponse::Ok(); + /// // the name, domain and path match the cookie created in the previous example + /// let mut cookie = Cookie::new("name", "value-does-not-matter"); + /// .domain("www.rust-lang.org") + /// .path("/"); + /// cookie.make_removal(); /// - /// if let Some(ref cookie) = req.cookie("name") { - /// builder.del_cookie(cookie); - /// } - /// - /// builder.finish() - /// } + /// let res = HttpResponse::Ok() + /// .cookie(cookie) + /// .finish(); /// ``` #[cfg(feature = "cookies")] - pub fn del_cookie(&mut self, cookie: &cookie::Cookie<'_>) -> &mut Self { - if self.cookies.is_none() { - self.cookies = Some(cookie::CookieJar::new()) + pub fn cookie(&mut self, cookie: cookie::Cookie<'_>) -> &mut Self { + match cookie.to_string().try_into_value() { + Ok(hdr_val) => self.append_header((header::SET_COOKIE, hdr_val)), + Err(err) => { + self.error = Some(err.into()); + self + } } - let jar = self.cookies.as_mut().unwrap(); - let cookie = cookie.clone().into_owned(); - jar.add_original(cookie.clone()); - jar.remove(cookie); - self } /// Returns a reference to the response-local data/extensions container. @@ -312,7 +297,7 @@ impl HttpResponseBuilder { /// /// `HttpResponseBuilder` can not be used after this call. pub fn message_body(&mut self, body: B) -> Result, Error> { - if let Some(err) = self.err.take() { + if let Some(err) = self.error.take() { return Err(err.into()); } @@ -325,16 +310,6 @@ impl HttpResponseBuilder { #[allow(unused_mut)] // mut is only unused when cookies are disabled let mut res = HttpResponse::from(res); - #[cfg(feature = "cookies")] - if let Some(ref jar) = self.cookies { - for cookie in jar.delta() { - match actix_http::header::HeaderValue::from_str(&cookie.to_string()) { - Ok(val) => res.headers_mut().append(header::SET_COOKIE, val), - Err(err) => return Err(err.into()), - }; - } - } - Ok(res) } @@ -384,14 +359,12 @@ impl HttpResponseBuilder { pub fn take(&mut self) -> Self { Self { res: self.res.take(), - err: self.err.take(), - #[cfg(feature = "cookies")] - cookies: self.cookies.take(), + error: self.error.take(), } } fn inner(&mut self) -> Option<&mut ResponseHead> { - if self.err.is_some() { + if self.error.is_some() { return None; }