From 6f333a8e299d84c98a07a0b708fe32f40aeeeb72 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Fri, 30 Aug 2024 19:15:47 +0200 Subject: [PATCH] net: Stop using both versions of the `time` crate in the cookie code (#33260) `std::time` is good enough for us here. `cookie` is using `time 0.3`, but Servo can convert to standard library types when getting data from `cookie`. This reduces our direct dependencies and removes more use of the very old `time 0.1` series. Signed-off-by: Martin Robinson --- Cargo.lock | 4 --- Cargo.toml | 1 - components/hyper_serde/Cargo.toml | 2 -- components/hyper_serde/lib.rs | 39 ----------------------- components/hyper_serde/tests/supported.rs | 2 -- components/hyper_serde/tests/tokens.rs | 16 ++-------- components/net/Cargo.toml | 1 - components/net/cookie.rs | 33 ++++++++----------- components/net/cookie_storage.rs | 26 +++++++-------- 9 files changed, 29 insertions(+), 95 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bffc14bebc1..39dc7b48f3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1277,7 +1277,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b42b6fa04a440b495c8b04d0e71b707c585f83cb9cb28cf8cd0d976c315e31b4" dependencies = [ "powerfmt", - "serde", ] [[package]] @@ -2967,8 +2966,6 @@ dependencies = [ "serde", "serde_bytes", "serde_test", - "time 0.1.45", - "time 0.3.36", ] [[package]] @@ -4518,7 +4515,6 @@ dependencies = [ "servo_url", "sha2", "time 0.1.45", - "time 0.3.36", "tokio", "tokio-rustls", "tokio-stream", diff --git a/Cargo.toml b/Cargo.toml index b85c887e860..0ae2b5bc518 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -121,7 +121,6 @@ syn = { version = "2", default-features = false, features = ["clone-impls", "der synstructure = "0.13" thin-vec = "0.2.13" time = "0.1.41" -time_03 = { package = "time", version = "0.3", features = ["serde"] } to_shmem = { git = "https://github.com/servo/stylo", branch = "2024-07-16" } tokio = "1" tokio-rustls = "0.24" diff --git a/components/hyper_serde/Cargo.toml b/components/hyper_serde/Cargo.toml index d41a8f03ca0..5e960e26344 100644 --- a/components/hyper_serde/Cargo.toml +++ b/components/hyper_serde/Cargo.toml @@ -23,8 +23,6 @@ hyper = { workspace = true } mime = { workspace = true } serde = { workspace = true } serde_bytes = { workspace = true } -time = { workspace = true } -time_03 = { workspace = true } [dev-dependencies] serde_test = "1.0" diff --git a/components/hyper_serde/lib.rs b/components/hyper_serde/lib.rs index bb48b1c47e5..58c54ae9f7c 100644 --- a/components/hyper_serde/lib.rs +++ b/components/hyper_serde/lib.rs @@ -20,7 +20,6 @@ //! * `hyper::Method` //! * `hyper::Uri` //! * `mime::Mime` -//! * `time::Tm` //! //! # How do I use a data type with a `HeaderMap` member with Serde? //! @@ -78,7 +77,6 @@ use serde::de::{self, Error, MapAccess, SeqAccess, Visitor}; use serde::ser::{SerializeMap, SerializeSeq}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use serde_bytes::{ByteBuf, Bytes}; -use time::{strptime, Tm}; /// Deserialises a `T` value with a given deserializer. /// @@ -604,43 +602,6 @@ impl<'de> Visitor<'de> for StatusVisitor { } } -impl<'de> Deserialize<'de> for De { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - struct TmVisitor; - - impl<'de> Visitor<'de> for TmVisitor { - type Value = De; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - write!(formatter, "a date and time according to RFC 3339") - } - - fn visit_str(self, v: &str) -> Result - where - E: de::Error, - { - strptime(v, "%Y-%m-%dT%H:%M:%SZ") - .map(De::new) - .map_err(|e| E::custom(e.to_string())) - } - } - - deserializer.deserialize_string(TmVisitor) - } -} - -impl<'a> Serialize for Ser<'a, Tm> { - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - serializer.serialize_str(&self.v.rfc3339().to_string()) - } -} - impl<'de> Deserialize<'de> for De { fn deserialize(deserializer: D) -> Result where diff --git a/components/hyper_serde/tests/supported.rs b/components/hyper_serde/tests/supported.rs index 54480903c7e..a634dfe67e7 100644 --- a/components/hyper_serde/tests/supported.rs +++ b/components/hyper_serde/tests/supported.rs @@ -15,7 +15,6 @@ use hyper::{Method, StatusCode, Uri}; use hyper_serde::{De, Ser, Serde}; use mime::Mime; use serde::{Deserialize, Serialize}; -use time::Tm; fn is_supported() where @@ -33,6 +32,5 @@ fn supported() { is_supported::(); is_supported::(); is_supported::(); - is_supported::(); is_supported::(); } diff --git a/components/hyper_serde/tests/tokens.rs b/components/hyper_serde/tests/tokens.rs index 3192f1043ff..dc0b51bb901 100644 --- a/components/hyper_serde/tests/tokens.rs +++ b/components/hyper_serde/tests/tokens.rs @@ -8,6 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::time::Duration; + use cookie::{Cookie, CookieBuilder}; use headers::ContentType; use http::header::{self, HeaderMap, HeaderValue}; @@ -15,7 +17,6 @@ use http::StatusCode; use hyper::{Method, Uri}; use hyper_serde::{De, Ser}; use serde_test::{assert_de_tokens, assert_ser_tokens, Token}; -use time_03::Duration; #[test] fn test_content_type() { @@ -32,7 +33,7 @@ fn test_cookie() { // string with a bunch of indices in it which apparently is different from the exact same // cookie but parsed as a bunch of strings. let cookie: Cookie = CookieBuilder::new("Hello", "World!") - .max_age(Duration::seconds(42)) + .max_age(Duration::from_secs(42).try_into().unwrap_or_default()) .domain("servo.org") .path("/") .secure(true) @@ -111,17 +112,6 @@ fn test_raw_status() { assert_de_tokens(&De::new(raw_status), tokens); } -#[test] -fn test_tm() { - use time::strptime; - - let time = strptime("2017-02-22T12:03:31Z", "%Y-%m-%dT%H:%M:%SZ").unwrap(); - let tokens = &[Token::Str("2017-02-22T12:03:31Z")]; - - assert_ser_tokens(&Ser::new(&time), tokens); - assert_de_tokens(&De::new(time), tokens); -} - #[test] fn test_uri() { use std::str::FromStr; diff --git a/components/net/Cargo.toml b/components/net/Cargo.toml index 653a5d3387e..5495c515e8f 100644 --- a/components/net/Cargo.toml +++ b/components/net/Cargo.toml @@ -57,7 +57,6 @@ servo_config = { path = "../config" } servo_url = { path = "../url" } sha2 = "0.10" time = { workspace = true } -time_03 = { workspace = true } chrono = { workspace = true } tokio = { workspace = true, features = ["sync", "macros", "rt-multi-thread"] } tokio-rustls = { workspace = true } diff --git a/components/net/cookie.rs b/components/net/cookie.rs index 647a00fa3b0..1a6ec3ed7c7 100644 --- a/components/net/cookie.rs +++ b/components/net/cookie.rs @@ -7,14 +7,13 @@ use std::borrow::ToOwned; use std::net::{Ipv4Addr, Ipv6Addr}; +use std::time::{Duration, SystemTime}; use cookie::Cookie; use net_traits::pub_domains::is_pub_domain; use net_traits::CookieSource; use serde::{Deserialize, Serialize}; use servo_url::ServoUrl; -use time::{now, Tm}; -use time_03::OffsetDateTime; /// A stored cookie that wraps the definition in cookie-rs. This is used to implement /// various behaviours defined in the spec that rely on an associated request URL, @@ -28,17 +27,9 @@ pub struct ServoCookie { pub cookie: Cookie<'static>, pub host_only: bool, pub persistent: bool, - #[serde( - deserialize_with = "hyper_serde::deserialize", - serialize_with = "hyper_serde::serialize" - )] - pub creation_time: Tm, - #[serde( - deserialize_with = "hyper_serde::deserialize", - serialize_with = "hyper_serde::serialize" - )] - pub last_access: Tm, - pub expiry_time: Option, + pub creation_time: SystemTime, + pub last_access: SystemTime, + pub expiry_time: Option, } impl ServoCookie { @@ -60,9 +51,11 @@ impl ServoCookie { source: CookieSource, ) -> Option { // Step 3 - let (persistent, expiry_time) = match (cookie.max_age(), cookie.expires_datetime()) { - (Some(max_age), _) => (true, Some(time_03::OffsetDateTime::now_utc() + max_age)), - (_, Some(date_time)) => (true, Some(date_time)), + let max_age: Option = + cookie.max_age().and_then(|max_age| max_age.try_into().ok()); + let (persistent, expiry_time) = match (max_age, cookie.expires_datetime()) { + (Some(max_age), _) => (true, Some(SystemTime::now() + max_age)), + (_, Some(date_time)) => (true, Some(date_time.into())), _ => (false, None), }; @@ -131,18 +124,18 @@ impl ServoCookie { cookie, host_only, persistent, - creation_time: now(), - last_access: now(), + creation_time: SystemTime::now(), + last_access: SystemTime::now(), expiry_time, }) } pub fn touch(&mut self) { - self.last_access = now(); + self.last_access = SystemTime::now(); } pub fn set_expiry_time_in_past(&mut self) { - self.expiry_time = Some(time_03::OffsetDateTime::UNIX_EPOCH); + self.expiry_time = Some(SystemTime::UNIX_EPOCH) } // http://tools.ietf.org/html/rfc6265#section-5.1.4 diff --git a/components/net/cookie_storage.rs b/components/net/cookie_storage.rs index a81c23b3c60..be20cf2412f 100644 --- a/components/net/cookie_storage.rs +++ b/components/net/cookie_storage.rs @@ -8,14 +8,13 @@ use std::cmp::Ordering; use std::collections::hash_map::Entry; use std::collections::HashMap; +use std::time::SystemTime; use log::{debug, info}; use net_traits::pub_domains::reg_suffix; use net_traits::CookieSource; use serde::{Deserialize, Serialize}; use servo_url::ServoUrl; -use time::Tm; -use time_03::OffsetDateTime; use crate::cookie::ServoCookie; @@ -145,11 +144,7 @@ impl CookieStorage { let a_path_len = a.cookie.path().as_ref().map_or(0, |p| p.len()); let b_path_len = b.cookie.path().as_ref().map_or(0, |p| p.len()); match a_path_len.cmp(&b_path_len) { - Ordering::Equal => { - let a_creation_time = a.creation_time.to_timespec(); - let b_creation_time = b.creation_time.to_timespec(); - a_creation_time.cmp(&b_creation_time) - }, + Ordering::Equal => a.creation_time.cmp(&b.creation_time), // Ensure that longer paths are sorted earlier than shorter paths Ordering::Greater => Ordering::Less, Ordering::Less => Ordering::Greater, @@ -235,12 +230,12 @@ fn reg_host(url: &str) -> String { } fn is_cookie_expired(cookie: &ServoCookie) -> bool { - matches!(cookie.expiry_time, Some(date_time) if date_time <= OffsetDateTime::now_utc()) + matches!(cookie.expiry_time, Some(date_time) if date_time <= SystemTime::now()) } fn evict_one_cookie(is_secure_cookie: bool, cookies: &mut Vec) -> bool { // Remove non-secure cookie with oldest access time - let oldest_accessed: Option<(usize, Tm)> = get_oldest_accessed(false, cookies); + let oldest_accessed = get_oldest_accessed(false, cookies); if let Some((index, _)) = oldest_accessed { cookies.remove(index); @@ -249,7 +244,7 @@ fn evict_one_cookie(is_secure_cookie: bool, cookies: &mut Vec) -> b if !is_secure_cookie { return false; } - let oldest_accessed: Option<(usize, Tm)> = get_oldest_accessed(true, cookies); + let oldest_accessed = get_oldest_accessed(true, cookies); if let Some((index, _)) = oldest_accessed { cookies.remove(index); } @@ -257,13 +252,18 @@ fn evict_one_cookie(is_secure_cookie: bool, cookies: &mut Vec) -> b true } -fn get_oldest_accessed(is_secure_cookie: bool, cookies: &mut [ServoCookie]) -> Option<(usize, Tm)> { - let mut oldest_accessed: Option<(usize, Tm)> = None; +fn get_oldest_accessed( + is_secure_cookie: bool, + cookies: &mut [ServoCookie], +) -> Option<(usize, SystemTime)> { + let mut oldest_accessed = None; for (i, c) in cookies.iter().enumerate() { if (c.cookie.secure().unwrap_or(false) == is_secure_cookie) && oldest_accessed .as_ref() - .map_or(true, |a| c.last_access < a.1) + .map_or(true, |(_, current_oldest_time)| { + c.last_access < *current_oldest_time + }) { oldest_accessed = Some((i, c.last_access)); }