Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add domain and path checks for secure cookies eviction #14491

Merged
merged 3 commits into from Dec 24, 2016
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev

Add unit tests

  • Loading branch information
KiChjang committed Dec 22, 2016
commit 16f1947e24d212612dfc4a029100aa11f2274ba6
@@ -133,6 +133,119 @@ fn test_sort_order() {
assert!(CookieStorage::cookie_comparator(&a, &a) == Ordering::Equal);
}

fn add_cookie_to_storage(storage: &mut CookieStorage, url: &ServoUrl, cookie_str: &str)
{
let source = CookieSource::HTTP;
let cookie = Cookie::new_wrapped(cookie_rs::Cookie::parse(cookie_str).unwrap(), url, source).unwrap();
storage.push(cookie, url, source);
}

#[test]
fn test_insecure_cookies_cannot_evict_secure_cookie() {
let mut storage = CookieStorage::new(5);
let secure_url = ServoUrl::parse("https://home.example.org:8888/cookie-parser?0001").unwrap();
let source = CookieSource::HTTP;
let mut cookies = Vec::new();

cookies.push(cookie_rs::Cookie::parse("foo=bar; Secure; Domain=home.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo2=bar; Secure; Domain=.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo3=bar; Secure; Path=/foo").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo4=bar; Secure; Path=/foo/bar").unwrap());

for bare_cookie in cookies {
let cookie = Cookie::new_wrapped(bare_cookie, &secure_url, source).unwrap();
storage.push(cookie, &secure_url, source);
}

let insecure_url = ServoUrl::parse("http://home.example.org:8888/cookie-parser?0001").unwrap();

add_cookie_to_storage(&mut storage, &insecure_url, "foo=value; Domain=home.example.org");
add_cookie_to_storage(&mut storage, &insecure_url, "foo2=value; Domain=.example.org");
add_cookie_to_storage(&mut storage, &insecure_url, "foo3=value; Path=/foo/bar");
add_cookie_to_storage(&mut storage, &insecure_url, "foo4=value; Path=/foo");

let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&secure_url, source).unwrap(), "foo=bar; foo2=bar");

let url = ServoUrl::parse("https://home.example.org:8888/foo/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo3=bar; foo4=value; foo=bar; foo2=bar");

let url = ServoUrl::parse("https://home.example.org:8888/foo/bar/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo4=bar; foo3=bar; foo4=value; foo=bar; foo2=bar");
}

