Skip to content

Commit

Permalink
Replace chrono with httpdate internally
Browse files Browse the repository at this point in the history
Remove chrono as a dependency to avoid any connection to the transitive [CVE-2020-26235 in `time` 0.2](https://github.com/rustsec/advisory-db/blob/9e93a3df4a54e70f2539a2ecdc3d70beef64c856/crates/time/RUSTSEC-2020-0071.md). This also requires removing chrono from the public API of `CookieBuilder`, which is probably for the best anyway before it is released so that we aren't tied to it. Expose `SystemTime` types instead, which both chrono and `time` are compatible with, should the consumer of the API choose to use either of those crates.

Replace formatting and parsing of HTTP date formats with the aptly-named    `httpdate` crate, which implements exactly those two operations and no more. As a side benefit this reduces our total transitive dependency count by 3 when the `cookies` feature is enabled.
  • Loading branch information
sagebind committed Nov 8, 2021
1 parent 9cde92d commit 30766bc
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 31 deletions.
12 changes: 6 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ status = "actively-developed"

[features]
default = ["http2", "static-curl", "text-decoding"]
cookies = ["chrono"]
cookies = ["httpdate"]
http2 = ["curl/http2"]
json = ["serde", "serde_json"]
psl = ["parking_lot", "publicsuffix"]
psl = ["httpdate", "parking_lot", "publicsuffix"]
spnego = ["curl-sys/spnego"]
static-curl = ["curl/static-curl"]
static-ssl = ["curl/static-ssl"]
Expand All @@ -47,14 +47,14 @@ sluice = "0.5.4"
url = "2.2"
waker-fn = "1"

[dependencies.chrono]
version = "0.4"
optional = true

[dependencies.encoding_rs]
version = "0.8"
optional = true

[dependencies.httpdate]
version = "1"
optional = true

[dependencies.mime]
version = "0.3"
optional = true
Expand Down
67 changes: 52 additions & 15 deletions src/cookies/cookie.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use chrono::{prelude::*, Duration};
use std::{error::Error, fmt, str};
use std::{
error::Error,
fmt,
str,
time::{Duration, SystemTime},
};

/// An error which can occur when attempting to parse a cookie string.
#[derive(Debug)]
Expand Down Expand Up @@ -46,7 +50,7 @@ pub struct CookieBuilder {

/// Time when this cookie expires. If not present, then this is a session
/// cookie that expires when the current client session ends.
expiration: Option<DateTime<Utc>>,
expiration: Option<SystemTime>,
}

impl CookieBuilder {
Expand Down Expand Up @@ -93,8 +97,11 @@ impl CookieBuilder {

/// Time when this cookie expires. If not present, then this is a session
/// cookie that expires when the current client session ends.
pub fn expiration(mut self, expiration: DateTime<Utc>) -> Self {
self.expiration = Some(expiration);
pub fn expiration<T>(mut self, expiration: T) -> Self
where
T: Into<SystemTime>,
{
self.expiration = Some(expiration.into());
self
}

Expand Down Expand Up @@ -160,7 +167,7 @@ pub struct Cookie {

/// Time when this cookie expires. If not present, then this is a session
/// cookie that expires when the current client session ends.
expiration: Option<DateTime<Utc>>,
expiration: Option<SystemTime>,
}

impl Cookie {
Expand Down Expand Up @@ -260,9 +267,10 @@ impl Cookie {

/// Check if the cookie has expired.
pub(crate) fn is_expired(&self) -> bool {
match self.expiration {
Some(time) => time < Utc::now(),
None => false,
if let Some(time) = self.expiration.as_ref() {
*time < SystemTime::now()
} else {
false
}
}

Expand All @@ -289,8 +297,8 @@ impl Cookie {
if name.eq_ignore_ascii_case(b"Expires") {
if cookie_expiration.is_none() {
if let Ok(value) = str::from_utf8(value) {
if let Ok(time) = DateTime::parse_from_rfc2822(value) {
cookie_expiration = Some(time.with_timezone(&Utc));
if let Ok(time) = httpdate::parse_http_date(value) {
cookie_expiration = Some(time);
}
}
}
Expand All @@ -301,7 +309,8 @@ impl Cookie {
} else if name.eq_ignore_ascii_case(b"Max-Age") {
if let Ok(value) = str::from_utf8(value) {
if let Ok(seconds) = value.parse() {
cookie_expiration = Some(Utc::now() + Duration::seconds(seconds));
cookie_expiration =
Some(SystemTime::now() + Duration::from_secs(seconds));
}
}
} else if name.eq_ignore_ascii_case(b"Path") {
Expand Down Expand Up @@ -416,6 +425,12 @@ mod tests {
use super::*;
use test_case::test_case;

fn system_time_timestamp(time: &SystemTime) -> u64 {
time.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_secs()
}

#[test_case("foo")]
#[test_case("foo;=bar")]
#[test_case("bad_name@?=bar")]
Expand Down Expand Up @@ -449,7 +464,7 @@ mod tests {
}

#[test]
fn parse_set_cookie_header() {
fn parse_set_cookie_header_expires() {
let cookie = Cookie::parse(
"foo=bar; path=/sub;Secure; DOMAIN=baz.com;expires=Wed, 21 Oct 2015 07:28:00 GMT",
)
Expand All @@ -460,15 +475,37 @@ mod tests {
assert_eq!(cookie.path(), Some("/sub"));
assert_eq!(cookie.domain.as_deref(), Some("baz.com"));
assert!(cookie.is_secure());
assert!(!cookie.is_expired());
assert_eq!(
cookie.expiration.as_ref().map(|t| t.timestamp()),
cookie.expiration.as_ref().map(system_time_timestamp),
Some(1_445_412_480)
);
}

#[test]
fn parse_set_cookie_header_max_age() {
let cookie =
Cookie::parse("foo=bar; path=/sub;Secure; DOMAIN=baz.com; max-age=60").unwrap();

assert_eq!(cookie.name(), "foo");
assert_eq!(cookie.value(), "bar");
assert_eq!(cookie.path(), Some("/sub"));
assert_eq!(cookie.domain.as_deref(), Some("baz.com"));
assert!(cookie.is_secure());
assert!(!cookie.is_expired());
assert!(
cookie
.expiration
.unwrap()
.duration_since(SystemTime::now())
.unwrap()
<= Duration::from_secs(60)
);
}

#[test]
fn create_cookie() {
let exp = Utc::now();
let exp = SystemTime::now();

let cookie = Cookie::builder("foo", "bar")
.domain("baz.com")
Expand Down
22 changes: 12 additions & 10 deletions src/cookies/psl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,21 @@
//! since a stale list is better than no list at all.
use crate::{request::RequestExt, ReadResponseExt};
use chrono::{prelude::*, Duration};
use once_cell::sync::Lazy;
use parking_lot::{RwLock, RwLockUpgradableReadGuard};
use publicsuffix::{List, Psl};
use std::error::Error;
use std::{error::Error, time::{Duration, SystemTime}};

/// How long should we use a cached list before refreshing?
static TTL: Lazy<Duration> = Lazy::new(|| Duration::hours(24));
static TTL: Lazy<Duration> = Lazy::new(|| Duration::from_secs(24 * 60 * 60));

/// Global in-memory PSL cache.
static CACHE: Lazy<RwLock<ListCache>> = Lazy::new(Default::default);

struct ListCache {
list: List,
last_refreshed: Option<DateTime<Utc>>,
last_updated: Option<DateTime<Utc>>,
last_refreshed: Option<SystemTime>,
last_updated: Option<SystemTime>,
}

impl Default for ListCache {
Expand All @@ -62,22 +61,25 @@ impl Default for ListCache {
impl ListCache {
fn needs_refreshed(&self) -> bool {
match self.last_refreshed {
Some(last_refreshed) => Utc::now() - last_refreshed > *TTL,
Some(last_refreshed) => match last_refreshed.elapsed() {
Ok(elapsed) => elapsed > *TTL,
Err(_) => false,
},
None => true,
}
}

fn refresh(&mut self) -> Result<(), Box<dyn Error>> {
let result = self.try_refresh();
self.last_refreshed = Some(Utc::now());
self.last_refreshed = Some(SystemTime::now());
result
}

fn try_refresh(&mut self) -> Result<(), Box<dyn Error>> {
let mut request = http::Request::get(publicsuffix::LIST_URL);

if let Some(last_updated) = self.last_updated {
request = request.header(http::header::IF_MODIFIED_SINCE, last_updated.to_rfc2822());
request = request.header(http::header::IF_MODIFIED_SINCE, httpdate::fmt_http_date(last_updated));
}

let mut response = request.body(())?.send()?;
Expand All @@ -86,13 +88,13 @@ impl ListCache {
http::StatusCode::OK => {
// Parse the suffix list.
self.list = response.text()?.parse()?;
self.last_updated = Some(Utc::now());
self.last_updated = Some(SystemTime::now());
tracing::debug!("public suffix list updated");
}

http::StatusCode::NOT_MODIFIED => {
// List hasn't changed and is still new.
self.last_updated = Some(Utc::now());
self.last_updated = Some(SystemTime::now());
}

status => tracing::warn!(
Expand Down

0 comments on commit 30766bc

Please sign in to comment.