From 722ee93f8ba8924c9700c23a24a74569c8b1ab2f Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Tue, 7 Mar 2017 01:19:06 -0800 Subject: [PATCH] Update to cookie 0.7. Use 256-bit session_keys. This commit involves several breaking changes: * `session_key` config param must be a 256-bit base64 encoded string. * `FromRequest` is implemented for `Cookies`, not `Cookie`. * Only a single `Cookies` instance can be retrieved at a time. * `Config::take_session_key` returns a `Vec`. * `Into
` is implemented for `&Cookie`, not `Cookie`. --- codegen/tests/run-pass/complete-decorator.rs | 2 +- examples/config/Rocket.toml | 4 +- examples/cookies/src/main.rs | 6 +- lib/Cargo.toml | 9 ++- lib/src/config/builder.rs | 4 +- lib/src/config/config.rs | 38 ++++++------ lib/src/config/mod.rs | 20 +++--- lib/src/http/cookies.rs | 65 +++++++++++++++++--- lib/src/http/header.rs | 4 +- lib/src/lib.rs | 1 + lib/src/request/from_request.rs | 2 +- lib/src/request/request.rs | 31 +++++++--- lib/src/response/flash.rs | 18 ++---- 13 files changed, 129 insertions(+), 75 deletions(-) diff --git a/codegen/tests/run-pass/complete-decorator.rs b/codegen/tests/run-pass/complete-decorator.rs index f0f09f58..1819ec5e 100644 --- a/codegen/tests/run-pass/complete-decorator.rs +++ b/codegen/tests/run-pass/complete-decorator.rs @@ -16,7 +16,7 @@ struct User<'a> { fn get<'r>(name: &str, query: User<'r>, user: Form<'r, User<'r>>, - cookies: &Cookies) + cookies: Cookies) -> &'static str { "hi" } diff --git a/examples/config/Rocket.toml b/examples/config/Rocket.toml index 3640d3e4..2ae9bd0f 100644 --- a/examples/config/Rocket.toml +++ b/examples/config/Rocket.toml @@ -15,7 +15,7 @@ port = 80 log = "normal" workers = 8 # don't use this key! generate your own and keep it private! -session_key = "VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5" +session_key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg=" [production] address = "0.0.0.0" @@ -23,4 +23,4 @@ port = 80 workers = 12 log = "critical" # don't use this key! generate your own and keep it private! -session_key = "adL5fFIPmZBrlyHk2YT4NLV3YCk2gFXz" +session_key = "hPRYyVRiMyxpw5sBB1XeCMN1kFsDCqKvBi2QJxBVHQk=" diff --git a/examples/cookies/src/main.rs b/examples/cookies/src/main.rs index b45fe625..243c1954 100644 --- a/examples/cookies/src/main.rs +++ b/examples/cookies/src/main.rs @@ -20,14 +20,14 @@ struct Message { } #[post("/submit", data = "")] -fn submit(cookies: &Cookies, message: Form) -> Redirect { +fn submit(mut cookies: Cookies, message: Form) -> Redirect { cookies.add(Cookie::new("message", message.into_inner().message)); Redirect::to("/") } #[get("/")] -fn index(cookies: &Cookies) -> Template { - let cookie = cookies.find("message"); +fn index(cookies: Cookies) -> Template { + let cookie = cookies.get("message"); let mut context = HashMap::new(); if let Some(ref cookie) = cookie { context.insert("message", cookie.value()); diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 6a3fd4ce..e95db71d 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -25,10 +25,13 @@ state = "^0.2" time = "^0.1" memchr = "1" +# FIXME: session support should be optional +base64 = "0.4" + +# FIXME: session support should be optional [dependencies.cookie] -version = "^0.6" -default-features = false -features = ["percent-encode"] +version = "^0.7" +features = ["percent-encode", "secure"] [dev-dependencies] lazy_static = "0.2" diff --git a/lib/src/config/builder.rs b/lib/src/config/builder.rs index 97f63a33..36746c4f 100644 --- a/lib/src/config/builder.rs +++ b/lib/src/config/builder.rs @@ -152,12 +152,12 @@ impl ConfigBuilder { /// use rocket::LoggingLevel; /// use rocket::config::{Config, Environment}; /// - /// let key = "VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5"; + /// let key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg="; /// let mut config = Config::build(Environment::Staging) /// .session_key(key) /// .unwrap(); /// - /// assert_eq!(config.take_session_key(), Some(key.to_string())); + /// assert!(config.take_session_key().is_some()); /// ``` pub fn session_key>(mut self, key: K) -> Self { self.session_key = Some(key.into()); diff --git a/lib/src/config/config.rs b/lib/src/config/config.rs index 65076166..1f48a8ac 100644 --- a/lib/src/config/config.rs +++ b/lib/src/config/config.rs @@ -9,7 +9,7 @@ use std::env; use config::Environment::*; use config::{self, Value, ConfigBuilder, Environment, ConfigError}; -use num_cpus; +use {num_cpus, base64}; use logger::LoggingLevel; /// Structure for Rocket application configuration. @@ -44,7 +44,7 @@ pub struct Config { /// The path to the configuration file this config belongs to. pub config_path: PathBuf, /// The session key. - session_key: RwLock>, + session_key: RwLock>>, } macro_rules! parse { @@ -175,12 +175,6 @@ impl Config { ConfigError::BadType(id, expect, actual, self.config_path.clone()) } - // Aliases to `set` before the method is removed. - pub(crate) fn set_raw(&mut self, name: &str, val: &Value) -> config::Result<()> { - #[allow(deprecated)] - self.set(name, val) - } - /// Sets the configuration `val` for the `name` entry. If the `name` is one /// of "address", "port", "session_key", "log", or "workers" (the "default" /// values), the appropriate value in the `self` Config structure is set. @@ -195,8 +189,7 @@ impl Config { /// * **workers**: Integer (16-bit unsigned) /// * **log**: String /// * **session_key**: String (192-bit base64) - #[deprecated(since="0.2", note="use the set_{param} methods instead")] - pub fn set(&mut self, name: &str, val: &Value) -> config::Result<()> { + pub(crate) fn set_raw(&mut self, name: &str, val: &Value) -> config::Result<()> { if name == "address" { let address_str = parse!(self, name, val, as_str, "a string")?; self.set_address(address_str)?; @@ -335,20 +328,27 @@ impl Config { /// # use rocket::config::ConfigError; /// # fn config_test() -> Result<(), ConfigError> { /// let mut config = Config::new(Environment::Staging)?; - /// assert!(config.set_session_key("VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5").is_ok()); + /// let key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg="; + /// assert!(config.set_session_key(key).is_ok()); /// assert!(config.set_session_key("hello? anyone there?").is_err()); /// # Ok(()) /// # } /// ``` pub fn set_session_key>(&mut self, key: K) -> config::Result<()> { let key = key.into(); - if key.len() != 32 { - return Err(self.bad_type("session_key", - "string", - "a 192-bit base64 string")); + let error = self.bad_type("session_key", "string", + "a 256-bit base64 encoded string"); + + if key.len() != 44 { + return Err(error); } - self.session_key = RwLock::new(Some(key)); + let bytes = match base64::decode(&key) { + Ok(bytes) => bytes, + Err(_) => return Err(error) + }; + + self.session_key = RwLock::new(Some(bytes)); Ok(()) } @@ -435,21 +435,21 @@ impl Config { /// use rocket::config::{Config, Environment}; /// /// // Create a new config with a session key. - /// let key = "adL5fFIPmZBrlyHk2YT4NLV3YCk2gFXz"; + /// let key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg="; /// let config = Config::build(Environment::Staging) /// .session_key(key) /// .unwrap(); /// /// // Get the key for the first time. /// let session_key = config.take_session_key(); - /// assert_eq!(session_key, Some(key.to_string())); + /// assert!(session_key.is_some()); /// /// // Try to get the key again. /// let session_key_again = config.take_session_key(); /// assert_eq!(session_key_again, None); /// ``` #[inline] - pub fn take_session_key(&self) -> Option { + pub fn take_session_key(&self) -> Option> { let mut key = self.session_key.write().expect("couldn't lock session key"); key.take() } diff --git a/lib/src/config/mod.rs b/lib/src/config/mod.rs index 113d92c5..1fc3c04a 100644 --- a/lib/src/config/mod.rs +++ b/lib/src/config/mod.rs @@ -40,9 +40,9 @@ //! * examples: `12`, `1`, `4` //! * **log**: _[string]_ how much information to log; one of `"normal"`, //! `"debug"`, or `"critical"` -//! * **session_key**: _[string]_ a 192-bit base64 encoded string (32 +//! * **session_key**: _[string]_ a 256-bit base64 encoded string (44 //! characters) to use as the session key -//! * example: `"VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5"` +//! * example: `"8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg="` //! //! ### Rocket.toml //! @@ -70,7 +70,7 @@ //! workers = max(number_of_cpus, 2) //! log = "normal" //! # don't use this key! generate your own and keep it private! -//! session_key = "VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5" +//! session_key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg=" //! //! [production] //! address = "0.0.0.0" @@ -78,7 +78,7 @@ //! workers = max(number_of_cpus, 2) //! log = "critical" //! # don't use this key! generate your own and keep it private! -//! session_key = "adL5fFIPmZBrlyHk2YT4NLV3YCk2gFXz" +//! session_key = "hPRYyVRiMyxpw5sBB1XeCMN1kFsDCqKvBi2QJxBVHQk=" //! ``` //! //! The `workers` parameter is computed by Rocket automatically; the value above @@ -587,7 +587,7 @@ mod test { port = 7810 workers = 21 log = "critical" - session_key = "01234567890123456789012345678901" + session_key = "8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg=" template_dir = "mine" json = true pi = 3.14 @@ -598,7 +598,7 @@ mod test { .port(7810) .workers(21) .log_level(LoggingLevel::Critical) - .session_key("01234567890123456789012345678901") + .session_key("8Xui8SN4mI+7egV/9dlfYYLGQJeEx4+DwmSQLwDVXJg=") .extra("template_dir", "mine") .extra("json", true) .extra("pi", 3.14); @@ -873,19 +873,19 @@ mod test { check_config!(RocketConfig::parse(r#" [stage] - session_key = "VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5" + session_key = "TpUiXK2d/v5DFxJnWL12suJKPExKR8h9zd/o+E7SU+0=" "#.to_string(), TEST_CONFIG_FILENAME), { default_config(Staging).session_key( - "VheMwXIBygSmOlZAhuWl2B+zgvTN3WW5" + "TpUiXK2d/v5DFxJnWL12suJKPExKR8h9zd/o+E7SU+0=" ) }); check_config!(RocketConfig::parse(r#" [stage] - session_key = "adL5fFIPmZBrlyHk2YT4NLV3YCk2gFXz" + session_key = "jTyprDberFUiUFsJ3vcb1XKsYHWNBRvWAnXTlbTgGFU=" "#.to_string(), TEST_CONFIG_FILENAME), { default_config(Staging).session_key( - "adL5fFIPmZBrlyHk2YT4NLV3YCk2gFXz" + "jTyprDberFUiUFsJ3vcb1XKsYHWNBRvWAnXTlbTgGFU=" ) }); } diff --git a/lib/src/http/cookies.rs b/lib/src/http/cookies.rs index 02c08162..1371f156 100644 --- a/lib/src/http/cookies.rs +++ b/lib/src/http/cookies.rs @@ -1,17 +1,62 @@ use http::Header; -pub use cookie::Cookie; -pub use cookie::CookieJar; -pub use cookie::CookieBuilder; +use std::cell::RefMut; -/// Type alias to a `'static` CookieJar. -/// -/// A `CookieJar` should never be used without a `'static` lifetime. As a -/// result, you should always use this alias. -pub type Cookies = self::CookieJar<'static>; +pub use cookie::{Cookie, CookieJar, Iter, CookieBuilder, Delta}; -impl<'c> From> for Header<'static> { - fn from(cookie: Cookie) -> Header<'static> { +#[derive(Debug)] +pub enum Cookies<'a> { + Jarred(RefMut<'a, CookieJar>), + Empty(CookieJar) +} + +impl<'a> From> for Cookies<'a> { + fn from(jar: RefMut<'a, CookieJar>) -> Cookies<'a> { + Cookies::Jarred(jar) + } +} + +impl<'a> Cookies<'a> { + pub(crate) fn empty() -> Cookies<'static> { + Cookies::Empty(CookieJar::new()) + } + + pub fn get(&self, name: &str) -> Option<&Cookie<'static>> { + match *self { + Cookies::Jarred(ref jar) => jar.get(name), + Cookies::Empty(_) => None + } + } + + pub fn add(&mut self, cookie: Cookie<'static>) { + if let Cookies::Jarred(ref mut jar) = *self { + jar.add(cookie) + } + } + + pub fn remove(&mut self, cookie: Cookie<'static>) { + if let Cookies::Jarred(ref mut jar) = *self { + jar.remove(cookie) + } + } + + pub fn iter(&self) -> Iter { + match *self { + Cookies::Jarred(ref jar) => jar.iter(), + Cookies::Empty(ref jar) => jar.iter() + } + } + + pub fn delta(&self) -> Delta { + match *self { + Cookies::Jarred(ref jar) => jar.delta(), + Cookies::Empty(ref jar) => jar.delta() + } + } +} + +impl<'a, 'c> From<&'a Cookie<'c>> for Header<'static> { + fn from(cookie: &Cookie) -> Header<'static> { Header::new("Set-Cookie", cookie.encoded().to_string()) } } diff --git a/lib/src/http/header.rs b/lib/src/http/header.rs index 92ae0590..2d203e13 100644 --- a/lib/src/http/header.rs +++ b/lib/src/http/header.rs @@ -340,10 +340,10 @@ impl<'h> HeaderMap<'h> { /// /// let mut map = HeaderMap::new(); /// - /// map.add(Cookie::new("a", "b")); + /// map.add(&Cookie::new("a", "b")); /// assert_eq!(map.get("Set-Cookie").count(), 1); /// - /// map.add(Cookie::new("c", "d")); + /// map.add(&Cookie::new("c", "d")); /// assert_eq!(map.get("Set-Cookie").count(), 2); /// ``` #[inline(always)] diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 06df88b8..6066853b 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -104,6 +104,7 @@ extern crate state; extern crate cookie; extern crate time; extern crate memchr; +extern crate base64; #[cfg(test)] #[macro_use] extern crate lazy_static; diff --git a/lib/src/request/from_request.rs b/lib/src/request/from_request.rs index f6ea9052..2d9bc309 100644 --- a/lib/src/request/from_request.rs +++ b/lib/src/request/from_request.rs @@ -204,7 +204,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for Method { } } -impl<'a, 'r> FromRequest<'a, 'r> for &'a Cookies { +impl<'a, 'r> FromRequest<'a, 'r> for Cookies<'a> { type Error = (); fn from_request(request: &'a Request<'r>) -> Outcome { diff --git a/lib/src/request/request.rs b/lib/src/request/request.rs index ea5d38f0..a4e90608 100644 --- a/lib/src/request/request.rs +++ b/lib/src/request/request.rs @@ -14,6 +14,8 @@ use router::Route; use http::uri::{URI, Segments}; use http::{Method, ContentType, Header, HeaderMap, Cookie, Cookies}; +use http::CookieJar; + use http::hyper; /// The type of an incoming web request. @@ -29,7 +31,7 @@ pub struct Request<'r> { headers: HeaderMap<'r>, remote: Option, params: RefCell>, - cookies: Cookies, + cookies: RefCell, state: Option<&'r Container>, } @@ -54,7 +56,7 @@ impl<'r> Request<'r> { headers: HeaderMap::new(), remote: None, params: RefCell::new(Vec::new()), - cookies: Cookies::new(&[]), + cookies: RefCell::new(CookieJar::new()), state: None } } @@ -251,15 +253,24 @@ impl<'r> Request<'r> { /// request.cookies().add(Cookie::new("key", "val")); /// request.cookies().add(Cookie::new("ans", format!("life: {}", 38 + 4))); /// ``` - #[inline(always)] - pub fn cookies(&self) -> &Cookies { - &self.cookies + #[inline] + pub fn cookies(&self) -> Cookies { + match self.cookies.try_borrow_mut() { + Ok(jar) => Cookies::from(jar), + Err(_) => { + error_!("Multiple `Cookies` instances are active at once."); + info_!("An instance of `Cookies` must be dropped before another \ + can be retrieved."); + warn_!("The retrieved `Cookies` instance will be empty."); + Cookies::empty() + } + } } /// Replace all of the cookies in `self` with `cookies`. #[inline] - pub(crate) fn set_cookies(&mut self, cookies: Cookies) { - self.cookies = cookies; + pub(crate) fn set_cookies(&mut self, jar: CookieJar) { + self.cookies = RefCell::new(jar); } /// Returns `Some` of the Content-Type header of `self`. If the header is @@ -419,7 +430,7 @@ impl<'r> Request<'r> { // Set the request cookies, if they exist. TODO: Use session key. if let Some(cookie_headers) = h_headers.get_raw("Cookie") { - let mut cookies = Cookies::new(&[]); + let mut jar = CookieJar::new(); for header in cookie_headers { let raw_str = match ::std::str::from_utf8(header) { Ok(string) => string, @@ -432,11 +443,11 @@ impl<'r> Request<'r> { Err(_) => continue }; - cookies.add_original(cookie); + jar.add_original(cookie); } } - request.set_cookies(cookies); + request.set_cookies(jar); } // Set the rest of the headers. diff --git a/lib/src/response/flash.rs b/lib/src/response/flash.rs index a270f3e1..c0c5478f 100644 --- a/lib/src/response/flash.rs +++ b/lib/src/response/flash.rs @@ -1,6 +1,6 @@ use std::convert::AsRef; -use time::{self, Duration}; +use time::Duration; use outcome::IntoOutcome; use response::{Response, Responder}; @@ -184,7 +184,7 @@ impl<'r, R: Responder<'r>> Responder<'r> for Flash { trace_!("Flash: setting message: {}:{}", self.name, self.message); let cookie = self.cookie(); Response::build_from(self.responder.respond()?) - .header_adjoin(cookie) + .header_adjoin(&cookie) .ok() } } @@ -220,18 +220,12 @@ impl<'a, 'r> FromRequest<'a, 'r> for Flash<()> { fn from_request(request: &'a Request<'r>) -> request::Outcome { trace_!("Flash: attemping to retrieve message."); - let r = request.cookies().find(FLASH_COOKIE_NAME).ok_or(()).and_then(|cookie| { + let r = request.cookies().get(FLASH_COOKIE_NAME).ok_or(()).and_then(|cookie| { trace_!("Flash: retrieving message: {:?}", cookie); - // Create the "deletion" cookie. We'll use it to clear the cookie. - let delete_cookie = Cookie::build(FLASH_COOKIE_NAME, "") - .max_age(Duration::seconds(0)) - .expires(time::now() - Duration::days(365)) - .path("/") - .finish(); - - // Add the deletion to the cookie jar, replacing the existing cookie. - request.cookies().add(delete_cookie); + // Delete the flash cookie from the jar. + let orig_cookie = Cookie::build(FLASH_COOKIE_NAME, "").path("/").finish(); + request.cookies().remove(orig_cookie); // Parse the flash message. let content = cookie.value();