cookies: follow-up fix for path checking

The initial fix to only compare full path names were done in commit
04f52e9b4db0 but found out to be incomplete. This takes should make the
change more complete and there's now two additional tests to verify
(test 31 and 62).

4 files changed, 134 insertions, 22 deletions

diff --git a/lib/cookie.c b/lib/cookie.c
index 1ac97fa66..fc918136a 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -106,6 +106,8 @@ static void freecookie(struct Cookie *co)
     free(co->domain);
   if(co->path)
     free(co->path);
+  if(co->spath)
+    free(co->spath);
   if(co->name)
     free(co->name);
   if(co->value)
@@ -143,32 +145,114 @@ static bool tailmatch(const char *cooke_domain, const char *hostname)
   return FALSE;
 }
 
-static bool pathmatch(const char* cookie_path, const char* url_path)
+/*
+ * matching cookie path and url path
+ * RFC6265 5.1.4 Paths and Path-Match
+ */
+static bool pathmatch(const char* cookie_path, const char* request_uri)
 {
-  size_t cookie_path_len = strlen(cookie_path);
-  size_t url_path_len = strlen(url_path);
+  size_t cookie_path_len;
+  size_t uri_path_len;
+  char* uri_path = NULL;
+  char* pos;
+  bool ret = FALSE;
+
+  /* cookie_path must not have last '/' separator. ex: /sample */
+  cookie_path_len = strlen(cookie_path);
+  if(1 == cookie_path_len) {
+    /* cookie_path must be '/' */
+    return TRUE;
+  }
 
-  if(url_path_len < cookie_path_len)
+  uri_path = strdup(request_uri);
+  if(!uri_path)
     return FALSE;
+  pos = strchr(uri_path, '?');
+  if(pos)
+    *pos = 0x0;
+
+  /* #-fragments are already cut off! */
+  if(0 == strlen(uri_path) || uri_path[0] != '/') {
+    free(uri_path);
+    uri_path = strdup("/");
+    if(!uri_path)
+      return FALSE;
+  }
+
+  /* here, RFC6265 5.1.4 says
+     4. Output the characters of the uri-path from the first character up
+        to, but not including, the right-most %x2F ("/").
+     but URL path /hoge?fuga=xxx means /hoge/index.cgi?fuga=xxx in some site
+     without redirect.
+     Ignore this algorithm because /hoge is uri path for this case
+     (uri path is not /).
+   */
+
+  uri_path_len = strlen(uri_path);
+
+  if(uri_path_len < cookie_path_len) {
+    ret = FALSE;
+    goto pathmatched;
+  }
 
   /* not using checkprefix() because matching should be case-sensitive */
-  if(strncmp(cookie_path, url_path, cookie_path_len))
-    return FALSE;
+  if(strncmp(cookie_path, uri_path, cookie_path_len)) {
+    ret = FALSE;
+    goto pathmatched;
+  }
 
-  /* it is true if cookie_path and url_path are the same */
-  if(cookie_path_len == url_path_len)
-    return TRUE;
+  /* The cookie-path and the uri-path are identical. */
+  if(cookie_path_len == uri_path_len) {
+    ret = TRUE;
+    goto pathmatched;
+  }
 
   /* here, cookie_path_len < url_path_len */
+  if(uri_path[cookie_path_len] == '/') {
+    ret = TRUE;
+    goto pathmatched;
+  }
 
-  /* it is false if cookie path is /example and url path is /examples */
-  if(cookie_path[cookie_path_len - 1] != '/') {
-    if(url_path[cookie_path_len] != '/') {
-      return FALSE;
-    }
+  ret = FALSE;
+
+pathmatched:
+  free(uri_path);
+  return ret;
+}
+
+/*
+ * cookie path sanitize
+ */
+static char *sanitize_cookie_path(const char *cookie_path)
+{
+  size_t len;
+  char *new_path = strdup(cookie_path);
+  if(!new_path)
+    return NULL;
+
+  /* some stupid site sends path attribute with '"'. */
+  if(new_path[0] == '\"') {
+    memmove((void *)new_path, (const void *)(new_path + 1), strlen(new_path));
+  }
+  if(new_path[strlen(new_path) - 1] == '\"') {
+    new_path[strlen(new_path) - 1] = 0x0;
+  }
+
+  /* RFC6265 5.2.4 The Path Attribute */
+  if(new_path[0] != '/') {
+    /* Let cookie-path be the default-path. */
+    free(new_path);
+    new_path = strdup("/");
+    return new_path;
+  }
+
+  /* convert /hoge/ to /hoge */
+  len = strlen(new_path);
+  if(1 < len && new_path[len - 1] == '/') {
+    new_path[len - 1] = 0x0;
   }
-  /* matching! */
-  return TRUE;
+
+  return new_path;
 }
 
 /*
@@ -319,6 +403,11 @@ Curl_cookie_add(struct SessionHandle *data,
             badcookie = TRUE; /* out of memory bad */
             break;
           }
+          co->spath = sanitize_cookie_path(co->path);
+          if(!co->spath) {
+            badcookie = TRUE; /* out of memory bad */
+            break;
+          }
         }
         else if(Curl_raw_equal("domain", name)) {
           /* Now, we make sure that our host is within the given domain,
@@ -454,6 +543,9 @@ Curl_cookie_add(struct SessionHandle *data,
         if(co->path) {
           memcpy(co->path, path, pathlen);
           co->path[pathlen]=0; /* zero terminate */
+          co->spath = sanitize_cookie_path(co->path);
+          if(!co->spath)
+            badcookie = TRUE; /* out of memory bad */
         }
         else
           badcookie = TRUE;
