Source/WebCore: [WinCairo] Support for cookies is incomplete
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2013 01:12:21 +0000 (01:12 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 6 Mar 2013 01:12:21 +0000 (01:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110147

Expired and HttpOnly cookies no longer accessible from JavaScript.
Cookies set in JavaScript now have correct domain/path.

Patch by Peter Nelson <peter@peterdn.com> on 2013-03-05
Reviewed by Brent Fulgham.

Test: http/tests/cookies/http-get-cookie-set-in-js.html

* platform/network/curl/CookieJarCurl.cpp:
(WebCore):
(WebCore::addMatchingCurlCookie):
(WebCore::getNetscapeCookieFormat):
(WebCore::setCookiesFromDOM):
(WebCore::cookieRequestHeaderFieldValue):

LayoutTests: [WinCairo] Support for cookies is incomplete
https://bugs.webkit.org/show_bug.cgi?id=110147

Patch by Peter Nelson <peter@peterdn.com> on 2013-03-05
Reviewed by Brent Fulgham.

Re-enabled cookie tests for WinCairo.
Added test to check whether cookie set in HTTP response is accessible in JS.

* http/tests/cookies/http-get-cookie-set-in-js-expected.txt: Added.
* http/tests/cookies/http-get-cookie-set-in-js.html: Added.
* http/tests/cookies/resources/cookies-test-pre.js:
(clearAllCookies): Cookies set in JS now correctly cleared.
* platform/wincairo/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@144853 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/cookies/http-get-cookie-set-in-js-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cookies/http-get-cookie-set-in-js.html [new file with mode: 0644]
LayoutTests/http/tests/cookies/resources/cookies-test-pre.js
LayoutTests/platform/wincairo/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/network/curl/CookieJarCurl.cpp

index 0fb34af..d45f9fd 100644 (file)
@@ -1,3 +1,19 @@
+2013-03-05  Peter Nelson  <peter@peterdn.com>
+
+        [WinCairo] Support for cookies is incomplete
+        https://bugs.webkit.org/show_bug.cgi?id=110147
+
+        Reviewed by Brent Fulgham.
+        
+        Re-enabled cookie tests for WinCairo.
+        Added test to check whether cookie set in HTTP response is accessible in JS.
+
+        * http/tests/cookies/http-get-cookie-set-in-js-expected.txt: Added.
+        * http/tests/cookies/http-get-cookie-set-in-js.html: Added.
+        * http/tests/cookies/resources/cookies-test-pre.js:
+        (clearAllCookies): Cookies set in JS now correctly cleared.
+        * platform/wincairo/TestExpectations:
+
 2013-03-05  Roger Fong  <roger_fong@apple.com>
 
         Unreviewed. AppleWin gardening.
diff --git a/LayoutTests/http/tests/cookies/http-get-cookie-set-in-js-expected.txt b/LayoutTests/http/tests/cookies/http-get-cookie-set-in-js-expected.txt
new file mode 100644 (file)
index 0000000..5121213
--- /dev/null
@@ -0,0 +1,11 @@
+Test that a cookie set using JavaScript can be correctly read by HTTP server
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+checking that the cookie set in javascript can be read by HTTP server
+PASS cookie is 'name=value'.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/cookies/http-get-cookie-set-in-js.html b/LayoutTests/http/tests/cookies/http-get-cookie-set-in-js.html
new file mode 100644 (file)
index 0000000..ab8eb4d
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="stylesheet" href="resources/cookies-test-style.css">
+<script src="resources/cookies-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description(
+'Test that a cookie set using JavaScript can be correctly read by HTTP server'
+);
+
+clearAllCookies();
+
+document.cookie = "name=value;Max-Age=1000";
+
+debug('checking that the cookie set in javascript can be read by HTTP server');
+testCookies("name=value");
+
+clearCookies();
+
+successfullyParsed = true;
+</script>
+<script src="resources/cookies-test-post.js"></script>
+</body>
+</html>
index 59c34c2..b7f5d9d 100644 (file)
@@ -204,6 +204,10 @@ function clearAllCookies()
         var cookieName = cookieString.substr(0, cookieString.indexOf("=") || cookieString.length());
         cookies.push(cookieName);
         clearCookies();
+
+        // In case clearCookies.cgi failed, for example,
+        // the domain/path do not match exactly:
+        document.cookie = cookieName + "=;Max-Age=0";
     }
 }
 
