[haiku-commits] haiku: hrev46227 - src/kits/network/libnetapi headers/os/net

  • From: pulkomandy@xxxxxxxxxxxxx
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 14 Oct 2013 15:26:24 +0200 (CEST)

hrev46227 adds 1 changeset to branch 'master'
old head: ae527df336f44a3a7f12353def921078e2bfabf0
new head: c9d31eeed6ffa94ae37ce66959242a9a9ed40a60
overview: http://cgit.haiku-os.org/haiku/log/?qt=range&q=c9d31ee+%5Eae527df

----------------------------------------------------------------------------

c9d31ee: More cookie fixes
  
   * Add some error handling in NetworkCookie and don't add broken cookies
  (or should I say crumbs?) to the cookie jar
   * More control on the path and domain, as well as the expiration time
  
  We now pass Opera cookie testsuite functionality tests, as well as some
  of the negative tests (we even do better than curl). Not going further
  right now as this works well enough for positive cases and most
  security/privacy issues are fixed (cross domain and cross path cookie
  setting or spying).

                             [ Adrien Destugues <pulkomandy@xxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

Revision:    hrev46227
Commit:      c9d31eeed6ffa94ae37ce66959242a9a9ed40a60
URL:         http://cgit.haiku-os.org/haiku/commit/?id=c9d31ee
Author:      Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Date:        Mon Oct 14 13:16:37 2013 UTC

----------------------------------------------------------------------------

5 files changed, 94 insertions(+), 51 deletions(-)
headers/os/net/NetworkCookie.h                  |   6 +-
src/kits/network/libnetapi/HttpRequest.cpp      |   5 +-
src/kits/network/libnetapi/HttpTime.cpp         |  10 ++
src/kits/network/libnetapi/NetworkCookie.cpp    | 119 ++++++++++++--------
src/kits/network/libnetapi/NetworkCookieJar.cpp |   5 +-

----------------------------------------------------------------------------

diff --git a/headers/os/net/NetworkCookie.h b/headers/os/net/NetworkCookie.h
index 9c900e4..56512d5 100644
--- a/headers/os/net/NetworkCookie.h
+++ b/headers/os/net/NetworkCookie.h
@@ -31,8 +31,8 @@ public:
        // Modify the cookie fields
                        BNetworkCookie&         SetName(const BString& name);
                        BNetworkCookie&         SetValue(const BString& value);
-                       BNetworkCookie&         SetDomain(const BString& 
domain);
-                       BNetworkCookie&         SetPath(const BString& path);
+                       status_t                        SetDomain(const 
BString& domain);
+                       status_t                        SetPath(const BString& 
path);
                        BNetworkCookie&         SetMaxAge(int32 maxAge);
                        BNetworkCookie&         SetExpirationDate(time_t 
expireDate);
                        BNetworkCookie&         SetExpirationDate(BDateTime& 
expireDate);
@@ -99,9 +99,9 @@ private:
                        BString                         fDomain;
                        BString                         fPath;
                        BDateTime                       fExpiration;
+                       status_t                        fInitStatus;
                        bool                            fSecure;
                        bool                            fHttpOnly;
-
                        bool                            fHostOnly;
                        bool                            fSessionCookie;
 };
diff --git a/src/kits/network/libnetapi/HttpRequest.cpp 
b/src/kits/network/libnetapi/HttpRequest.cpp
index 21e11fb..796764d 100644
--- a/src/kits/network/libnetapi/HttpRequest.cpp
+++ b/src/kits/network/libnetapi/HttpRequest.cpp
@@ -876,10 +876,13 @@ BHttpRequest::_AddHeaders()
                for (BNetworkCookieJar::UrlIterator it
                                = fContext->GetCookieJar().GetUrlIterator(fUrl);
                                (cookie = it.Next()) != NULL;) {
-                       cookieString << cookie->RawCookie(false);
                        cookieString << "; ";
+                       cookieString << cookie->RawCookie(false);
                }
 
+               // Remove the extra "; " we added before the first item
+               cookieString.Remove(0, 2);
+
                if (cookieString.Length() > 0)
                        fOutputHeaders.AddHeader("Cookie", cookieString);
        }
diff --git a/src/kits/network/libnetapi/HttpTime.cpp 
b/src/kits/network/libnetapi/HttpTime.cpp
index 067a1f7..b963f95 100644
--- a/src/kits/network/libnetapi/HttpTime.cpp
+++ b/src/kits/network/libnetapi/HttpTime.cpp
@@ -77,6 +77,16 @@ BHttpTime::Parse()
        
        if (fDateString.Length() < 4)
                return 0;
