diff --git a/headers/os/net/NetworkCookieJar.h b/headers/os/net/NetworkCookieJar.h index 58364a6a97..c6639b9af6 100644 --- a/headers/os/net/NetworkCookieJar.h +++ b/headers/os/net/NetworkCookieJar.h @@ -5,16 +5,28 @@ #ifndef _B_NETWORK_COOKIE_JAR_H_ #define _B_NETWORK_COOKIE_JAR_H_ +#include + #include #include #include -#include #include +#include #include #include -typedef BObjectList BNetworkCookieList; +class BNetworkCookieList: public BObjectList { +public: + BNetworkCookieList(); + ~BNetworkCookieList(); + + status_t LockForReading(); + status_t LockForWriting(); + status_t Unlock(); +private: + pthread_rwlock_t fLock; +}; class BNetworkCookieJar : public BArchivable, public BFlattenable { @@ -79,18 +91,19 @@ private: class BNetworkCookieJar::Iterator { public: - Iterator(const Iterator& other); + Iterator(const BNetworkCookieJar* map); ~Iterator(); - bool HasNext() const; - BNetworkCookie* Next(); - BNetworkCookie* NextDomain(); - BNetworkCookie* Remove(); - void RemoveDomain(); Iterator& operator=(const Iterator& other); + bool HasNext() const; + const BNetworkCookie* Next(); + const BNetworkCookie* NextDomain(); + const BNetworkCookie* Remove(); + void RemoveDomain(); + private: - Iterator(const BNetworkCookieJar* map); + Iterator(const Iterator& other); void _FindNext(); @@ -101,26 +114,29 @@ private: PrivateIterator* fIterator; BNetworkCookieList* fLastList; BNetworkCookieList* fList; - BNetworkCookie* fElement; - BNetworkCookie* fLastElement; + const BNetworkCookie* fElement; + const BNetworkCookie* fLastElement; int32 fIndex; }; +// The copy constructor and assignment operator create new iterators for the +// same cookie jar and url. Iteration will start over. class BNetworkCookieJar::UrlIterator { public: UrlIterator(const UrlIterator& other); ~UrlIterator(); bool HasNext() const; - BNetworkCookie* Next(); - BNetworkCookie* Remove(); + const BNetworkCookie* Next(); + const BNetworkCookie* Remove(); UrlIterator& operator=(const UrlIterator& other); private: UrlIterator(const BNetworkCookieJar* map, const BUrl& url); + void _Initialize(); bool _SuperDomain(); void _FindNext(); void _FindDomain(); @@ -133,8 +149,8 @@ private: PrivateIterator* fIterator; BNetworkCookieList* fList; BNetworkCookieList* fLastList; - BNetworkCookie* fElement; - BNetworkCookie* fLastElement; + const BNetworkCookie* fElement; + const BNetworkCookie* fLastElement; int32 fIndex; int32 fLastIndex; diff --git a/src/kits/network/libnetapi/HttpRequest.cpp b/src/kits/network/libnetapi/HttpRequest.cpp index 5d63987238..2905c01450 100644 --- a/src/kits/network/libnetapi/HttpRequest.cpp +++ b/src/kits/network/libnetapi/HttpRequest.cpp @@ -972,7 +972,7 @@ BHttpRequest::_SendHeaders() BNetworkCookieJar::UrlIterator iterator = fContext->GetCookieJar().GetUrlIterator(fUrl); - BNetworkCookie* cookie = iterator.Next(); + const BNetworkCookie* cookie = iterator.Next(); if (cookie != NULL) { while (true) { cookieString << cookie->RawCookie(false); diff --git a/src/kits/network/libnetapi/NetworkCookieJar.cpp b/src/kits/network/libnetapi/NetworkCookieJar.cpp index ba4949030b..8c44ac906a 100644 --- a/src/kits/network/libnetapi/NetworkCookieJar.cpp +++ b/src/kits/network/libnetapi/NetworkCookieJar.cpp @@ -133,19 +133,38 @@ BNetworkCookieJar::AddCookie(BNetworkCookie* cookie) HashString key(cookie->Domain()); + if (!fCookieHashMap->Lock()) + return B_ERROR; + // Get the cookies for the requested domain, or create a new list if there // isn't one yet. - BNetworkCookieList* list = fCookieHashMap->fHashMap.Get(key); + BNetworkCookieList* list = fCookieHashMap->Get(key); if (list == NULL) { list = new(std::nothrow) BNetworkCookieList(); - if (list == NULL || fCookieHashMap->fHashMap.Put(key, list) != B_OK) + + if (list == NULL) { + fCookieHashMap->Unlock(); return B_NO_MEMORY; + } + + if (fCookieHashMap->Put(key, list) != B_OK) { + fCookieHashMap->Unlock(); + delete list; + return B_NO_MEMORY; + } } + if (list->LockForWriting() != B_OK) { + fCookieHashMap->Unlock(); + return B_ERROR; + } + + fCookieHashMap->Unlock(); + // Remove any cookie with the same key as the one we're trying to add (it // replaces/updates them) for (int32 i = 0; i < list->CountItems(); i++) { - BNetworkCookie* c = list->ItemAt(i); + const BNetworkCookie* c = list->ItemAt(i); if (c->Name() == cookie->Name() && c->Path() == cookie->Path()) { list->RemoveItemAt(i); @@ -159,19 +178,28 @@ BNetworkCookieJar::AddCookie(BNetworkCookie* cookie) TRACE("Remove cookie: %s\n", cookie->RawCookie(true).String()); delete cookie; } else { - TRACE("Add cookie: %s\n", cookie->RawCookie(true).String()); + // Make sure the cookie has cached the raw string and expiration date + // string, so it is now actually immutable. This way we can safely + // read the cookie data from multiple threads without any locking. + const BString& raw = cookie->RawCookie(true); + (void)raw; + + TRACE("Add cookie: %s\n", raw.String()); + // Keep the list sorted by path length (longest first). This makes sure // that cookies for most specific paths are returned first when // iterating the cookie jar. int32 i; for (i = 0; i < list->CountItems(); i++) { - BNetworkCookie* current = list->ItemAt(i); + const BNetworkCookie* current = list->ItemAt(i); if (current->Path().Length() < cookie->Path().Length()) break; } list->AddItem(cookie, i); } + list->Unlock(); + return B_OK; } @@ -180,7 +208,7 @@ status_t BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies) { for (int32 i = 0; i < cookies.CountItems(); i++) { - BNetworkCookie* cookiePtr = cookies.ItemAt(i); + const BNetworkCookie* cookiePtr = cookies.ItemAt(i); // Using AddCookie by reference in order to avoid multiple // cookie jar share the same cookie pointers @@ -200,7 +228,7 @@ uint32 BNetworkCookieJar::DeleteOutdatedCookies() { int32 deleteCount = 0; - BNetworkCookie* cookiePtr; + const BNetworkCookie* cookiePtr; for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { if (cookiePtr->ShouldDeleteNow()) { @@ -217,7 +245,7 @@ uint32 BNetworkCookieJar::PurgeForExit() { int32 deleteCount = 0; - BNetworkCookie* cookiePtr; + const BNetworkCookie* cookiePtr; for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { if (cookiePtr->ShouldDeleteAtExit()) { @@ -239,7 +267,7 @@ BNetworkCookieJar::Archive(BMessage* into, bool deep) const status_t error = BArchivable::Archive(into, deep); if (error == B_OK) { - BNetworkCookie* cookiePtr; + const BNetworkCookie* cookiePtr; for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { BMessage subArchive; @@ -392,6 +420,9 @@ BNetworkCookieJar::operator=(const BNetworkCookieJar& other) if (&other == this) return *this; + for (Iterator it = GetIterator(); it.Next() != NULL;) + delete it.Remove(); + BArchivable::operator=(other); BFlattenable::operator=(other); @@ -401,7 +432,7 @@ BNetworkCookieJar::operator=(const BNetworkCookieJar& other) fCookieHashMap = new(std::nothrow) PrivateHashMap(); for (Iterator it = other.GetIterator(); it.HasNext();) { - BNetworkCookie* cookie = it.Next(); + const BNetworkCookie* cookie = it.Next(); AddCookie(*cookie); // Pass by reference so the cookie is copied. } @@ -437,7 +468,7 @@ BNetworkCookieJar::_DoFlatten() const { fFlattened.Truncate(0); - BNetworkCookie* cookiePtr; + const BNetworkCookie* cookiePtr; for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { fFlattened << cookiePtr->Domain() << '\t' << "TRUE" << '\t' << cookiePtr->Path() << '\t' @@ -454,13 +485,17 @@ BNetworkCookieJar::_DoFlatten() const BNetworkCookieJar::Iterator::Iterator(const Iterator& other) : fCookieJar(other.fCookieJar), - fIterator(other.fIterator), - fLastList(other.fLastList), - fList(other.fList), - fElement(other.fElement), - fLastElement(other.fLastElement), - fIndex(other.fIndex) + fIterator(NULL), + fLastList(NULL), + fList(NULL), + fElement(NULL), + fLastElement(NULL), + fIndex(0) { + fIterator = new(std::nothrow) PrivateIterator( + fCookieJar->fCookieHashMap->GetIterator()); + + _FindNext(); } @@ -474,8 +509,11 @@ BNetworkCookieJar::Iterator::Iterator(const BNetworkCookieJar* cookieJar) fLastElement(NULL), fIndex(0) { + if (!fCookieJar->fCookieHashMap->Lock()) + return; + fIterator = new(std::nothrow) PrivateIterator( - fCookieJar->fCookieHashMap->fHashMap.GetIterator()); + fCookieJar->fCookieHashMap->GetIterator()); // Locate first cookie _FindNext(); @@ -484,7 +522,43 @@ BNetworkCookieJar::Iterator::Iterator(const BNetworkCookieJar* cookieJar) BNetworkCookieJar::Iterator::~Iterator() { + if (!fIterator) + return; + + if (fList != NULL) + fList->Unlock(); + delete fIterator; + + fCookieJar->fCookieHashMap->Unlock(); +} + + +BNetworkCookieJar::Iterator& +BNetworkCookieJar::Iterator::operator=(const Iterator& other) +{ + if (this == &other) + return *this; + + fCookieJar->fCookieHashMap->Unlock(); + + fCookieJar = other.fCookieJar; + fIterator = NULL; + fLastList = NULL; + fList = NULL; + fElement = NULL; + fLastElement = NULL; + fIndex = 0; + + if (fCookieJar->fCookieHashMap->Lock()) + { + fIterator = new(std::nothrow) PrivateIterator( + fCookieJar->fCookieHashMap->GetIterator()); + + _FindNext(); + } + + return *this; } @@ -495,32 +569,36 @@ BNetworkCookieJar::Iterator::HasNext() const } -BNetworkCookie* +const BNetworkCookie* BNetworkCookieJar::Iterator::Next() { if (!fElement) return NULL; - BNetworkCookie* result = fElement; + const BNetworkCookie* result = fElement; _FindNext(); return result; } -BNetworkCookie* +const BNetworkCookie* BNetworkCookieJar::Iterator::NextDomain() { if (!fElement) return NULL; - BNetworkCookie* result = fElement; + const BNetworkCookie* result = fElement; if (!fIterator->fCookieMapIterator.HasNext()) { fElement = NULL; return NULL; } + if (fList != NULL) + fList->Unlock(); + fList = *fIterator->fCookieMapIterator.NextValue(); + fList->LockForReading(); fIndex = 0; fElement = fList->ItemAt(fIndex); @@ -528,23 +606,37 @@ BNetworkCookieJar::Iterator::NextDomain() } -BNetworkCookie* +const BNetworkCookie* BNetworkCookieJar::Iterator::Remove() { if (!fLastElement) return NULL; - BNetworkCookie* result = fLastElement; + const BNetworkCookie* result = fLastElement; if (fIndex == 0) { - if (fLastList->CountItems() == 1) { - fIterator->fCookieMapIterator.Remove(); - delete fLastList; - } else - fLastList->RemoveItemAt(fLastList->CountItems() - 1); + if (fLastList->LockForWriting() == B_OK) + { + if (fLastList->CountItems() == 1) { + fIterator->fCookieMapIterator.Remove(); + delete fLastList; + fLastList = NULL; + } else { + fLastList->RemoveItemAt(fLastList->CountItems() - 1); + fLastList->Unlock(); + } + } } else { fIndex--; - fList->RemoveItemAt(fIndex); + + // Switch to a write lock + fList->Unlock(); + if (fList->LockForWriting() == B_OK) + { + fList->RemoveItemAt(fIndex); + fList->Unlock(); + } + fList->LockForReading(); } fLastElement = NULL; @@ -552,26 +644,6 @@ BNetworkCookieJar::Iterator::Remove() } -BNetworkCookieJar::Iterator& -BNetworkCookieJar::Iterator::operator=(const BNetworkCookieJar::Iterator& other) -{ - fCookieJar = other.fCookieJar; - fLastList = other.fLastList; - fList = other.fList; - fElement = other.fElement; - fLastElement = other.fLastElement; - fIndex = other.fIndex; - - fIterator = new(std::nothrow) PrivateIterator(*other.fIterator); - if (fIterator == NULL) { - // Make the iterator unusable. - fElement = NULL; - fLastElement = NULL; - } - return *this; -} - - void BNetworkCookieJar::Iterator::_FindNext() { @@ -589,7 +661,13 @@ BNetworkCookieJar::Iterator::_FindNext() } fLastList = fList; + + if (fList) + fList->Unlock(); + fList = *(fIterator->fCookieMapIterator.NextValue()); + fList->LockForReading(); + fIndex = 0; fElement = fList->ItemAt(fIndex); } @@ -599,8 +677,18 @@ BNetworkCookieJar::Iterator::_FindNext() BNetworkCookieJar::UrlIterator::UrlIterator(const UrlIterator& other) + : + fCookieJar(other.fCookieJar), + fIterator(NULL), + fList(NULL), + fLastList(NULL), + fElement(NULL), + fLastElement(NULL), + fIndex(0), + fLastIndex(0), + fUrl(other.fUrl) { - *this = other; + _Initialize(); } @@ -617,29 +705,15 @@ BNetworkCookieJar::UrlIterator::UrlIterator(const BNetworkCookieJar* cookieJar, fLastIndex(0), fUrl(url) { - BString domain = url.Host(); - - if (!domain.Length()) { - if (url.Protocol() == "file") - domain = "localhost"; - else - return; - } - - fIterator = new(std::nothrow) PrivateIterator( - fCookieJar->fCookieHashMap->fHashMap.GetIterator()); - - if (fIterator != NULL) { - // Prepending a dot since _FindNext is going to call _SupDomain() - domain.Prepend("."); - fIterator->fKey.SetTo(domain, domain.Length()); - _FindNext(); - } + _Initialize(); } BNetworkCookieJar::UrlIterator::~UrlIterator() { + if (fList) + fList->Unlock(); + delete fIterator; } @@ -651,33 +725,38 @@ BNetworkCookieJar::UrlIterator::HasNext() const } -BNetworkCookie* +const BNetworkCookie* BNetworkCookieJar::UrlIterator::Next() { if (!fElement) return NULL; - BNetworkCookie* result = fElement; + const BNetworkCookie* result = fElement; _FindNext(); return result; } -BNetworkCookie* +const BNetworkCookie* BNetworkCookieJar::UrlIterator::Remove() { if (!fLastElement) return NULL; - BNetworkCookie* result = fLastElement; + const BNetworkCookie* result = fLastElement; - fLastList->RemoveItemAt(fLastIndex); + if (fLastList->LockForWriting() == B_OK) + { + fLastList->RemoveItemAt(fLastIndex); - if (fLastList->CountItems() == 0) { - HashString lastKey(fLastElement->Domain(), - fLastElement->Domain().Length()); + if (fLastList->CountItems() == 0) { + HashString lastKey(fLastElement->Domain(), + fLastElement->Domain().Length()); - delete fCookieJar->fCookieHashMap->fHashMap.Remove(lastKey); + delete fCookieJar->fCookieHashMap->Remove(lastKey); + } + + fLastList->Unlock(); } fLastElement = NULL; @@ -689,25 +768,56 @@ BNetworkCookieJar::UrlIterator& BNetworkCookieJar::UrlIterator::operator=( const BNetworkCookieJar::UrlIterator& other) { - fCookieJar = other.fCookieJar; - fList = other.fList; - fLastList = other.fLastList; - fElement = other.fElement; - fLastElement = other.fLastElement; - fIndex = other.fIndex; - fLastIndex = other.fLastIndex; + if (this == &other) + return *this; - fIterator = new(std::nothrow) PrivateIterator(*other.fIterator); - if (fIterator == NULL) { - // Make the iterator unusable. - fElement = NULL; - fLastElement = NULL; - } + // Teardown + if (fList) + fList->Unlock(); + + delete fIterator; + + // Init + fCookieJar = other.fCookieJar; + fIterator = NULL; + fList = NULL; + fLastList = NULL; + fElement = NULL; + fLastElement = NULL; + fIndex = 0; + fLastIndex = 0; + fUrl = other.fUrl; + + _Initialize(); return *this; } +void +BNetworkCookieJar::UrlIterator::_Initialize() +{ + BString domain = fUrl.Host(); + + if (!domain.Length()) { + if (fUrl.Protocol() == "file") + domain = "localhost"; + else + return; + } + + fIterator = new(std::nothrow) PrivateIterator( + fCookieJar->fCookieHashMap->GetIterator()); + + if (fIterator != NULL) { + // Prepending a dot since _FindNext is going to call _SupDomain() + domain.Prepend("."); + fIterator->fKey.SetTo(domain, domain.Length()); + _FindNext(); + } +} + + bool BNetworkCookieJar::UrlIterator::_SuperDomain() { @@ -747,10 +857,18 @@ BNetworkCookieJar::UrlIterator::_FindNext() void BNetworkCookieJar::UrlIterator::_FindDomain() { - fList = fCookieJar->fCookieHashMap->fHashMap.Get(fIterator->fKey); + if (fList != NULL) + fList->Unlock(); - if (fList == NULL) - fElement = NULL; + if (fCookieJar->fCookieHashMap->Lock()) { + fList = fCookieJar->fCookieHashMap->Get(fIterator->fKey); + + if (fList == NULL) + fElement = NULL; + else + fList->LockForReading(); + fCookieJar->fCookieHashMap->Unlock(); + } fIndex = -1; } @@ -771,3 +889,40 @@ BNetworkCookieJar::UrlIterator::_FindPath() return false; } + + +// #pragma mark - BNetworkCookieList + + +BNetworkCookieList::BNetworkCookieList() +{ + pthread_rwlock_init(&fLock, NULL); +} + + +BNetworkCookieList::~BNetworkCookieList() +{ + pthread_rwlock_destroy(&fLock); +} + + +status_t +BNetworkCookieList::LockForReading() +{ + return pthread_rwlock_rdlock(&fLock); +} + + +status_t +BNetworkCookieList::LockForWriting() +{ + return pthread_rwlock_wrlock(&fLock); +} + + +status_t +BNetworkCookieList::Unlock() +{ + return pthread_rwlock_unlock(&fLock); +} + diff --git a/src/kits/network/libnetapi/NetworkCookieJarPrivate.h b/src/kits/network/libnetapi/NetworkCookieJarPrivate.h index 2f5f6b96ef..6751496175 100644 --- a/src/kits/network/libnetapi/NetworkCookieJarPrivate.h +++ b/src/kits/network/libnetapi/NetworkCookieJarPrivate.h @@ -5,10 +5,14 @@ #ifndef _B_NETWORK_COOKIE_JAR_PRIVATE_H_ #define _B_NETWORK_COOKIE_JAR_PRIVATE_H_ -typedef BPrivate::HashMap BNetworkCookieHashMap; -struct BNetworkCookieJar::PrivateHashMap { - BNetworkCookieHashMap fHashMap; +#include + + +typedef BPrivate::SynchronizedHashMap + BNetworkCookieHashMap; + +struct BNetworkCookieJar::PrivateHashMap : public BNetworkCookieHashMap { }; struct BNetworkCookieJar::PrivateIterator {