From 5703bd8160e7f0788af70740f84f9973272b09eb Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Tue, 26 Mar 2019 21:31:18 -0700 Subject: [PATCH] fix client cookies parsing --- awc/src/request.rs | 4 +- awc/src/response.rs | 27 ++++++++- awc/tests/test_client.rs | 126 +++++++++++++++++++-------------------- 3 files changed, 91 insertions(+), 66 deletions(-) diff --git a/awc/src/request.rs b/awc/src/request.rs index 649797df8..16e42939c 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -242,7 +242,7 @@ impl ClientRequest { self.header(header::CONTENT_LENGTH, wrt.get_mut().take().freeze()) } - /// Set HTTP basic authorization + /// Set HTTP basic authorization header pub fn basic_auth(self, username: U, password: Option

) -> Self where U: fmt::Display, @@ -258,7 +258,7 @@ impl ClientRequest { ) } - /// Set HTTP bearer authentication + /// Set HTTP bearer authentication header pub fn bearer_auth(self, token: T) -> Self where T: fmt::Display, diff --git a/awc/src/response.rs b/awc/src/response.rs index 4525bbc1a..5806dc91d 100644 --- a/awc/src/response.rs +++ b/awc/src/response.rs @@ -5,10 +5,15 @@ use bytes::{Bytes, BytesMut}; use futures::{Future, Poll, Stream}; use actix_http::error::PayloadError; -use actix_http::http::header::CONTENT_LENGTH; +use actix_http::http::header::{CONTENT_LENGTH, SET_COOKIE}; use actix_http::http::{HeaderMap, StatusCode, Version}; use actix_http::{Extensions, Head, HttpMessage, Payload, PayloadStream, ResponseHead}; +#[cfg(feature = "cookies")] +use actix_http::error::CookieParseError; +#[cfg(feature = "cookies")] +use cookie::Cookie; + /// Client Response pub struct ClientResponse { pub(crate) head: ResponseHead, @@ -33,6 +38,26 @@ impl HttpMessage for ClientResponse { fn take_payload(&mut self) -> Payload { std::mem::replace(&mut self.payload, Payload::None) } + + /// Load request cookies. + #[inline] + #[cfg(feature = "cookies")] + fn cookies(&self) -> Result>>, CookieParseError> { + struct Cookies(Vec>); + + if self.extensions().get::().is_none() { + let mut cookies = Vec::new(); + for hdr in self.headers().get_all(SET_COOKIE) { + let s = std::str::from_utf8(hdr.as_bytes()) + .map_err(CookieParseError::from)?; + cookies.push(Cookie::parse_encoded(s)?.into_owned()); + } + self.extensions_mut().insert(Cookies(cookies)); + } + Ok(Ref::map(self.extensions(), |ext| { + &ext.get::().unwrap().0 + })) + } } impl ClientResponse { diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index ac07eb6d0..698b5ab7d 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -8,7 +8,7 @@ use rand::Rng; use actix_http::HttpService; use actix_http_test::TestServer; -use actix_web::{http::header, web, App, HttpMessage, HttpRequest, HttpResponse}; +use actix_web::{http::header, web, App, Error, HttpMessage, HttpRequest, HttpResponse}; const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ Hello World Hello World Hello World Hello World Hello World \ @@ -352,69 +352,69 @@ fn test_client_brotli_encoding() { // assert_eq!(bytes, Bytes::from_static(STR.as_ref())); // } -// #[test] -// fn test_client_cookie_handling() { -// use actix_web::http::Cookie; -// fn err() -> Error { -// use std::io::{Error as IoError, ErrorKind}; -// // stub some generic error -// Error::from(IoError::from(ErrorKind::NotFound)) -// } -// let cookie1 = Cookie::build("cookie1", "value1").finish(); -// let cookie2 = Cookie::build("cookie2", "value2") -// .domain("www.example.org") -// .path("/") -// .secure(true) -// .http_only(true) -// .finish(); -// // Q: are all these clones really necessary? A: Yes, possibly -// let cookie1b = cookie1.clone(); -// let cookie2b = cookie2.clone(); -// let mut srv = test::TestServer::new(move |app| { -// let cookie1 = cookie1b.clone(); -// let cookie2 = cookie2b.clone(); -// app.handler(move |req: &HttpRequest| { -// // Check cookies were sent correctly -// req.cookie("cookie1") -// .ok_or_else(err) -// .and_then(|c1| { -// if c1.value() == "value1" { -// Ok(()) -// } else { -// Err(err()) -// } -// }) -// .and_then(|()| req.cookie("cookie2").ok_or_else(err)) -// .and_then(|c2| { -// if c2.value() == "value2" { -// Ok(()) -// } else { -// Err(err()) -// } -// }) -// // Send some cookies back -// .map(|_| { -// HttpResponse::Ok() -// .cookie(cookie1.clone()) -// .cookie(cookie2.clone()) -// .finish() -// }) -// }) -// }); +#[test] +fn test_client_cookie_handling() { + use actix_web::http::Cookie; + fn err() -> Error { + use std::io::{Error as IoError, ErrorKind}; + // stub some generic error + Error::from(IoError::from(ErrorKind::NotFound)) + } + let cookie1 = Cookie::build("cookie1", "value1").finish(); + let cookie2 = Cookie::build("cookie2", "value2") + .domain("www.example.org") + .path("/") + .secure(true) + .http_only(true) + .finish(); + // Q: are all these clones really necessary? A: Yes, possibly + let cookie1b = cookie1.clone(); + let cookie2b = cookie2.clone(); -// let request = srv -// .get() -// .cookie(cookie1.clone()) -// .cookie(cookie2.clone()) -// .finish() -// .unwrap(); -// let response = srv.execute(request.send()).unwrap(); -// assert!(response.status().is_success()); -// let c1 = response.cookie("cookie1").expect("Missing cookie1"); -// assert_eq!(c1, cookie1); -// let c2 = response.cookie("cookie2").expect("Missing cookie2"); -// assert_eq!(c2, cookie2); -// } + let mut srv = TestServer::new(move || { + let cookie1 = cookie1b.clone(); + let cookie2 = cookie2b.clone(); + + HttpService::new(App::new().route( + "/", + web::to(move |req: HttpRequest| { + // Check cookies were sent correctly + req.cookie("cookie1") + .ok_or_else(err) + .and_then(|c1| { + if c1.value() == "value1" { + Ok(()) + } else { + Err(err()) + } + }) + .and_then(|()| req.cookie("cookie2").ok_or_else(err)) + .and_then(|c2| { + if c2.value() == "value2" { + Ok(()) + } else { + Err(err()) + } + }) + // Send some cookies back + .map(|_| { + HttpResponse::Ok() + .cookie(cookie1.clone()) + .cookie(cookie2.clone()) + .finish() + }) + }), + )) + }); + + let request = srv.get().cookie(cookie1.clone()).cookie(cookie2.clone()); + let response = srv.block_on(request.send()).unwrap(); + assert!(response.status().is_success()); + let c1 = response.cookie("cookie1").expect("Missing cookie1"); + assert_eq!(c1, cookie1); + let c2 = response.cookie("cookie2").expect("Missing cookie2"); + assert_eq!(c2, cookie2); +} // #[test] // fn test_default_headers() {