fix(net): default cookie Path to the request directory (RFC 6265 5.1.4) (#403)
* fix(net): default cookie Path to the request directory (RFC 6265 5.1.4) When a Set-Cookie carried no Path attribute, the jar stored the full request path as the cookie's path. RFC 6265 5.1.4 default-path is the request URI's directory — the path up to but not including the right-most '/'. Storing the full path scoped a session cookie to the exact URL that set it: a cookie set on GET /app/login was stored with path=/app/login and then, because path-match (5.1.4, fixed earlier) requires a '/' boundary, was not sent to /app/dashboard. The user appeared logged out on the very next navigation. Add default_cookie_path() implementing the RFC algorithm and use it as the initial path in both set_cookie (HTTP) and set_cookie_from_js (document.cookie). An explicit Path attribute still overrides it. Tests cover the algorithm and the end-to-end 'cookie set on /app/login is sent to /app/dashboard' path for both the HTTP and JS entry points; both failed before this change. * fix(cdp): apply RFC 6265 default cookie Path to CDP-injected cookies Network.setCookie/setCookies (the Puppeteer page.setCookie and Playwright addCookies surface) derived the cookie Path from the full request URL when the caller omitted path, the same bug the parent commit fixed for Set-Cookie responses. Reuse default_cookie_path (now pub) so a cookie set under /v1/things scopes to /v1 and is resent on sibling paths. Fixes the CDP half of #401. Correct the test that locked in the old full-path behavior and add a case confirming an explicit Path still wins. --------- Co-authored-by: SGavrl <277843151+SGavrl@users.noreply.github.com>
This commit is contained in:
1 parent
c8c78ba229
commit
5ada3b404b
3 files changed
+100
-4
No files matched your search
@@ -31,7 +31,11 @@ pub fn parse_cdp_cookie(value: &Value) -> Option<CookieInfo> {
|
||||
.get("path")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(|s| s.to_string())
|
||||
.or_else(|| url_parsed.as_ref().map(|u| u.path().to_string()))
|
||||
.or_else(|| {
|
||||
url_parsed
|
||||
.as_ref()
|
||||
.map(|u| obscura_net::default_cookie_path(u.path()))
|
||||
})
|
||||
.unwrap_or_else(|| DEFAULT_COOKIE_PATH.to_string());
|
||||
|
||||
let secure = value.get("secure").and_then(|v| v.as_bool()).unwrap_or(false);
|
||||
@@ -118,6 +122,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn parse_with_url_fallback_for_domain_and_path() {
|
||||
// RFC 6265 5.1.4 default-path: a cookie set under /v1/things with no
|
||||
// explicit Path is scoped to the directory /v1, not the full request
|
||||
// path. Puppeteer's page.setCookie / Playwright's addCookies reach this
|
||||
// path when the caller omits `path`.
|
||||
let v = json!({
|
||||
"name": "tok",
|
||||
"value": "xyz",
|
||||
@@ -125,6 +133,19 @@ mod tests {
|
||||
});
|
||||
let c = parse_cdp_cookie(&v).unwrap();
|
||||
assert_eq!(c.domain, "api.example.com");
|
||||
assert_eq!(c.path, "/v1");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_explicit_path_overrides_default() {
|
||||
// An explicit Path attribute wins over the RFC default-path.
|
||||
let v = json!({
|
||||
"name": "tok",
|
||||
"value": "xyz",
|
||||
"url": "https://api.example.com/v1/things",
|
||||
"path": "/v1/things",
|
||||
});
|
||||
let c = parse_cdp_cookie(&v).unwrap();
|
||||
assert_eq!(c.path, "/v1/things");
|
||||
}
|
||||
|
||||
|
||||
@@ -54,7 +54,7 @@ impl CookieJar {
|
||||
|
||||
let request_host = url.host_str().unwrap_or("").to_lowercase();
|
||||
let mut domain_attr: Option<String> = None;
|
||||
let mut path = url.path().to_string();
|
||||
let mut path = default_cookie_path(url.path());
|
||||
let mut secure = false;
|
||||
let mut http_only = false;
|
||||
let mut expires: Option<u64> = None;
|
||||
@@ -282,7 +282,7 @@ impl CookieJar {
|
||||
|
||||
let request_host = url.host_str().unwrap_or("").to_lowercase();
|
||||
let mut domain_attr: Option<String> = None;
|
||||
let mut path = url.path().to_string();
|
||||
let mut path = default_cookie_path(url.path());
|
||||
let mut secure = false;
|
||||
let mut expires: Option<u64> = None;
|
||||
let mut same_site = "Lax".to_string();
|
||||
@@ -567,6 +567,25 @@ fn resolve_cookie_domain(origin_host: &str, domain_attr: Option<&str>) -> Option
|
||||
}
|
||||
}
|
||||
|
||||
// RFC 6265 5.1.4 default-path: the path a cookie is scoped to when its
|
||||
// Set-Cookie carries no Path attribute. It is the request URI's directory — the
|
||||
// path up to but not including the right-most '/' — NOT the full request path.
|
||||
// Using the full path scopes a session cookie to the exact URL that set it: a
|
||||
// cookie set on `/app/login` would then not match `/app/dashboard`, silently
|
||||
// logging the user out on the next navigation. Browsers store `/app` here.
|
||||
pub fn default_cookie_path(request_path: &str) -> String {
|
||||
// "If the uri-path is empty or if the first character is not '/', output /."
|
||||
if !request_path.starts_with('/') {
|
||||
return "/".to_string();
|
||||
}
|
||||
match request_path.rfind('/') {
|
||||
// "If the uri-path contains no more than one '/', output /."
|
||||
Some(0) | None => "/".to_string(),
|
||||
// "Output the characters up to, but not including, the right-most '/'."
|
||||
Some(idx) => request_path[..idx].to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
// RFC 6265 5.1.4 path-match. A bare `starts_with` over-matches sibling paths
|
||||
// that share a string prefix (a Path=/admin cookie leaking to /administrator),
|
||||
// so a prefix match also requires the boundary to fall on a '/'.
|
||||
@@ -986,4 +1005,60 @@ mod tests {
|
||||
let sub = Url::parse("https://example.com/admin/panel").unwrap();
|
||||
assert!(jar.get_cookie_header(&sub).contains("sess=1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_cookie_path_is_request_directory() {
|
||||
// RFC 6265 5.1.4 default-path: up to (not including) the right-most '/'.
|
||||
assert_eq!(default_cookie_path("/app/login"), "/app");
|
||||
assert_eq!(default_cookie_path("/app/"), "/app");
|
||||
assert_eq!(default_cookie_path("/a/b/c"), "/a/b");
|
||||
// No more than one '/', empty, or non-absolute -> "/".
|
||||
assert_eq!(default_cookie_path("/foo"), "/");
|
||||
assert_eq!(default_cookie_path("/"), "/");
|
||||
assert_eq!(default_cookie_path(""), "/");
|
||||
assert_eq!(default_cookie_path("relative"), "/");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cookie_without_path_defaults_to_directory_not_full_path() {
|
||||
// A Set-Cookie with no Path attribute on /app/login must scope to /app
|
||||
// (RFC 6265 5.1.4), so the session survives navigation to /app/dashboard.
|
||||
// Before this fix it was scoped to the full path /app/login and vanished
|
||||
// on the next page, appearing as a silent logout.
|
||||
let jar = CookieJar::new();
|
||||
let login = Url::parse("https://example.com/app/login").unwrap();
|
||||
jar.set_cookie("sid=abc", &login);
|
||||
|
||||
let dashboard = Url::parse("https://example.com/app/dashboard").unwrap();
|
||||
assert!(
|
||||
jar.get_cookie_header(&dashboard).contains("sid=abc"),
|
||||
"session cookie was not sent to a sibling path under the same directory: {}",
|
||||
jar.get_cookie_header(&dashboard)
|
||||
);
|
||||
// Still sent at the directory root and the original path.
|
||||
let app_root = Url::parse("https://example.com/app/").unwrap();
|
||||
assert!(jar.get_cookie_header(&app_root).contains("sid=abc"));
|
||||
assert!(jar.get_cookie_header(&login).contains("sid=abc"));
|
||||
|
||||
// But not to an unrelated top-level path outside the directory.
|
||||
let other = Url::parse("https://example.com/other").unwrap();
|
||||
assert!(
|
||||
!jar.get_cookie_header(&other).contains("sid=abc"),
|
||||
"cookie leaked outside its default-path directory"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn js_cookie_without_path_also_defaults_to_directory() {
|
||||
// document.cookie set on /shop/cart with no path must reach /shop/checkout.
|
||||
let jar = CookieJar::new();
|
||||
let cart = Url::parse("https://example.com/shop/cart").unwrap();
|
||||
jar.set_cookie_from_js("cart=xyz", &cart);
|
||||
let checkout = Url::parse("https://example.com/shop/checkout").unwrap();
|
||||
assert!(
|
||||
jar.get_js_visible_cookies(&checkout).contains("cart=xyz"),
|
||||
"JS cookie not visible at sibling path: {}",
|
||||
jar.get_js_visible_cookies(&checkout)
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -11,7 +11,7 @@ pub use client::{
|
||||
env_allows_private_network, is_forbidden_ip, ObscuraHttpClient, ObscuraNetError, RequestCallback,
|
||||
RequestInfo, ResourceType, Response, ResponseCallback, SsrfGuardResolver,
|
||||
};
|
||||
pub use cookies::{CookieInfo, CookieJar};
|
||||
pub use cookies::{default_cookie_path, CookieInfo, CookieJar};
|
||||
pub use encoding::{
|
||||
decode_non_html, decode_response, decode_response_with_name, decode_with_label, label_name,
|
||||
url_encode_query,
|
||||
|
||||
Reference in new issue
Block a user