Cleanup and fix cookies handling

* The cookie jar iterator now use a BObjectList instead of a BList
 * Add a convenience method to the cookie jar to add a cookie by BUrl
and raw cookie string.
 * Remove some methods in BNetworkCookie that could lead to invalid
cookies (cross-domain or with no domain at all).
 * Make the cookie parsing able to report errors
 * Fix off-by-one error in domain cookies validation.

headers/os/net/NetworkCookie.h

@@ -16,8 +16,7 @@
 class BNetworkCookie : public BArchivable {
 public:
 								BNetworkCookie(const char* name,
-									const char* value);
+									const char* value, const BUrl& url);
-								BNetworkCookie(const BString& cookieString);
 								BNetworkCookie(const BString& cookieString,
 									const BUrl& url);
 								BNetworkCookie(BMessage* archive);
@@ -26,9 +25,8 @@ public:
 
 	// Parse a "SetCookie" string
 
-			BNetworkCookie&		ParseCookieStringFromUrl(const BString& string,
+			status_t			ParseCookieString(const BString& string,
 									const BUrl& url);
-			BNetworkCookie& 	ParseCookieString(const BString& cookieString);
 
 	// Modify the cookie fields
 			BNetworkCookie&		SetName(const BString& name);
@@ -76,7 +74,6 @@ public:
 	static	BArchivable*		Instantiate(BMessage* archive);
 
 	// Overloaded operators
-			BNetworkCookie&		operator=(const char* string);
 			bool				operator==(const BNetworkCookie& other);
 			bool				operator!=(const BNetworkCookie& other);
 private:

headers/os/net/NetworkCookieJar.h

@@ -7,14 +7,14 @@
 
 #include <Archivable.h>
 #include <Flattenable.h>
-#include <List.h>
 #include <Message.h>
+#include <ObjectList.h>
 #include <NetworkCookie.h>
 #include <String.h>
 #include <Url.h>
 
 
-typedef BList BNetworkCookieList;
+typedef BObjectList<BNetworkCookie> BNetworkCookieList;
 
 
 class BNetworkCookieJar : public BArchivable, public BFlattenable {
@@ -35,6 +35,8 @@ public:
 	virtual						~BNetworkCookieJar();
 
 			status_t			AddCookie(const BNetworkCookie& cookie);
+			status_t			AddCookie(const BString& cookie,
+									const BUrl& url);
 			status_t			AddCookie(BNetworkCookie* cookie);
 			status_t			AddCookies(const BNetworkCookieList& cookies);
 

src/kits/network/libnetapi/NetworkCookie.cpp

@@ -30,26 +30,22 @@ static const char* kArchivedCookieHttpOnly = "be:cookie.httponly";
 static const char* kArchivedCookieHostOnly = "be:cookie.hostonly";
 
 
-BNetworkCookie::BNetworkCookie(const char* name, const char* value)
+BNetworkCookie::BNetworkCookie(const char* name, const char* value,
+	const BUrl& url)
 {
 	_Reset();
 	fName = name;
 	fValue = value;
+
+	SetDomain(url.Host());
+	SetPath(_DefaultPathForUrl(url));
 }
 
 
-BNetworkCookie::BNetworkCookie(const BString& cookieString)
+BNetworkCookie::BNetworkCookie(const BString& cookieString, const BUrl& url)
 {
 	_Reset();
-	ParseCookieString(cookieString);
+	ParseCookieString(cookieString, url);
-}
-
-
-BNetworkCookie::BNetworkCookie(const BString& cookieString,
-	const BUrl& url)
-{
-	_Reset();
-	ParseCookieStringFromUrl(cookieString, url);
 }
 
 
@@ -88,9 +84,8 @@ BNetworkCookie::~BNetworkCookie()
 // #pragma mark String to cookie fields
 
 
-BNetworkCookie&
+status_t
-BNetworkCookie::ParseCookieStringFromUrl(const BString& string,
+BNetworkCookie::ParseCookieString(const BString& string, const BUrl& url)
-	const BUrl& url)
 {
 	_Reset();
 
@@ -102,7 +97,7 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string,
 	index = _ExtractNameValuePair(string, name, value, index);
 	if (index == -1) {
 		// The set-cookie-string is not valid
-		return *this;
+		return B_BAD_DATA;
 	}
 
 	SetName(name);
@@ -149,10 +144,8 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string,
 		if (!IsValidForDomain(url.Host())) {
 			// Invalidate the cookie.
 			_Reset();
-			return *this;
+			return B_NOT_ALLOWED;
 		}
-		// We should also reject cookies with domains that match public
-		// suffixes.
 	}
 
 	// If no path was specified or the path is invalid, we compute the default
@@ -160,16 +153,7 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string,
 	if (!HasPath() || Path()[0] != '/')
 		SetPath(_DefaultPathForUrl(url));
 
-	return *this;
+	return B_OK;
-}
-
-
-BNetworkCookie&
-BNetworkCookie::ParseCookieString(const BString& string)
-{
-	BUrl url;
-	ParseCookieStringFromUrl(string, url);
-	return *this;
 }
 
 
@@ -417,16 +401,20 @@ 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.
+	// Here's a list of TLD:
+	// https://github.com/rsimoes/Mozilla-PublicSuffix/blob/master/effective_tld_names.dat
 
 	// Otherwise, the domains must match exactly, or the cookie domain
-	// must be a suffix with the preceeding character being a dot.
+	// must be a suffix starting with a dot.
 	const char* suffix = domain.String() + difference;
 	if (strcmp(suffix, cookieDomain.String()) == 0) {
-		if (difference == 0)
+		if (difference == 0 || suffix[0] == '.')
-			return true;
-		else if (domain[difference - 1] == '.')
 			return true;
 	}
 
@@ -591,13 +579,6 @@ BNetworkCookie::Instantiate(BMessage* archive)
 // #pragma mark Overloaded operators
 
 
-BNetworkCookie&
-BNetworkCookie::operator=(const char* string)
-{
-	return ParseCookieString(string);
-}
-
-
 bool
 BNetworkCookie::operator==(const BNetworkCookie& other)
 {

src/kits/network/libnetapi/NetworkCookieJar.cpp

@@ -96,6 +96,24 @@ BNetworkCookieJar::AddCookie(const BNetworkCookie& cookie)
 }
 
 
+status_t
+BNetworkCookieJar::AddCookie(const BString& cookie, const BUrl& referrer)
+{
+	BNetworkCookie* heapCookie = new(std::nothrow) BNetworkCookie(cookie,
+		referrer);
+
+	if (heapCookie == NULL)
+		return B_NO_MEMORY;
+
+	status_t result = AddCookie(heapCookie);
+
+	if (result != B_OK)
+		delete heapCookie;
+
+	return result;
+}
+
+
 status_t
 BNetworkCookieJar::AddCookie(BNetworkCookie* cookie)
 {
@@ -112,11 +130,10 @@ BNetworkCookieJar::AddCookie(BNetworkCookie* cookie)
 	}
 
 	for (int32 i = 0; i < list->CountItems(); i++) {
-		BNetworkCookie* c
+		BNetworkCookie* c = list->ItemAt(i);
-			= reinterpret_cast<BNetworkCookie*>(list->ItemAt(i));
 
 		if (c->Name() == cookie->Name() && c->Path() == cookie->Path()) {
-			list->RemoveItem(i);
+			list->RemoveItemAt(i);
 			break;
 		}
 	}
@@ -134,8 +151,7 @@ status_t
 BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies)
 {
 	for (int32 i = 0; i < cookies.CountItems(); i++) {
-		BNetworkCookie* cookiePtr
+		BNetworkCookie* cookiePtr = cookies.ItemAt(i);
-			= reinterpret_cast<BNetworkCookie*>(cookies.ItemAt(i));
 
 		// Using AddCookie by reference in order to avoid multiple
 		// cookie jar share the same cookie pointers
@@ -454,7 +470,7 @@ BNetworkCookieJar::Iterator::NextDomain()
 
 	fList = *fIterator->fCookieMapIterator.NextValue();
 	fIndex = 0;
-	fElement = reinterpret_cast<BNetworkCookie*>(fList->ItemAt(fIndex));
+	fElement = fList->ItemAt(fIndex);
 
 	return result;
 }
@@ -474,10 +490,10 @@ BNetworkCookieJar::Iterator::Remove()
 			delete fLastList;
 		}
 		else
