Skip to content

Commit

Permalink
Replace chrono with httpdate internally (#361)
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` (added in #349), 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.

This has been the only thing holding back a 1.6.0 release with #349 in it, since CVE-2020-26235 was brought to wider attention shortly after its merge and I've been thinking on how to best address that concern before the API is released and can't be changed without a BC break.
  • Loading branch information
sagebind committed Nov 8, 2021
1 parent 9cde92d commit 46a6593
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 33 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
71 changes: 54 additions & 17 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 All @@ -16,14 +20,14 @@ impl Error for ParseError {}
/// Builder for a [`Cookie`].
///
/// ```rust
/// use chrono::{Utc, Duration};
/// use isahc::cookies::Cookie;
/// use std::time::{Duration, SystemTime};
///
/// let cookie: Cookie = Cookie::builder("name", "value") // or CookieBuilder::new("name", "value")
/// .domain("example.com")
/// .path("/")
/// .secure(true)
/// .expiration(Utc::now() + Duration::minutes(30))
/// .expiration(SystemTime::now() + Duration::from_secs(30 * 60))
/// .build()
/// .unwrap();
/// ```
Expand All @@ -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 46a6593

Please sign in to comment.