From f13311a00e88a282608e23f2404393b44ac1414c Mon Sep 17 00:00:00 2001 From: Sebastian C Date: Mon, 14 Jul 2025 17:15:27 -0500 Subject: [PATCH] Refactor and document cookie-list retrieval (#38070) This refactors some of the cookie retrieval mechanism to be less repetitive and separates the cookie-list from the cookie-string which will also be needed for Cookie Store. Testing: No new behavior, should be covered by existing WPT tests. Signed-off-by: Sebastian C --- components/net/cookie.rs | 21 +++++++++++++ components/net/cookie_storage.rs | 52 ++++++++++++++------------------ 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/components/net/cookie.rs b/components/net/cookie.rs index 2d44bdedaa9..986a02a07a3 100644 --- a/components/net/cookie.rs +++ b/components/net/cookie.rs @@ -10,6 +10,7 @@ use std::net::{Ipv4Addr, Ipv6Addr}; use std::time::SystemTime; use cookie::Cookie; +use log::{Level, debug, log_enabled}; use net_traits::CookieSource; use net_traits::pub_domains::is_pub_domain; use nom::IResult; @@ -322,7 +323,21 @@ impl ServoCookie { /// step 1 pub fn appropriate_for_url(&self, url: &ServoUrl, source: CookieSource) -> bool { + if log_enabled!(Level::Debug) { + debug!( + " === SENT COOKIE : {} {} {:?} {:?}", + self.cookie.name(), + self.cookie.value(), + self.cookie.domain(), + self.cookie.path() + ); + } + let domain = url.host_str(); + // Either: The cookie's host-only-flag is true and the canonicalized host of the + // retrieval's URI is identical to the cookie's domain + // Or: The cookie's host-only-flag is false and the canonicalized host of the + // retrieval's URI domain-matches the cookie's domain if self.host_only { if self.cookie.domain() != domain { return false; @@ -333,18 +348,24 @@ impl ServoCookie { } } + // The retrieval's URI's path path-matches the cookie's path. if let Some(cookie_path) = self.cookie.path() { if !ServoCookie::path_match(url.path(), cookie_path) { return false; } } + // If the cookie's secure-only-flag is true, then the retrieval's URI must denote a "secure" connection if self.cookie.secure().unwrap_or(false) && !url.is_secure_scheme() { return false; } + + // If the cookie's http-only-flag is true, then exclude the cookie if the retrieval's type is "non-HTTP" if self.cookie.http_only().unwrap_or(false) && source == CookieSource::NonHTTP { return false; } + // TODO: Apply same site checks + // TOOD: Apply Partitioning checks true } diff --git a/components/net/cookie_storage.rs b/components/net/cookie_storage.rs index f158fd035e7..e37e8ac1619 100644 --- a/components/net/cookie_storage.rs +++ b/components/net/cookie_storage.rs @@ -10,7 +10,9 @@ use std::collections::HashMap; use std::collections::hash_map::Entry; use std::time::SystemTime; -use log::{debug, info}; +use cookie::Cookie; +use itertools::Itertools; +use log::info; use net_traits::CookieSource; use net_traits::pub_domains::reg_suffix; use serde::{Deserialize, Serialize}; @@ -175,41 +177,26 @@ impl CookieStorage { // http://tools.ietf.org/html/rfc6265#section-5.4 pub fn cookies_for_url(&mut self, url: &ServoUrl, source: CookieSource) -> Option { - let filterer = |c: &&mut ServoCookie| -> bool { - debug!( - " === SENT COOKIE : {} {} {:?} {:?}", - c.cookie.name(), - c.cookie.value(), - c.cookie.domain(), - c.cookie.path() - ); - debug!( - " === SENT COOKIE RESULT {}", - c.appropriate_for_url(url, source) - ); - // Step 1 - c.appropriate_for_url(url, source) - }; - // Step 2 - let domain = reg_host(url.host_str().unwrap_or("")); - let cookies = self.cookies_map.entry(domain).or_default(); + // Let cookie-list be the set of cookies from the cookie store + let cookie_list = self.cookies_data_for_url(url, source); - let mut url_cookies: Vec<&mut ServoCookie> = cookies.iter_mut().filter(filterer).collect(); - url_cookies.sort_by(|a, b| CookieStorage::cookie_comparator(a, b)); - - let reducer = |acc: String, c: &mut &mut ServoCookie| -> String { - // Step 3 - c.touch(); - - // Step 4 + let reducer = |acc: String, cookie: Cookie<'static>| -> String { + // Serialize the cookie-list into a cookie-string by processing each cookie in the cookie-list in order: + // If the cookies' name is not empty, output the cookie's name followed by the %x3D ("=") character. + // If the cookies' value is not empty, output the cookie's value. + // If there is an unprocessed cookie in the cookie-list, output the characters %x3B and %x20 ("; "). + // Security: the above steps allow for "nameless" cookies which have proved to be a security footgun + // especially with the new cookie name prefix proposals (match acc.len() { 0 => acc, _ => acc + "; ", - }) + c.cookie.name() + + }) + cookie.name() + "=" + - c.cookie.value() + cookie.value() }; - let result = url_cookies.iter_mut().fold("".to_owned(), reducer); + + // Serialize the cookie-list into a cookie-string by processing each cookie in the cookie-list in order + let result = cookie_list.fold("".to_owned(), reducer); info!(" === COOKIES SENT: {}", result); match result.len() { @@ -229,7 +216,12 @@ impl CookieStorage { cookies .iter_mut() .filter(move |c| c.appropriate_for_url(url, source)) + .sorted_by(|a: &&mut ServoCookie, b: &&mut ServoCookie| { + // The user agent SHOULD sort the cookie-list + CookieStorage::cookie_comparator(a, b) + }) .map(|c| { + // Update the last-access-time of each cookie in the cookie-list to the current date and time c.touch(); c.cookie.clone() })