+
+       expireTime.tm_sec = 0;
+       expireTime.tm_min = 0;
+       expireTime.tm_hour = 0;
+       expireTime.tm_mday = 0;
+       expireTime.tm_mon = 0;
+       expireTime.tm_year = 0;
+       expireTime.tm_wday = 0;
+       expireTime.tm_yday = 0;
+       expireTime.tm_isdst = 0;
                
        if (fDateString[3] == ',') {
                if (strptime(fDateString.String(), kRfc1123Format, &expireTime)
diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp 
b/src/kits/network/libnetapi/NetworkCookie.cpp
index 4b5c4fb..615e218 100644
--- a/src/kits/network/libnetapi/NetworkCookie.cpp
+++ b/src/kits/network/libnetapi/NetworkCookie.cpp
@@ -45,7 +45,7 @@ BNetworkCookie::BNetworkCookie(const char* name, const char* 
value,
 BNetworkCookie::BNetworkCookie(const BString& cookieString, const BUrl& url)
 {
        _Reset();
-       ParseCookieString(cookieString, url);
+       fInitStatus = ParseCookieString(cookieString, url);
 }
 
 
@@ -89,6 +89,11 @@ BNetworkCookie::ParseCookieString(const BString& string, 
const BUrl& url)
 {
        _Reset();
 
+       // Set default values (these can be overriden later on)
+       SetPath(_DefaultPathForUrl(url));
+       SetDomain(url.Host());
+       fHostOnly = true;
+
        BString name;
        BString value;
        int32 index = 0;
@@ -103,6 +108,10 @@ BNetworkCookie::ParseCookieString(const BString& string, 
const BUrl& url)
        SetName(name);
        SetValue(value);
 
+       // Note on error handling: even if there are parse errors, we will 
continue
+       // and try to parse as much from the cookie as we can.
+       status_t result = B_OK;
+
        // Parse the remaining cookie attributes.
        while (index < string.Length()) {
                ASSERT(string[index] == ';');
@@ -116,44 +125,55 @@ BNetworkCookie::ParseCookieString(const BString& string, 
const BUrl& url)
                        SetHttpOnly(true);
 
                // The following attributes require a value.
-               if (value.IsEmpty())
-                       continue;
 
                if (name.ICompare("max-age") == 0) {
+                       if (value.IsEmpty()) {
+                               result = B_BAD_VALUE;
+                               continue;
+                       }
                        // Validate the max-age value.
                        char* end = NULL;
                        long maxAge = strtol(value.String(), &end, 10);
                        if (*end == '\0')
                                SetMaxAge((int)maxAge);
+                       else
+                               SetMaxAge(-1); // cookie will expire immediately
                } else if (name.ICompare("expires") == 0) {
+                       if (value.IsEmpty()) {
+                               result = B_BAD_VALUE;
+                               continue;
+                       }
                        BHttpTime date(value);
                        SetExpirationDate(date.Parse());
                } else if (name.ICompare("domain") == 0) {
-                       SetDomain(value);
+                       if (value.IsEmpty()) {
+                               result = B_BAD_VALUE;
+                               continue;
+                       }
+
+                       status_t domainResult = SetDomain(value);
+                       // Do not reset the result to B_OK if something else 
already failed
+                       if (result == B_OK)
+                               result = domainResult;
                } else if (name.ICompare("path") == 0) {
-                       SetPath(value);
+                       if (value.IsEmpty()) {
+                               result = B_BAD_VALUE;
+                               continue;
+                       }
+                       status_t pathResult = SetPath(value);
+                       if (result == B_OK)
+                               result = pathResult;
                }
        }
 
-       // If no domain was specified, we set a host-only domain from the URL.
-       if (!HasDomain()) {
-               SetDomain(url.Host());
-               fHostOnly = true;
-       } else {
-               // Otherwise the setting URL must domain-match the domain it 
set.
-               if (!IsValidForDomain(url.Host())) {
-                       // Invalidate the cookie.
-                       _Reset();
-                       return B_NOT_ALLOWED;
-               }
+       if (!IsValidForDomain(url.Host())) {
+               // Invalidate the cookie.
+               _Reset();
+               return B_NOT_ALLOWED;
        }
 
-       // If no path was specified or the path is invalid, we compute the 
default
-       // path from the URL.
-       if (!HasPath() || Path()[0] != '/')
-               SetPath(_DefaultPathForUrl(url));
 
-       return B_OK;
+       return result;
 }
 
 
@@ -180,21 +200,24 @@ BNetworkCookie::SetValue(const BString& value)
 }
 
 
-BNetworkCookie&
+status_t
 BNetworkCookie::SetPath(const BString& path)
 {
+       if(path[0] != '/')
+               return B_BAD_DATA;
+
        // TODO: canonicalize the path
        fPath = path;
        fRawFullCookieValid = false;
-       return *this;
+       return B_OK;
 }
 
 
-BNetworkCookie&
+status_t
 BNetworkCookie::SetDomain(const BString& domain)
 {
        // TODO: canonicalize the domain
-       fDomain = domain;
+       BString newDomain = domain;
 
        // RFC 2109 (legacy) support: domain string may start with a dot,
        // meant to indicate the cookie should also be used for subdomains.
@@ -202,12 +225,19 @@ BNetworkCookie::SetDomain(const BString& domain)
        // not specified at all (in this case it has to exactly match the Url of
        // the page that set the cookie). In any case, we don't need to handle
        // dot-cookies specifically anymore, so just remove the extra dot.
-       if(fDomain[0] == '.')
-               fDomain.Remove(0, 1);
+       if(newDomain[0] == '.')
+               newDomain.Remove(0, 1);
+
+       // check we're not trying to set a cookie on a TLD or empty domain
+       if(newDomain.FindLast('.') <= 0)
+               return B_BAD_DATA;
+
+       fDomain = newDomain.ToLower();
+
        fHostOnly = false;
 
        fRawFullCookieValid = false;
-       return *this;
+       return B_OK;
 }
 
 
@@ -383,7 +413,7 @@ BNetworkCookie::IsSessionCookie() const
 bool
 BNetworkCookie::IsValid() const
 {
-       return HasName() && HasDomain() && HasPath();
+       return fInitStatus == B_OK && HasName() && HasDomain() && HasPath();
 }
 
 
@@ -410,9 +440,8 @@ BNetworkCookie::IsValidForDomain(const BString& domain) 
const
                return false;
 
        // If the cookie is host-only the domains must match exactly.
-       if (IsHostOnly()) {
+       if (IsHostOnly())
                return domain == cookieDomain;
-       }
 
        // FIXME prevent supercookies with a domain of ".com" or similar
        // This is NOT as straightforward as relying on the last dot in the 
domain.
@@ -436,24 +465,17 @@ bool
 BNetworkCookie::IsValidForPath(const BString& path) const
 {
        const BString& cookiePath = Path();
+       BString normalizedPath = path;
 
-       if (path.Length() < cookiePath.Length())
-               return false;
+       int slashPos = normalizedPath.FindLast('/');
+       if(slashPos != normalizedPath.Length() - 1)
+               normalizedPath.Truncate(slashPos + 1);
 
-       // The cookie path must be a prefix of the path string
-       if (path.Compare(cookiePath, cookiePath.Length()) != 0)
+       if (normalizedPath.Length() < cookiePath.Length())
                return false;
 
-       // The paths match if they are identical, or if the last
-       // character of the prefix is a slash, or if the character
-       // after the prefix is a slash.
-       if (path.Length() == cookiePath.Length()
-                       || cookiePath[cookiePath.Length() - 1] == '/'
-                       || path[cookiePath.Length()] == '/') {
-               return true;
-       }
-
-       return false;
+       // The cookie path must be a prefix of the path string
+       return normalizedPath.Compare(cookiePath, cookiePath.Length()) == 0;
 }
 
 
@@ -720,6 +742,13 @@ BNetworkCookie::_ExtractAttributeValuePair(const BString& 
cookieString,
        else
                value.SetTo("");
 
+       // values may (or may not) have quotes around them.
+       if(value[0] == '"' && value[value.Length() - 1] == '"')
+       {
+               value.Remove(0, 1);
+               value.Remove(value.Length() - 1, 1);
+       }
+
        return cookieAVEnd;
 }
 
diff --git a/src/kits/network/libnetapi/NetworkCookieJar.cpp 
b/src/kits/network/libnetapi/NetworkCookieJar.cpp
index 4383b1e..2a99a0f 100644
--- a/src/kits/network/libnetapi/NetworkCookieJar.cpp
+++ b/src/kits/network/libnetapi/NetworkCookieJar.cpp
@@ -117,7 +117,7 @@ BNetworkCookieJar::AddCookie(const BString& cookie, const 
BUrl& referrer)
 status_t
 BNetworkCookieJar::AddCookie(BNetworkCookie* cookie)
 {
-       if (cookie == NULL)
+       if (cookie == NULL || !cookie->IsValid())
                return B_BAD_VALUE;
 
        HashString key(cookie->Domain());
@@ -140,8 +140,9 @@ BNetworkCookieJar::AddCookie(BNetworkCookie* cookie)
 
        if (cookie->ShouldDeleteNow())
                delete cookie;
-       else
+       else {
                list->AddItem(cookie);
+       }
 
        return B_OK;
 }


Other related posts:

  • » [haiku-commits] haiku: hrev46227 - src/kits/network/libnetapi headers/os/net - pulkomandy