[BlackBerry] Cookies should be checked during parsing to improve performance.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 May 2012 11:43:14 +0000 (11:43 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 May 2012 11:43:14 +0000 (11:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=85028

Patch by Jason Liu <jason.liu@torchmobile.com.cn> on 2012-05-16
Reviewed by George Staikos.

We shouldn't waste time and memery on invalid cookies. It is better to drop them during parsing.
We shouldn't check the default domain since it is set with host. So we only check domains which are parsed
from response headers.

No new tests. No functionality change.

* platform/blackberry/CookieManager.cpp:
(WebCore::CookieManager::setCookies):
* platform/blackberry/CookieManager.h:
* platform/blackberry/CookieParser.cpp:
(WebCore::CookieParser::parseOneCookie):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/blackberry/CookieManager.cpp
Source/WebCore/platform/blackberry/CookieManager.h
Source/WebCore/platform/blackberry/CookieParser.cpp

index 8ae93a5..99f72b4 100644 (file)
@@ -1,3 +1,22 @@
+2012-05-16  Jason Liu  <jason.liu@torchmobile.com.cn>
+
+        [BlackBerry] Cookies should be checked during parsing to improve performance.
+        https://bugs.webkit.org/show_bug.cgi?id=85028
+
+        Reviewed by George Staikos.
+
+        We shouldn't waste time and memery on invalid cookies. It is better to drop them during parsing.
+        We shouldn't check the default domain since it is set with host. So we only check domains which are parsed
+        from response headers.
+
+        No new tests. No functionality change.
+
+        * platform/blackberry/CookieManager.cpp:
+        (WebCore::CookieManager::setCookies):
+        * platform/blackberry/CookieManager.h:
+        * platform/blackberry/CookieParser.cpp:
+        (WebCore::CookieParser::parseOneCookie):
+
 2012-05-15  Pierre Rossi  <pierre.rossi@gmail.com>
 
         [Qt] Enable SVG Fonts by default
index dd72e67..6a99d2d 100644 (file)
@@ -138,61 +138,9 @@ void CookieManager::setCookies(const KURL& url, const String& value)
     Vector<ParsedCookie*> cookies = parser.parse(value);
 
     for (size_t i = 0; i < cookies.size(); ++i) {
-        ParsedCookie* cookie = cookies[i];
-        if (!shouldRejectForSecurityReason(cookie, url)) {
-            BackingStoreRemovalPolicy treatment = m_privateMode ? DoNotRemoveFromBackingStore : RemoveFromBackingStore;
-            checkAndTreatCookie(cookie, treatment);
-        } else
-            delete cookie;
-    }
-}
-
-bool CookieManager::shouldRejectForSecurityReason(const ParsedCookie* cookie, const KURL& url)
-{
-    // We have to disable the following check because sites like Facebook and
-    // Gmail currently do not follow the spec.
-#if 0
-    // Check if path attribute is a prefix of the request URI.
-    if (!url.path().startsWith(cookie->path())) {
-        LOG_ERROR("Cookie %s is rejected because its path does not math the URL %s\n", cookie->toString().utf8().data(), url.string().utf8().data());
-        return true;
-    }
-#endif
-
-    // ignore domain security if protocol doesn't have domains
-    if (shouldIgnoreDomain(cookie->protocol()))
-        return false;
-
-    // Reject Cookie if domain is empty
-    if (!cookie->domain().length())
-        return true;
-
-    // If an explicitly specified value does not start with a dot, the user agent supplies. (RFC 2965 3.2.2)
-    // Domain: Defaults to the effective request-host. There is no dot at the beginning of request-host. (RFC 2965 3.3.1)
-    if (cookie->domain()[0] == '.') {
-        // Check if the domain contains an embedded dot.
-        size_t dotPosition = cookie->domain().find(".", 1);
-        if (dotPosition == notFound || dotPosition == cookie->domain().length()) {
-            LOG_ERROR("Cookie %s is rejected because its domain does not contain an embedded dot.\n", cookie->toString().utf8().data());
-            return true;
-        }
+        BackingStoreRemovalPolicy treatment = m_privateMode ? DoNotRemoveFromBackingStore : RemoveFromBackingStore;
+        checkAndTreatCookie(cookies[i], treatment);
     }
-
-    // The request host should domain match the Domain attribute.
-    // Domain string starts with a dot, so a.b.com should domain match .a.b.com.
-    // add a "." at beginning of host name, because it can handle many cases such as
-    // a.b.com matches b.com, a.b.com matches .B.com and a.b.com matches .A.b.Com
-    // and so on.
-    String hostDomainName = url.host();
-    hostDomainName = hostDomainName.startsWith('.') ? hostDomainName : "." + hostDomainName;
-    if (!hostDomainName.endsWith(cookie->domain(), false)) {
-        LOG_ERROR("Cookie %s is rejected because its domain does not domain match the URL %s\n", cookie->toString().utf8().data(), url.string().utf8().data());
-        return true;
-    }
-    // We should check for an embedded dot in the portion of string in the host not in the domain
-    // but to match firefox behaviour we do not.
-
-    return false;
 }
 
 String CookieManager::getCookie(const KURL& url, CookieFilter filter) const
index 904551e..0614e7d 100644 (file)
@@ -119,8 +119,6 @@ private:
 
     void checkAndTreatCookie(ParsedCookie*, BackingStoreRemovalPolicy);
 
-    bool shouldRejectForSecurityReason(const ParsedCookie*, const KURL&);
-
     void addCookieToMap(CookieMap* targetMap, ParsedCookie* candidateCookie, BackingStoreRemovalPolicy postToBackingStore);
 
     CookieMap* findOrCreateCookieMap(CookieMap* protocolMap, const String& domain, bool findOnly);
index e089da0..91babc2 100644 (file)
@@ -230,6 +230,15 @@ ParsedCookie* CookieParser::parseOneCookie(const String& cookie, unsigned start,
                 // The path attribute may or may not include percent-encoded characters. Fortunately
                 // if there are no percent-encoded characters, decoding the url is a no-op.
                 res->setPath(decodeURLEscapeSequences(parsedValue));
+
+                // We have to disable the following check because sites like Facebook and
+                // Gmail currently do not follow the spec.
+#if 0
+                // Check if path attribute is a prefix of the request URI.
+                if (!m_defaultCookieURL.path().startsWith(res->path()))
+                    LOG_AND_DELETE("Invalid cookie %s (path): it does not math the URL", cookie.ascii().data());
+#endif
+
             } else
                 LOG_AND_DELETE("Invalid cookie %s (path)", cookie.ascii().data());
             break;
@@ -240,9 +249,28 @@ ParsedCookie* CookieParser::parseOneCookie(const String& cookie, unsigned start,
             if (length >= 6 && cookie.find("omain", tokenStartSvg + 1, false)) {
                 if (parsedValue.length() > 1 && parsedValue[0] == '"' && parsedValue[parsedValue.length() - 1] == '"')
                     parsedValue = parsedValue.substring(1, parsedValue.length() - 2);
+
+                // Check if the domain contains an embedded dot.
+                size_t dotPosition = parsedValue.find(".", 1);
+                if (dotPosition == notFound || dotPosition == parsedValue.length())
+                    LOG_AND_DELETE("Invalid cookie %s (domain): it does not contain an embedded dot", cookie.ascii().data());
+
                 // If the domain does not start with a dot, add one for security checks,
                 // For example: ab.c.com dose not domain match b.c.com;
                 String realDomain = parsedValue[0] == '.' ? parsedValue : "." + parsedValue;
+
+                // The request host should domain match the Domain attribute.
+                // Domain string starts with a dot, so a.b.com should domain match .a.b.com.
+                // add a "." at beginning of host name, because it can handle many cases such as
+                // a.b.com matches b.com, a.b.com matches .B.com and a.b.com matches .A.b.Com
+                // and so on.
+                String hostDomainName = m_defaultCookieURL.host();
+                hostDomainName = hostDomainName.startsWith('.') ? hostDomainName : "." + hostDomainName;
+                if (!hostDomainName.endsWith(realDomain, false))
+                    LOG_AND_DELETE("Invalid cookie %s (domain): it does not domain match the host");
+                // We should check for an embedded dot in the portion of string in the host not in the domain
+                // but to match firefox behaviour we do not.
+
                 res->setDomain(realDomain);
             } else
                 LOG_AND_DELETE("Invalid cookie %s (domain)", cookie.ascii().data());