index fe7d616..2f5bd8e 100644 (file)
@@ -528,7 +528,6 @@ transitions/svg-text-shadow-transition.html
 
 http/tests/cache
 http/tests/canvas/philip/tests
-http/tests/cookies
 http/tests/css
 http/tests/history
 http/tests/incremental
@@ -1032,10 +1031,6 @@ fast/loader/create-frame-in-DOMContentLoaded.html
 # Times out <rdar://problem/9304941>
 http/tests/multipart/invalid-image-data-standalone.html
 
-# Sometimes fail <rdar://problem/9349921>
-http/tests/cookies/simple-cookies-expired.html
-http/tests/cookies/simple-cookies-max-age.html
-
 ################################################################################
 ####################### No bugs filed about the below yet#######################
 ################################################################################
@@ -2782,11 +2777,6 @@ http/tests/security/xss-DENIED-window-open-javascript-url-with-spaces.html
 http/tests/security/xss-DENIED-window-open-parent.html
 http/tests/security/xss-DENIED-xsl-document-securityOrigin.xml
 
-# Flaky http/tests/cookie tests
-# https://bugs.webkit.org/show_bug.cgi?id=95805
-http/tests/cookies/multiple-cookies.html
-http/tests/cookies/single-quoted-value.html
-
 # Flaky media/video tests
 # https://bugs.webkit.org/show_bug.cgi?id=95806
 media/video-aspect-ratio.html
index e074dfc..6c3fbc9 100644 (file)
@@ -1,3 +1,22 @@
+2013-03-05  Peter Nelson  <peter@peterdn.com>
+
+        [WinCairo] Support for cookies is incomplete
+        https://bugs.webkit.org/show_bug.cgi?id=110147
+        
+        Expired and HttpOnly cookies no longer accessible from JavaScript.
+        Cookies set in JavaScript now have correct domain/path.
+
+        Reviewed by Brent Fulgham.
+
+        Test: http/tests/cookies/http-get-cookie-set-in-js.html
+
+        * platform/network/curl/CookieJarCurl.cpp:
+        (WebCore):
+        (WebCore::addMatchingCurlCookie):
+        (WebCore::getNetscapeCookieFormat):
+        (WebCore::setCookiesFromDOM):
+        (WebCore::cookieRequestHeaderFieldValue):
+
 2013-03-05  Alec Flett  <alecflett@chromium.org>
 
         Fix mac clang build with long long
