Improve and fix default_path cookie algorithm

* Previously, the function returned an owned String, which is not
  necessary, so now it returns a slice
* Steps have now been documented/labeled
* The last step of the algorithm was incorrect; it would only slice the
  path if the "/" was the last character, which is not what the spec
  says. The spec says to slice up until (but not including) the last
  "/". Also added a regression test for this.

1 files changed, 16 insertions, 9 deletions

diff --git a/components/net/cookie.rs b/components/net/cookie.rs
index 46094ac00a3..3c56fe8bac6 100644
--- a/components/net/cookie.rs
+++ b/components/net/cookie.rs
@@ -71,7 +71,7 @@ impl Cookie {
         if path.is_empty() || path.char_at(0) != '/' {
             let url_path = request.serialize_path();
             let url_path = url_path.as_ref().map(|path| &**path);
-            path = Cookie::default_path(url_path.unwrap_or(""));
+            path = Cookie::default_path(url_path.unwrap_or("")).to_owned();
         }
         cookie.path = Some(path);
 
@@ -96,15 +96,21 @@ impl Cookie {
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.1.4
-    fn default_path(request_path: &str) -> String {
-        if request_path == "" || request_path.char_at(0) != '/' ||
-           request_path.chars().filter(|&c| c == '/').count() == 1 {
-            "/".to_owned()
-        } else if request_path.ends_with("/") {
-            request_path[..request_path.len() - 1].to_owned()
-        } else {
-            request_path.to_owned()
+    fn default_path(request_path: &str) -> &str {
+        // Step 2
+        if request_path.is_empty() || !request_path.starts_with("/") {
+            return "/";
+        }
+
+        // Step 3
+        let rightmost_slash_idx = request_path.rfind("/").unwrap();
+        if rightmost_slash_idx == 0 {
+            // There's only one slash; it's the first character
+            return "/";
         }
+
+        // Step 4
+        &request_path[..rightmost_slash_idx]
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.1.4
@@ -180,6 +186,7 @@ fn test_domain_match() {
 #[test]
 fn test_default_path() {
     assert!(&*Cookie::default_path("/foo/bar/baz/") == "/foo/bar/baz");
+    assert!(&*Cookie::default_path("/foo/bar/baz") == "/foo/bar");
     assert!(&*Cookie::default_path("/foo/") == "/foo");
     assert!(&*Cookie::default_path("/foo") == "/");
     assert!(&*Cookie::default_path("/") == "/");