-			fLastList->RemoveItem(fLastList->CountItems() - 1);
+			fLastList->RemoveItemAt(fLastList->CountItems() - 1);
 	} else {
 		fIndex--;
-		fList->RemoveItem(fIndex);
+		fList->RemoveItemAt(fIndex);
 	}
 
 	fLastElement = NULL;
@@ -512,7 +528,7 @@ BNetworkCookieJar::Iterator::_FindNext()
 
 	fIndex++;
 	if (fList && fIndex < fList->CountItems()) {
-		fElement = reinterpret_cast<BNetworkCookie*>(fList->ItemAt(fIndex));
+		fElement = fList->ItemAt(fIndex);
 		return;
 	}
 
@@ -524,7 +540,7 @@ BNetworkCookieJar::Iterator::_FindNext()
 	fLastList = fList;
 	fList = *(fIterator->fCookieMapIterator.NextValue());
 	fIndex = 0;
-	fElement = reinterpret_cast<BNetworkCookie*>(fList->ItemAt(fIndex));
+	fElement = fList->ItemAt(fIndex);
 }
 
 
@@ -600,7 +616,7 @@ BNetworkCookieJar::UrlIterator::Remove()
 
 	BNetworkCookie* result = fLastElement;
 
-	fLastList->RemoveItem(fLastIndex);
+	fLastList->RemoveItemAt(fLastIndex);
 
 	if (fLastList->CountItems() == 0) {
 		HashString lastKey(fLastElement->Domain(),
@@ -686,7 +702,7 @@ BNetworkCookieJar::UrlIterator::_FindPath()
 {
 	fIndex++;
 	while (fList && fIndex < fList->CountItems()) {
-		fElement = reinterpret_cast<BNetworkCookie*>(fList->ItemAt(fIndex));
+		fElement = fList->ItemAt(fIndex);
 
 		if (fElement->IsValidForPath(fUrl.Path()))
 			return true;