#[test]
fn test_secure_cookies_eviction() {
let mut storage = CookieStorage::new(5);
let url = ServoUrl::parse("https://home.example.org:8888/cookie-parser?0001").unwrap();

This comment has been minimized.

@jdm

jdm Dec 15, 2016

Member

Let's call this secure_url and reuse it later.

let source = CookieSource::HTTP;
let mut cookies = Vec::new();

cookies.push(cookie_rs::Cookie::parse("foo=bar; Secure; Domain=home.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo2=bar; Secure; Domain=.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo3=bar; Secure; Path=/foo").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo4=bar; Secure; Path=/foo/bar").unwrap());

for bare_cookie in cookies {
let cookie = Cookie::new_wrapped(bare_cookie, &url, source).unwrap();
storage.push(cookie, &url, source);
}

add_cookie_to_storage(&mut storage, &url, "foo=value; Domain=home.example.org");
add_cookie_to_storage(&mut storage, &url, "foo2=value; Domain=.example.org");
add_cookie_to_storage(&mut storage, &url, "foo3=value; Path=/foo/bar");
add_cookie_to_storage(&mut storage, &url, "foo4=value; Path=/foo");

let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo2=value");

let url = ServoUrl::parse("https://home.example.org:8888/foo/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo3=bar; foo4=value; foo2=value");

let url = ServoUrl::parse("https://home.example.org:8888/foo/bar/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(),
"foo4=bar; foo3=value; foo3=bar; foo4=value; foo2=value");
}

#[test]
fn test_secure_cookies_eviction_non_http_source() {
let mut storage = CookieStorage::new(5);
let url = ServoUrl::parse("https://home.example.org:8888/cookie-parser?0001").unwrap();
let source = CookieSource::NonHTTP;
let mut cookies = Vec::new();

cookies.push(cookie_rs::Cookie::parse("foo=bar; Secure; Domain=home.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo2=bar; Secure; Domain=.example.org").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo3=bar; Secure; Path=/foo").unwrap());
cookies.push(cookie_rs::Cookie::parse("foo4=bar; Secure; Path=/foo/bar").unwrap());

for bare_cookie in cookies {
let cookie = Cookie::new_wrapped(bare_cookie, &url, source).unwrap();
storage.push(cookie, &url, source);
}

add_cookie_to_storage(&mut storage, &url, "foo=value; Domain=home.example.org");
add_cookie_to_storage(&mut storage, &url, "foo2=value; Domain=.example.org");
add_cookie_to_storage(&mut storage, &url, "foo3=value; Path=/foo/bar");
add_cookie_to_storage(&mut storage, &url, "foo4=value; Path=/foo");

This comment has been minimized.

@jdm

jdm Dec 15, 2016

Member

We need another test that tries setting these cookies using a secure scheme, too.


let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo2=value");

let url = ServoUrl::parse("https://home.example.org:8888/foo/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(), "foo3=bar; foo4=value; foo2=value");

let url = ServoUrl::parse("https://home.example.org:8888/foo/bar/cookie-parser-result?0001").unwrap();
let source = CookieSource::HTTP;
assert_eq!(storage.cookies_for_url(&url, source).unwrap(),
"foo4=bar; foo3=value; foo3=bar; foo4=value; foo2=value");
}

This comment has been minimized.

@jdm

jdm Dec 15, 2016

Member

We should add a test that it's still possible to set secure cookies using a NonHTTP source and a secure URL.


fn add_retrieve_cookies(set_location: &str,
set_cookies: &[String],
@@ -149,7 +262,7 @@ fn add_retrieve_cookies(set_location: &str,
let SetCookie(cookies) = header;
for bare_cookie in cookies {
let cookie = Cookie::new_wrapped(bare_cookie, &url, source).unwrap();
storage.push(cookie, source);
storage.push(cookie, &url, source);
}
}

@@ -21,7 +21,7 @@ fn run(set_location: &str, set_cookies: &[&str], final_location: &str) -> String
if let Ok(SetCookie(cookies)) = header {
for bare_cookie in cookies {
if let Some(cookie) = Cookie::new_wrapped(bare_cookie, &url, source) {
storage.push(cookie, source);
storage.push(cookie, &url, source);
}
}
}
@@ -552,7 +552,7 @@ fn test_load_sets_requests_cookies_header_for_url_by_getting_cookies_from_the_re
&url,
CookieSource::HTTP
).unwrap();
cookie_jar.push(cookie, CookieSource::HTTP);
cookie_jar.push(cookie, &url, CookieSource::HTTP);
}

let request = Request::from_init(RequestInit {
@@ -590,7 +590,7 @@ fn test_load_sends_cookie_if_nonhttp() {
&url,
CookieSource::NonHTTP
).unwrap();
cookie_jar.push(cookie, CookieSource::HTTP);
cookie_jar.push(cookie, &url, CookieSource::HTTP);
}

let request = Request::from_init(RequestInit {
@@ -982,14 +982,14 @@ fn test_redirect_from_x_to_y_provides_y_cookies_from_y() {
CookieSource::HTTP
).unwrap();

cookie_jar.push(cookie_x, CookieSource::HTTP);
cookie_jar.push(cookie_x, &url_x, CookieSource::HTTP);

let cookie_y = Cookie::new_wrapped(
CookiePair::new("mozillaIs".to_owned(), "theBest".to_owned()),
&url_y,
CookieSource::HTTP
).unwrap();
cookie_jar.push(cookie_y, CookieSource::HTTP);
cookie_jar.push(cookie_y, &url_y, CookieSource::HTTP);
}

let request = Request::from_init(RequestInit {
@@ -1175,7 +1175,7 @@ fn test_cookies_blocked() {
&url,
CookieSource::HTTP
).unwrap();
cookie_jar.push(cookie, CookieSource::HTTP);
cookie_jar.push(cookie, &url, CookieSource::HTTP);
}

let request = Request::from_init(RequestInit {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.