Skip to content

Commit

Permalink
Cleanup and fix cookies handling
Browse files Browse the repository at this point in the history 
 * 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.
  • Loading branch information
@pulkomandy
pulkomandy committed Oct 9, 2013
1 parent a24b8b8 commit 780967d
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 58 deletions.
7 changes: 2 additions & 5 deletions headers/os/net/NetworkCookie.h
View file
Expand Up @@ -16,8 +16,7 @@
class BNetworkCookie : public BArchivable {
public:
BNetworkCookie(const char* name,
const char* value);
BNetworkCookie(const BString& cookieString);
const char* value, const BUrl& url);
BNetworkCookie(const BString& cookieString,
const BUrl& url);
BNetworkCookie(BMessage* archive);
Expand All @@ -26,9 +25,8 @@ class BNetworkCookie : public BArchivable {

// 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);
Expand Down Expand Up @@ -76,7 +74,6 @@ class BNetworkCookie : public BArchivable {
static BArchivable* Instantiate(BMessage* archive);

// Overloaded operators
BNetworkCookie& operator=(const char* string);
bool operator==(const BNetworkCookie& other);
bool operator!=(const BNetworkCookie& other);
private:
Expand Down
6 changes: 4 additions & 2 deletions headers/os/net/NetworkCookieJar.h
View file
Expand Up @@ -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 {
Expand All @@ -35,6 +35,8 @@ class BNetworkCookieJar : public BArchivable, public BFlattenable {
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);

Expand Down
59 changes: 20 additions & 39 deletions src/kits/network/libnetapi/NetworkCookie.cpp
View file
Expand Up @@ -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;
}


BNetworkCookie::BNetworkCookie(const BString& cookieString)
{
_Reset();
ParseCookieString(cookieString);
SetDomain(url.Host());
SetPath(_DefaultPathForUrl(url));
}


BNetworkCookie::BNetworkCookie(const BString& cookieString,
const BUrl& url)
BNetworkCookie::BNetworkCookie(const BString& cookieString, const BUrl& url)
{
_Reset();
ParseCookieStringFromUrl(cookieString, url);
ParseCookieString(cookieString, url);
}


Expand Down Expand Up @@ -88,9 +84,8 @@ BNetworkCookie::~BNetworkCookie()
// #pragma mark String to cookie fields


BNetworkCookie&
BNetworkCookie::ParseCookieStringFromUrl(const BString& string,
const BUrl& url)
status_t
BNetworkCookie::ParseCookieString(const BString& string, const BUrl& url)
{
_Reset();

Expand All @@ -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);
Expand Down Expand Up @@ -149,27 +144,16 @@ 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
// path from the URL.
if (!HasPath() || Path()[0] != '/')
SetPath(_DefaultPathForUrl(url));

return *this;
}


BNetworkCookie&
BNetworkCookie::ParseCookieString(const BString& string)
{
BUrl url;
ParseCookieStringFromUrl(string, url);
return *this;
return B_OK;
}


Expand Down Expand Up @@ -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)
return true;
else if (domain[difference - 1] == '.')
if (difference == 0 || suffix[0] == '.')
return true;
}

Expand Down Expand Up @@ -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)
{
Expand Down
40 changes: 28 additions & 12 deletions src/kits/network/libnetapi/NetworkCookieJar.cpp
View file
Expand Up @@ -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)
{
Expand All @@ -112,11 +130,10 @@ BNetworkCookieJar::AddCookie(BNetworkCookie* cookie)
}

for (int32 i = 0; i < list->CountItems(); i++) {
BNetworkCookie* c
= reinterpret_cast<BNetworkCookie*>(list->ItemAt(i));
BNetworkCookie* c = list->ItemAt(i);

if (c->Name() == cookie->Name() && c->Path() == cookie->Path()) {
list->RemoveItem(i);
list->RemoveItemAt(i);
break;
}
}
Expand All @@ -134,8 +151,7 @@ status_t
BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies)
{
for (int32 i = 0; i < cookies.CountItems(); i++) {
BNetworkCookie* cookiePtr
= reinterpret_cast<BNetworkCookie*>(cookies.ItemAt(i));
BNetworkCookie* cookiePtr = cookies.ItemAt(i);

// Using AddCookie by reference in order to avoid multiple
// cookie jar share the same cookie pointers
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}


Expand Down Expand Up @@ -600,7 +616,7 @@ BNetworkCookieJar::UrlIterator::Remove()

BNetworkCookie* result = fLastElement;

fLastList->RemoveItem(fLastIndex);
fLastList->RemoveItemAt(fLastIndex);

if (fLastList->CountItems() == 0) {
HashString lastKey(fLastElement->Domain(),
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 780967d

Please sign in to comment.