index f2c5a3a..65777f6 100644 (file)
@@ -27,8 +27,6 @@
 
 namespace WebCore {
 
-static HashMap<String, String> cookieJar;
-
 static void readCurlCookieToken(const char*& cookie, String& token)
 {
     // Read the next token from a cookie with the Netscape cookie format.
@@ -70,6 +68,10 @@ static void addMatchingCurlCookie(const char* cookie, const String& domain, cons
 
     bool subDomain = false;
 
+    // HttpOnly cookie entries begin with "#HttpOnly_".
+    if (cookieDomain.startsWith("#HttpOnly_"))
+        return;
+
     if (cookieDomain[0] == '.') {
         // Check if domain is a subdomain of the domain in the cookie.
         // Curl uses a '.' in front of domains to indicate its valid on subdomains.
@@ -119,10 +121,92 @@ static void addMatchingCurlCookie(const char* cookie, const String& domain, cons
     cookies = cookies + strName + "=" + strValue + "; ";
 }
 
-void setCookiesFromDOM(const NetworkStorageSession&, const KURL&, const KURL& url, const String& value)
+static String getNetscapeCookieFormat(const KURL& url, const String& value)
 {
-    cookieJar.set(url.string(), value);
+    // Constructs a cookie string in Netscape Cookie file format.
+
+    if (value.isEmpty())
+        return "";
+
+    String valueStr;
+    if (value.is8Bit())
+        valueStr = value;
+    else
+        valueStr = String::make8BitFrom16BitSource(value.characters16(), value.length());
+
+    Vector<String> attributes;
+    valueStr.split(';', false, attributes);
+
+    if (!attributes.size())
+        return "";
+
+    // First attribute should be <cookiename>=<cookievalue>
+    String cookieName, cookieValue;
+    Vector<String>::iterator attribute = attributes.begin();
+    if (attribute->contains('=')) {
+        Vector<String> nameValuePair;
+        attribute->split('=', true, nameValuePair);
+        cookieName = nameValuePair[0];
+        cookieValue = nameValuePair[1];
+    } else {
+        // According to RFC6265 we should ignore the entire
+        // set-cookie string now, but other browsers appear
+        // to treat this as <cookiename>=<empty>
+        cookieName = *attribute;
+    }
+    
+    int expires = 0;
+    String secure = "FALSE";
+    String path = url.baseAsString().substring(url.pathStart());
+    if (path.length() > 1 && path.endsWith('/'))
+        path.remove(path.length() - 1);
+    String domain = url.host();
+
+    // Iterate through remaining attributes
+    for (++attribute; attribute != attributes.end(); ++attribute) {
+        if (attribute->contains('=')) {
+            Vector<String> keyValuePair;
+            attribute->split('=', true, keyValuePair);
+            String key = keyValuePair[0].stripWhiteSpace().lower();
+            String val = keyValuePair[1].stripWhiteSpace();
+            if (key == "expires") {
+                CString dateStr(reinterpret_cast<const char*>(val.characters8()), val.length());
+                expires = WTF::parseDateFromNullTerminatedCharacters(dateStr.data()) / WTF::msPerSecond;
+            } else if (key == "max-age")
+                expires = time(0) + val.toInt();
+            else if (key == "domain") 
+                domain = val;
+            else if (key == "path") 
+                path = val;
+        } else {
+            String key = attribute->stripWhiteSpace().lower();
+            if (key == "secure")
+                secure = "TRUE";
+        }
+    }
+    
+    String allowSubdomains = domain.startsWith('.') ? "TRUE" : "FALSE";
+    String expiresStr = String::number(expires);
+
+    int finalStringLength = domain.length() + path.length() + expiresStr.length() + cookieName.length();
+    finalStringLength += cookieValue.length() + secure.length() + allowSubdomains.length();
+    finalStringLength += 6; // Account for \t separators.
+    
+    StringBuilder cookieStr;
+    cookieStr.reserveCapacity(finalStringLength);
+    cookieStr.append(domain + "\t");
+    cookieStr.append(allowSubdomains + "\t");
+    cookieStr.append(path + "\t");
+    cookieStr.append(secure + "\t");
+    cookieStr.append(expiresStr + "\t");
+    cookieStr.append(cookieName + "\t");
+    cookieStr.append(cookieValue);
+
+    return cookieStr.toString();
+}
 
+void setCookiesFromDOM(const NetworkStorageSession&, const KURL&, const KURL& url, const String& value)
+{
     CURL* curl = curl_easy_init();
 
     if (!curl)
@@ -135,17 +219,11 @@ void setCookiesFromDOM(const NetworkStorageSession&, const KURL&, const KURL& ur
     curl_easy_setopt(curl, CURLOPT_COOKIEFILE, cookieJarFileName);
     curl_easy_setopt(curl, CURLOPT_SHARE, curlsh);
 
-    String cookie("Set-Cookie: ");
-    if (value.is8Bit())
-        cookie.append(value);
-    else
-        cookie.append(String::make8BitFrom16BitSource(value.characters16(), value.length()));
-
-    if (cookie.find(String("domain")) == notFound) {
-        // If no domain is set, set domain and path from url.
-        cookie.append(String("; domain=") + url.host());
-        cookie.append(String("; path=") + url.path());
-    }
+    // CURL accepts cookies in either Set-Cookie or Netscape file format.
+    // However with Set-Cookie format, there is no way to specify that we
+    // should not allow cookies to be read from subdomains, which is the
+    // required behavior if the domain field is not explicity specified.
+    String cookie = getNetscapeCookieFormat(url, value);
 
     CString strCookie(reinterpret_cast<const char*>(cookie.characters8()), cookie.length());
 
@@ -192,7 +270,7 @@ String cookiesForDOM(const NetworkStorageSession&, const KURL&, const KURL& url)
 String cookieRequestHeaderFieldValue(const NetworkStorageSession&, const KURL& /*firstParty*/, const KURL& url)
 {
     // FIXME: include HttpOnly cookie.
-    return cookieJar.get(url.string());
+    return "";
 }
 
 bool cookiesEnabled(const NetworkStorageSession&, const KURL& /*firstParty*/, const KURL& /*url*/)