@@ -539,12 +631,21 @@ Curl_cookie_add(struct SessionHandle *data,
           co->path = strdup(ptr);
           if(!co->path)
             badcookie = TRUE;
+          else {
+            co->spath = sanitize_cookie_path(co->path);
+            if(!co->spath) {
+              badcookie = TRUE; /* out of memory bad */
+            }
+          }
           break;
         }
         /* this doesn't look like a path, make one up! */
         co->path = strdup("/");
         if(!co->path)
           badcookie = TRUE;
+        co->spath = strdup("/");
+        if(!co->spath)
+          badcookie = TRUE;
         fields++; /* add a field and fall down to secure */
         /* FALLTHROUGH */
       case 3:
@@ -615,14 +716,14 @@ Curl_cookie_add(struct SessionHandle *data,
       if(replace_old) {
         /* the domains were identical */
 
-        if(clist->path && co->path) {
-          if(Curl_raw_equal(clist->path, co->path)) {
+        if(clist->spath && co->spath) {
+          if(Curl_raw_equal(clist->spath, co->spath)) {
             replace_old = TRUE;
           }
           else
             replace_old = FALSE;
         }
-        else if(!clist->path && !co->path)
+        else if(!clist->spath && !co->spath)
           replace_old = TRUE;
         else
           replace_old = FALSE;
@@ -651,6 +752,8 @@ Curl_cookie_add(struct SessionHandle *data,
           free(clist->domain);
         if(clist->path)
           free(clist->path);
+        if(clist->spath)
+          free(clist->spath);
         if(clist->expirestr)
           free(clist->expirestr);
 
@@ -845,7 +948,7 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
 
         /* now check the left part of the path with the cookies path
            requirement */
-        if(!co->path || pathmatch(co->path, path) ) {
+        if(!co->spath || pathmatch(co->spath, path) ) {
 
           /* and now, we know this is a match and we should create an
              entry for the return-linked-list */
diff --git a/lib/cookie.h b/lib/cookie.h
index d3b63f780..bd890827c 100644
--- a/lib/cookie.h
+++ b/lib/cookie.h
@@ -29,7 +29,8 @@ struct Cookie {
   struct Cookie *next; /* next in the chain */
   char *name;        /* <this> = value */
   char *value;       /* name = <this> */
-  char *path;         /* path = <this> */
+  char *path;         /* path = <this> which is in Set-Cookie: */
+  char *spath;        /* sanitized cookie path */
   char *domain;      /* domain = <this> */
   curl_off_t expires;  /* expires = <this> */
   char *expirestr;   /* the plain text version */
diff --git a/tests/data/test31 b/tests/data/test31
index b1171d81d..38af83bb6 100644
--- a/tests/data/test31
+++ b/tests/data/test31
@@ -18,6 +18,8 @@ Content-Type: text/html
 Funny-head: yesyes

 Set-Cookie: foobar=name; domain=anything.com; path=/ ; secure

 Set-Cookie:ismatch=this  ; domain=127.0.0.1; path=/silly/

+Set-Cookie: overwrite=this  ; domain=127.0.0.1; path=/overwrite/

+Set-Cookie: overwrite=this2  ; domain=127.0.0.1; path=/overwrite

 Set-Cookie: sec1value=secure1  ; domain=127.0.0.1; path=/secure1/ ; secure

 Set-Cookie: sec2value=secure2  ; domain=127.0.0.1; path=/secure2/ ; secure=

 Set-Cookie: sec3value=secure3  ; domain=127.0.0.1; path=/secure3/ ; secure=

@@ -94,6 +96,7 @@ Accept: */*
 # This file was generated by libcurl! Edit at your own risk.
 
 .127.0.0.1	TRUE	/silly/	FALSE	0	ismatch	this
+.127.0.0.1	TRUE	/overwrite	FALSE	0	overwrite	this2
 .127.0.0.1	TRUE	/secure1/	TRUE	0	sec1value	secure1
 .127.0.0.1	TRUE	/secure2/	TRUE	0	sec2value	secure2
 .127.0.0.1	TRUE	/secure3/	TRUE	0	sec3value	secure3
diff --git a/tests/data/test62 b/tests/data/test62
index 19886066f..2e5d1db03 100644
--- a/tests/data/test62
+++ b/tests/data/test62
@@ -29,7 +29,7 @@ http
 HTTP, send cookies when using custom Host:
  </name>
  <command>
-http://%HOSTIP:%HTTPPORT/we/want/62 -b log/jar62.txt -H "Host: www.host.foo.com"
+http://%HOSTIP:%HTTPPORT/we/want/62 http://%HOSTIP:%HTTPPORT/we/want?hoge=fuga -b log/jar62.txt -H "Host: www.host.foo.com"
 </command>
 <file name="log/jar62.txt">
 # Netscape HTTP Cookie File
@@ -55,6 +55,11 @@ Accept: */*
 Cookie: test2=yes; test=yes

 Host: www.host.foo.com

 

+GET /we/want?hoge=fuga HTTP/1.1

+Accept: */*

+Cookie: test2=yes; test=yes

+Host: www.host.foo.com

+

 </protocol>
 </verify>
 </testcase>