From 2e7b5f9e185297670adc17f3aada0a4af6a48838 Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Sat, 26 Jan 2013 19:23:34 +0000 Subject: [PATCH 1/9] NetworkCookieJar: don't leak a new cookie if it has expired --- .../network/libnetapi/NetworkCookieJar.cpp | 188 +++++++++--------- 1 file changed, 93 insertions(+), 95 deletions(-) diff --git a/src/kits/network/libnetapi/NetworkCookieJar.cpp b/src/kits/network/libnetapi/NetworkCookieJar.cpp index eef91d110b..5b8a94583b 100644 --- a/src/kits/network/libnetapi/NetworkCookieJar.cpp +++ b/src/kits/network/libnetapi/NetworkCookieJar.cpp @@ -20,14 +20,14 @@ const char* kArchivedCookieMessageName = "be:cookie"; BNetworkCookieJar::BNetworkCookieJar() - : + : fCookieHashMap(new PrivateHashMap) { } BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieJar&) - : + : BArchivable(), fCookieHashMap(new PrivateHashMap) { @@ -36,7 +36,7 @@ BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieJar&) BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieList& otherList) - : + : fCookieHashMap(new PrivateHashMap) { AddCookies(otherList); @@ -44,18 +44,18 @@ BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieList& otherList) BNetworkCookieJar::BNetworkCookieJar(BMessage* archive) - : + : fCookieHashMap(new PrivateHashMap) { BMessage extractedCookie; - + for (int32 i = 0; archive->FindMessage(kArchivedCookieMessageName, i, &extractedCookie) == B_OK; i++) { - BNetworkCookie* heapCookie + BNetworkCookie* heapCookie = new(std::nothrow) BNetworkCookie(&extractedCookie); - + if (heapCookie == NULL || !AddCookie(heapCookie)) break; } @@ -65,7 +65,7 @@ BNetworkCookieJar::BNetworkCookieJar(BMessage* archive) BNetworkCookieJar::~BNetworkCookieJar() { BNetworkCookie* cookiePtr; - + for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) delete it.Remove(); } @@ -78,12 +78,12 @@ bool BNetworkCookieJar::AddCookie(const BNetworkCookie& cookie) { BNetworkCookie* heapCookie = new(std::nothrow) BNetworkCookie(cookie); - + if (!AddCookie(heapCookie)) { delete heapCookie; return false; } - + return true; } @@ -91,29 +91,27 @@ BNetworkCookieJar::AddCookie(const BNetworkCookie& cookie) bool BNetworkCookieJar::AddCookie(BNetworkCookie* cookie) { - if (cookie != NULL) { - HashString key(cookie->Domain()); - - if (!fCookieHashMap->fHashMap.ContainsKey(key)) - fCookieHashMap->fHashMap.Put(key, new BList); - - BNetworkCookieList* list = fCookieHashMap->fHashMap.Get(key); - - for (int32 i = 0; i < list->CountItems(); i++) { - BNetworkCookie* c - = reinterpret_cast(list->ItemAt(i)); - - if (c->Name() == cookie->Name()) { - list->RemoveItem(i); - break; - } + if (cookie == NULL || cookie->ShouldDeleteNow()) + return false; + + HashString key(cookie->Domain()); + + if (!fCookieHashMap->fHashMap.ContainsKey(key)) + fCookieHashMap->fHashMap.Put(key, new BList); + + BNetworkCookieList* list = fCookieHashMap->fHashMap.Get(key); + + for (int32 i = 0; i < list->CountItems(); i++) { + BNetworkCookie* c + = reinterpret_cast(list->ItemAt(i)); + + if (c->Name() == cookie->Name()) { + list->RemoveItem(i); + break; } - - // Discard the cookie if it's to be deleted - if (!cookie->ShouldDeleteNow()) - list->AddItem(cookie); } - + + list->AddItem(cookie); return true; } @@ -122,15 +120,15 @@ bool BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies) { for (int32 i = 0; i < cookies.CountItems(); i++) { - BNetworkCookie* cookiePtr + BNetworkCookie* cookiePtr = reinterpret_cast(cookies.ItemAt(i)); - + // Using AddCookie by reference in order to avoid multiple // cookie jar share the same cookie pointers if (!AddCookie(*cookiePtr)) return false; } - + return true; } @@ -143,8 +141,8 @@ BNetworkCookieJar::DeleteOutdatedCookies() { int32 deleteCount = 0; BNetworkCookie* cookiePtr; - - for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) { + + for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { if (cookiePtr->ShouldDeleteNow()) { delete it.Remove(); deleteCount++; @@ -160,8 +158,8 @@ BNetworkCookieJar::PurgeForExit() { int32 deleteCount = 0; BNetworkCookie* cookiePtr; - - for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) { + + for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { if (cookiePtr->ShouldDeleteAtExit()) { delete it.Remove(); deleteCount++; @@ -182,10 +180,10 @@ BNetworkCookieJar::Archive(BMessage* into, bool deep) const if (error == B_OK) { BNetworkCookie* cookiePtr; - - for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) { + + for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { BMessage subArchive; - + error = cookiePtr->Archive(&subArchive, deep); if (error != B_OK) return error; @@ -205,7 +203,7 @@ BNetworkCookieJar::Instantiate(BMessage* archive) { if (archive->HasMessage(kArchivedCookieMessageName)) return new(std::nothrow) BNetworkCookieJar(archive); - + return NULL; } @@ -242,11 +240,11 @@ BNetworkCookieJar::Flatten(void* buffer, ssize_t size) const { if (FlattenedSize() > size) return B_ERROR; - - fFlattened.CopyInto(reinterpret_cast(buffer), 0, + + fFlattened.CopyInto(reinterpret_cast(buffer), 0, fFlattened.Length()); reinterpret_cast(buffer)[fFlattened.Length()] = 0; - + return B_OK; } @@ -262,13 +260,13 @@ BNetworkCookieJar::AllowsTypeCode(type_code) const status_t BNetworkCookieJar::Unflatten(type_code, const void* buffer, ssize_t size) { - BString flattenedCookies; + BString flattenedCookies; flattenedCookies.SetTo(reinterpret_cast(buffer), size); - - while (flattenedCookies.Length() > 0) { + + while (flattenedCookies.Length() > 0) { BNetworkCookie tempCookie; BString tempCookieLine; - + int32 endOfLine = flattenedCookies.FindFirst('\n', 0); if (endOfLine == -1) tempCookieLine = flattenedCookies; @@ -276,11 +274,11 @@ BNetworkCookieJar::Unflatten(type_code, const void* buffer, ssize_t size) flattenedCookies.MoveInto(tempCookieLine, 0, endOfLine); flattenedCookies.Remove(0, 1); } - + if (tempCookieLine.Length() != 0 && tempCookieLine[0] != '#') { for (int32 field = 0; field < 7; field++) { BString tempString; - + int32 endOfField = tempCookieLine.FindFirst('\t', 0); if (endOfField == -1) tempString = tempCookieLine; @@ -288,42 +286,42 @@ BNetworkCookieJar::Unflatten(type_code, const void* buffer, ssize_t size) tempCookieLine.MoveInto(tempString, 0, endOfField); tempCookieLine.Remove(0, 1); } - + switch (field) { case 0: tempCookie.SetDomain(tempString); break; - + case 1: // TODO: Useless field ATM break; - + case 2: tempCookie.SetPath(tempString); break; - + case 3: tempCookie.SetSecure(tempString == "TRUE"); break; - + case 4: tempCookie.SetExpirationDate(atoi(tempString)); break; - + case 5: tempCookie.SetName(tempString); break; - + case 6: tempCookie.SetValue(tempString); break; } // switch } // for loop - + AddCookie(tempCookie); } - } - + } + return B_OK; } @@ -346,7 +344,7 @@ BNetworkCookieJar::GetUrlIterator(const BUrl& url) const copy.SetPath("/"); return BNetworkCookieJar::UrlIterator(this, copy); } - + return BNetworkCookieJar::UrlIterator(this, url); } @@ -357,10 +355,10 @@ BNetworkCookieJar::_DoFlatten() const fFlattened.Truncate(0); BNetworkCookie* cookiePtr; - for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) { - fFlattened << cookiePtr->Domain() << '\t' << "TRUE" << '\t' - << cookiePtr->Path() << '\t' - << (cookiePtr->Secure()?"TRUE":"FALSE") << '\t' + for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) { + fFlattened << cookiePtr->Domain() << '\t' << "TRUE" << '\t' + << cookiePtr->Path() << '\t' + << (cookiePtr->Secure()?"TRUE":"FALSE") << '\t' << (int32)cookiePtr->ExpirationDate() << '\t' << cookiePtr->Name() << '\t' << cookiePtr->Value() << '\n'; } @@ -395,7 +393,7 @@ BNetworkCookieJar::Iterator::Iterator(const BNetworkCookieJar* cookieJar) { fIterator = new(std::nothrow) PrivateIterator( fCookieJar->fCookieHashMap->fHashMap.GetIterator()); - + // Locate first cookie _FindNext(); } @@ -419,7 +417,7 @@ BNetworkCookieJar::Iterator::Next() { if (!fElement) return NULL; - + BNetworkCookie* result = fElement; _FindNext(); return result; @@ -431,18 +429,18 @@ BNetworkCookieJar::Iterator::NextDomain() { if (!fElement) return NULL; - + BNetworkCookie* result = fElement; - + if (!fIterator->fCookieMapIterator.HasNext()) { fElement = NULL; return NULL; } - + fList = *(fIterator->fCookieMapIterator.NextValue()); fIndex = 0; fElement = reinterpret_cast(fList->ItemAt(fIndex)); - + return result; } @@ -452,7 +450,7 @@ BNetworkCookieJar::Iterator::Remove() { if (!fLastElement) return NULL; - + BNetworkCookie* result = fLastElement; if (fIndex == 0) { @@ -466,7 +464,7 @@ BNetworkCookieJar::Iterator::Remove() fList->RemoveItem(fIndex-1); fIndex--; } - + fLastElement = NULL; return result; } @@ -490,18 +488,18 @@ void BNetworkCookieJar::Iterator::_FindNext() { fLastElement = fElement; - + fIndex++; if (fList && fIndex < fList->CountItems()) { fElement = reinterpret_cast(fList->ItemAt(fIndex)); return; } - + if (!fIterator->fCookieMapIterator.HasNext()) { fElement = NULL; return; } - + fLastList = fList; fList = *(fIterator->fCookieMapIterator.NextValue()); fIndex = 0; @@ -538,11 +536,11 @@ BNetworkCookieJar::UrlIterator::UrlIterator(const BNetworkCookieJar* cookieJar, if (domain[0] != '.') domain.Prepend("."); - + // Prepending another dot since _FindNext is going to // call _SupDomain() domain.Prepend("."); - + fIterator = new(std::nothrow) PrivateIterator( fCookieJar->fCookieHashMap->fHashMap.GetIterator()); fIterator->fKey.SetTo(domain, domain.Length()); @@ -569,7 +567,7 @@ BNetworkCookieJar::UrlIterator::Next() { if (!fElement) return NULL; - + BNetworkCookie* result = fElement; _FindNext(); return result; @@ -581,15 +579,15 @@ BNetworkCookieJar::UrlIterator::Remove() { if (!fLastElement) return NULL; - + BNetworkCookie* result = fLastElement; fLastList->RemoveItem(fLastIndex); if (fLastList->CountItems() == 0) { - HashString lastKey(fLastElement->Domain(), + HashString lastKey(fLastElement->Domain(), fLastElement->Domain().Length()); - + delete fCookieJar->fCookieHashMap->fHashMap.Remove(lastKey); } @@ -620,10 +618,10 @@ BNetworkCookieJar::UrlIterator::_SupDomain() { BString domain(fIterator->fKey.GetString()); int32 nextDot = domain.FindFirst('.', 1); - + if (nextDot == -1) return false; - + domain.Remove(0, nextDot); fIterator->fKey.SetTo(domain.String(), domain.Length()); return true; @@ -635,17 +633,17 @@ BNetworkCookieJar::UrlIterator::_FindNext() { fLastIndex = fIndex; fLastElement = fElement; - + if (_FindPath()) return; - + fLastList = fList; do { if (!_SupDomain()) { fElement = NULL; return; } - + _FindDomain(); } while (!_FindPath()); } @@ -655,10 +653,10 @@ void BNetworkCookieJar::UrlIterator::_FindDomain() { fList = fCookieJar->fCookieHashMap->fHashMap.Get(fIterator->fKey); - + if (fList == NULL) fElement = NULL; - + fIndex = -1; } @@ -669,15 +667,15 @@ BNetworkCookieJar::UrlIterator::_FindPath() fIndex++; if (fList && fIndex < fList->CountItems()) { do { - fElement + fElement = reinterpret_cast(fList->ItemAt(fIndex)); - - if (fElement->IsValidForPath(fUrl.Path())) + + if (fElement->IsValidForPath(fUrl.Path())) return true; - + fIndex++; } while (fList && fIndex < fList->CountItems()); } - + return false; } From 05f42aaba4c6b4ad6e023e029e1e2b3c55ec4d25 Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Wed, 30 Jan 2013 14:35:11 +0000 Subject: [PATCH 2/9] NetworkCookie: set the default path according to RFC 6265 --- headers/os/net/NetworkCookie.h | 27 ++-- src/kits/network/libnetapi/NetworkCookie.cpp | 130 +++++++++++-------- 2 files changed, 89 insertions(+), 68 deletions(-) diff --git a/headers/os/net/NetworkCookie.h b/headers/os/net/NetworkCookie.h index 168c9b10e4..9ba5bc3be9 100644 --- a/headers/os/net/NetworkCookie.h +++ b/headers/os/net/NetworkCookie.h @@ -24,13 +24,13 @@ public: BNetworkCookie(BMessage* archive); BNetworkCookie(); virtual ~BNetworkCookie(); - + // Parse a "SetCookie" string, or "name=value" - - BNetworkCookie& ParseCookieStringFromUrl(const BString& string, + + BNetworkCookie& ParseCookieStringFromUrl(const BString& string, const BUrl& url); BNetworkCookie& ParseCookieString(const BString& cookieString); - + // Modify the cookie fields BNetworkCookie& SetComment(const BString& comment); BNetworkCookie& SetCommentUrl(const BString& commentUrl); @@ -44,7 +44,7 @@ public: BNetworkCookie& SetVersion(int8 version); BNetworkCookie& SetName(const BString& name); BNetworkCookie& SetValue(const BString& value); - + // Access the cookie fields const BString& CommentUrl() const; const BString& Comment() const; @@ -64,7 +64,7 @@ public: bool IsValidForUrl(const BUrl& url) const; bool IsValidForDomain(const BString& domain) const; bool IsValidForPath(const BString& path) const; - + // Test if cookie fields are defined bool HasCommentUrl() const; bool HasComment() const; @@ -76,16 +76,16 @@ public: bool HasVersion() const; bool HasName() const; bool HasValue() const; - + // Test if cookie could be deleted bool ShouldDeleteAtExit() const; bool ShouldDeleteNow() const; - + // BArchivable members virtual status_t Archive(BMessage* into, bool deep = true) const; static BArchivable* Instantiate(BMessage* archive); - + // Overloaded operators BNetworkCookie& operator=(const BNetworkCookie& other); BNetworkCookie& operator=(const char* string); @@ -94,15 +94,16 @@ public: private: void _Reset(); void _ExtractNameValuePair( - const BString& cookieString, int16* index, + const BString& cookieString, int16* index, bool parseField = false); - + void _SetDefaultPathForUrl(const BUrl& url); + private: mutable BString fRawCookie; mutable bool fRawCookieValid; mutable BString fRawFullCookie; mutable bool fRawFullCookieValid; - + BString fComment; BString fCommentUrl; bool fDiscard; @@ -115,7 +116,7 @@ private: int8 fVersion; BString fName; BString fValue; - + bool fHasDiscard; bool fHasExpirationDate; bool fSessionCookie; diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp b/src/kits/network/libnetapi/NetworkCookie.cpp index 16fad675fa..475a21a751 100644 --- a/src/kits/network/libnetapi/NetworkCookie.cpp +++ b/src/kits/network/libnetapi/NetworkCookie.cpp @@ -32,7 +32,7 @@ static const char* kArchivedCookieValue = "be:cookie.value"; BNetworkCookie::BNetworkCookie(const char* name, const char* value) - : + : fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), fVersion(0), @@ -45,7 +45,7 @@ BNetworkCookie::BNetworkCookie(const char* name, const char* value) BNetworkCookie::BNetworkCookie(const BNetworkCookie& other) - : + : BArchivable(), fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), @@ -58,7 +58,7 @@ BNetworkCookie::BNetworkCookie(const BNetworkCookie& other) BNetworkCookie::BNetworkCookie(const BString& cookieString) - : + : fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), fVersion(0), @@ -71,7 +71,7 @@ BNetworkCookie::BNetworkCookie(const BString& cookieString) BNetworkCookie::BNetworkCookie(const BString& cookieString, const BUrl& url) - : + : fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), fVersion(0), @@ -83,29 +83,29 @@ BNetworkCookie::BNetworkCookie(const BString& cookieString, BNetworkCookie::BNetworkCookie(BMessage* archive) - : + : fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), fVersion(0), fSessionCookie(true) { _Reset(); - + archive->FindString(kArchivedCookieName, &fName); archive->FindString(kArchivedCookieValue, &fValue); - + archive->FindString(kArchivedCookieComment, &fComment); archive->FindString(kArchivedCookieCommentUrl, &fCommentUrl); archive->FindString(kArchivedCookieDomain, &fDomain); archive->FindString(kArchivedCookiePath, &fPath); archive->FindBool(kArchivedCookieSecure, &fSecure); - + if (archive->FindBool(kArchivedCookieDiscard, &fDiscard) == B_OK) fHasDiscard = true; - + if (archive->FindInt8(kArchivedCookieVersion, &fVersion) == B_OK) fHasVersion = true; - + int32 expiration; if (archive->FindInt32(kArchivedCookieExpirationDate, &expiration) == B_OK) { @@ -115,7 +115,7 @@ BNetworkCookie::BNetworkCookie(BMessage* archive) BNetworkCookie::BNetworkCookie() - : + : fDiscard(false), fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), fPath("/"), @@ -140,19 +140,19 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string, { BString cookieString(string); int16 index = 0; - + _Reset(); - + // Default values from url SetDomain(url.Host()); - SetPath(url.Path()); - + _SetDefaultPathForUrl(url); + _ExtractNameValuePair(cookieString, &index); while (index < cookieString.Length()) _ExtractNameValuePair(cookieString, &index, true); - - return *this; + + return *this; } @@ -200,11 +200,11 @@ BNetworkCookie& BNetworkCookie::SetDomain(const BString& domain) { fDomain = domain; - + // We always use pre-dotted domains for tail matching if (fDomain.ByteAt(0) != '.') fDomain.Prepend("."); - + fRawFullCookieValid = false; return *this; } @@ -244,7 +244,7 @@ BNetworkCookie::SetExpirationDate(BDateTime& expireDate) fRawFullCookieValid = false; fHasExpirationDate = true; } - + return *this; } @@ -346,12 +346,12 @@ const BString& BNetworkCookie::ExpirationString() const { BHttpTime date(ExpirationDate()); - + if (!fExpirationStringValid) { fExpirationString = date.ToString(BPrivate::B_HTTP_TIME_FORMAT_COOKIE); fExpirationStringValid = true; } - + return fExpirationString; } @@ -397,9 +397,9 @@ BNetworkCookie::RawCookie(bool full) const if (full && !fRawFullCookieValid) { fRawFullCookie.Truncate(0); fRawFullCookieValid = true; - + fRawFullCookie << fName << "=" << fValue; - + if (HasCommentUrl()) fRawFullCookie << "; Comment-Url=" << fCommentUrl; if (HasComment()) @@ -417,11 +417,11 @@ BNetworkCookie::RawCookie(bool full) const fRawFullCookie << "; Secure=" << (fSecure?"true":"false"); if (HasVersion()) fRawFullCookie << ", Version=" << fVersion; - + } else if (!full && !fRawCookieValid) { fRawCookie.Truncate(0); fRawCookieValid = true; - + fRawCookie << fName << "=" << fValue; } @@ -451,7 +451,7 @@ BNetworkCookie::IsValidForUrl(const BUrl& url) const { BString urlHost = url.Host(); BString urlPath = url.Path(); - + return IsValidForDomain(urlHost) && IsValidForPath(urlPath); } @@ -461,7 +461,7 @@ BNetworkCookie::IsValidForDomain(const BString& domain) const { if (fDomain.Length() > domain.Length()) return false; - + return domain.FindLast(fDomain) == (domain.Length() - fDomain.Length()); } @@ -471,7 +471,7 @@ BNetworkCookie::IsValidForPath(const BString& path) const { if (fPath.Length() > path.Length()) return false; - + return path.FindFirst(fPath) == 0; } @@ -559,7 +559,7 @@ BNetworkCookie::ShouldDeleteNow() const { if (!IsSessionCookie() && HasExpirationDate()) return (BDateTime::CurrentDateTime(B_GMT_TIME) > fExpiration); - + return false; } @@ -574,60 +574,60 @@ BNetworkCookie::Archive(BMessage* into, bool deep) const if (error != B_OK) return error; - + error = into->AddString(kArchivedCookieName, fName); if (error != B_OK) return error; - + error = into->AddString(kArchivedCookieValue, fValue); if (error != B_OK) return error; - - + + // We add optional fields only if they're defined if (HasComment()) { error = into->AddString(kArchivedCookieComment, fComment); if (error != B_OK) return error; - } - + } + if (HasCommentUrl()) { error = into->AddString(kArchivedCookieCommentUrl, fCommentUrl); if (error != B_OK) return error; } - + if (HasDiscard()) { error = into->AddBool(kArchivedCookieDiscard, fDiscard); if (error != B_OK) return error; } - + if (HasDomain()) { error = into->AddString(kArchivedCookieDomain, fDomain); if (error != B_OK) return error; } - + if (fHasExpirationDate) { - error = into->AddInt32(kArchivedCookieExpirationDate, + error = into->AddInt32(kArchivedCookieExpirationDate, fExpiration.Time_t()); if (error != B_OK) return error; } - + if (HasPath()) { error = into->AddString(kArchivedCookiePath, fPath); if (error != B_OK) return error; } - + if (Secure()) { error = into->AddBool(kArchivedCookieSecure, fSecure); if (error != B_OK) return error; } - + if (HasVersion()) { error = into->AddInt8(kArchivedCookieVersion, fVersion); if (error != B_OK) @@ -652,7 +652,7 @@ BNetworkCookie::Instantiate(BMessage* archive) // #pragma mark Overloaded operators -BNetworkCookie& +BNetworkCookie& BNetworkCookie::operator=(const BNetworkCookie& other) { // Should we prefer to discard the cache ? @@ -662,7 +662,7 @@ BNetworkCookie::operator=(const BNetworkCookie& other) fRawFullCookieValid = other.fRawFullCookieValid; fExpirationString = other.fExpirationString; fExpirationStringValid = other.fExpirationStringValid; - + fComment = other.fComment; fCommentUrl = other.fCommentUrl; fDiscard = other.fDiscard; @@ -673,12 +673,12 @@ BNetworkCookie::operator=(const BNetworkCookie& other) fVersion = other.fVersion; fName = other.fName; fValue = other.fValue; - + fHasDiscard = other.fHasDiscard; fHasExpirationDate = other.fHasExpirationDate; fSessionCookie = other.fSessionCookie; fHasVersion = other.fHasVersion; - + return *this; } @@ -723,7 +723,7 @@ BNetworkCookie::_Reset() fHasExpirationDate = false; fSessionCookie = true; fHasVersion = false; - + fRawCookieValid = false; fRawFullCookieValid = false; fExpirationStringValid = false; @@ -731,32 +731,32 @@ BNetworkCookie::_Reset() void -BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, +BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, int16* index, bool parseField) { // Skip whitespaces while (cookieString.ByteAt(*index) == ' ' && *index < cookieString.Length()) (*index)++; - + if (*index >= cookieString.Length()) return; - + // Look for a name=value pair int16 firstSemiColon = cookieString.FindFirst(";", *index); int16 firstEqual = cookieString.FindFirst("=", *index); - + BString name; BString value; - + if (firstSemiColon == -1) { if (firstEqual != -1) { cookieString.CopyInto(name, *index, firstEqual - *index); cookieString.CopyInto(value, firstEqual + 1, cookieString.Length() - firstEqual - 1); } else - cookieString.CopyInto(value, *index, + cookieString.CopyInto(value, *index, cookieString.Length() - *index); *index = cookieString.Length() + 1; @@ -799,7 +799,7 @@ BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, BHttpTime date(value); SetExpirationDate(date.Parse()); // Cookie valid domain - } else if (name == "domain") + } else if (name == "domain") SetDomain(value); // Cookie valid path else if (name == "path") @@ -811,3 +811,23 @@ BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, else if (name == "version") SetVersion(atoi(value.String())); } + + +void +BNetworkCookie::_SetDefaultPathForUrl(const BUrl& url) +{ + const BString& path = url.Path(); + if (path.IsEmpty() || path.ByteAt(0) != '/') { + SetPath("/"); + return; + } + + int32 index = path.FindLast('/'); + if (index == 0) { + SetPath("/"); + return; + } + + BString newPath = path; + SetPath(newPath.Truncate(index)); +} From 2db5d2bc95b8b37ce6f2cd0547cd5e6336992388 Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Wed, 30 Jan 2013 16:49:37 +0000 Subject: [PATCH 3/9] NetworkCookie: remove unused cookie attributes and add HttpOnly --- headers/os/net/NetworkCookie.h | 46 +-- src/kits/network/libnetapi/NetworkCookie.cpp | 282 ++++--------------- 2 files changed, 71 insertions(+), 257 deletions(-) diff --git a/headers/os/net/NetworkCookie.h b/headers/os/net/NetworkCookie.h index 9ba5bc3be9..5a6fd0139f 100644 --- a/headers/os/net/NetworkCookie.h +++ b/headers/os/net/NetworkCookie.h @@ -32,33 +32,27 @@ public: BNetworkCookie& ParseCookieString(const BString& cookieString); // Modify the cookie fields - BNetworkCookie& SetComment(const BString& comment); - BNetworkCookie& SetCommentUrl(const BString& commentUrl); - BNetworkCookie& SetDiscard(bool discard); + BNetworkCookie& SetName(const BString& name); + BNetworkCookie& SetValue(const BString& value); BNetworkCookie& SetDomain(const BString& domain); + BNetworkCookie& SetPath(const BString& path); BNetworkCookie& SetMaxAge(int32 maxAge); BNetworkCookie& SetExpirationDate(time_t expireDate); BNetworkCookie& SetExpirationDate(BDateTime& expireDate); - BNetworkCookie& SetPath(const BString& path); BNetworkCookie& SetSecure(bool secure); - BNetworkCookie& SetVersion(int8 version); - BNetworkCookie& SetName(const BString& name); - BNetworkCookie& SetValue(const BString& value); + BNetworkCookie& SetHttpOnly(bool httpOnly); // Access the cookie fields - const BString& CommentUrl() const; - const BString& Comment() const; - bool Discard() const; - const BString& Domain() const; - int32 MaxAge() const; - time_t ExpirationDate() const; - const BString& ExpirationString() const; - const BString& Path() const; - bool Secure() const; - int8 Version() const; const BString& Name() const; const BString& Value() const; + const BString& Domain() const; + const BString& Path() const; + time_t ExpirationDate() const; + const BString& ExpirationString() const; + bool Secure() const; + bool HttpOnly() const; const BString& RawCookie(bool full) const; + bool IsSessionCookie() const; bool IsValid(bool strict = false) const; bool IsValidForUrl(const BUrl& url) const; @@ -66,16 +60,11 @@ public: bool IsValidForPath(const BString& path) const; // Test if cookie fields are defined - bool HasCommentUrl() const; - bool HasComment() const; - bool HasDiscard() const; - bool HasDomain() const; - bool HasMaxAge() const; - bool HasExpirationDate() const; - bool HasPath() const; - bool HasVersion() const; bool HasName() const; bool HasValue() const; + bool HasDomain() const; + bool HasPath() const; + bool HasExpirationDate() const; // Test if cookie could be deleted bool ShouldDeleteAtExit() const; @@ -104,23 +93,18 @@ private: mutable BString fRawFullCookie; mutable bool fRawFullCookieValid; - BString fComment; - BString fCommentUrl; - bool fDiscard; BString fDomain; BDateTime fExpiration; mutable BString fExpirationString; mutable bool fExpirationStringValid; BString fPath; bool fSecure; - int8 fVersion; + bool fHttpOnly; BString fName; BString fValue; - bool fHasDiscard; bool fHasExpirationDate; bool fSessionCookie; - bool fHasVersion; }; #endif // _B_NETWORK_COOKIE_H_ diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp b/src/kits/network/libnetapi/NetworkCookie.cpp index 475a21a751..ca86851553 100644 --- a/src/kits/network/libnetapi/NetworkCookie.cpp +++ b/src/kits/network/libnetapi/NetworkCookie.cpp @@ -7,35 +7,30 @@ */ -#include -#include #include +#include +#include +#include + #include #include -#include #define PRINT(x) printf x; using BPrivate::BHttpTime; -static const char* kArchivedCookieComment = "be:cookie.comment"; -static const char* kArchivedCookieCommentUrl = "be:cookie.commenturl"; -static const char* kArchivedCookieDiscard = "be:cookie.discard"; -static const char* kArchivedCookieDomain = "be:cookie.domain"; -static const char* kArchivedCookieExpirationDate = "be:cookie.expiredate"; -static const char* kArchivedCookiePath = "be:cookie.path"; -static const char* kArchivedCookieSecure = "be:cookie.secure"; -static const char* kArchivedCookieVersion = "be:cookie.version"; -static const char* kArchivedCookieName = "be:cookie.name"; -static const char* kArchivedCookieValue = "be:cookie.value"; +static const char* kArchivedCookieName = "be:cookie.name"; +static const char* kArchivedCookieValue = "be:cookie.value"; +static const char* kArchivedCookieDomain = "be:cookie.domain"; +static const char* kArchivedCookiePath = "be:cookie.path"; +static const char* kArchivedCookieExpirationDate = "be:cookie.expirationdate"; +static const char* kArchivedCookieSecure = "be:cookie.secure"; +static const char* kArchivedCookieHttpOnly = "be:cookie.httponly"; BNetworkCookie::BNetworkCookie(const char* name, const char* value) : - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fVersion(0), fName(name), fValue(value), fSessionCookie(true) @@ -45,12 +40,6 @@ BNetworkCookie::BNetworkCookie(const char* name, const char* value) BNetworkCookie::BNetworkCookie(const BNetworkCookie& other) - : - BArchivable(), - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fVersion(0), - fSessionCookie(true) { _Reset(); *this = other; @@ -58,11 +47,6 @@ BNetworkCookie::BNetworkCookie(const BNetworkCookie& other) BNetworkCookie::BNetworkCookie(const BString& cookieString) - : - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fVersion(0), - fSessionCookie(true) { _Reset(); ParseCookieString(cookieString); @@ -71,11 +55,6 @@ BNetworkCookie::BNetworkCookie(const BString& cookieString) BNetworkCookie::BNetworkCookie(const BString& cookieString, const BUrl& url) - : - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fVersion(0), - fSessionCookie(true) { _Reset(); ParseCookieStringFromUrl(cookieString, url); @@ -84,9 +63,6 @@ BNetworkCookie::BNetworkCookie(const BString& cookieString, BNetworkCookie::BNetworkCookie(BMessage* archive) : - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fVersion(0), fSessionCookie(true) { _Reset(); @@ -94,17 +70,10 @@ BNetworkCookie::BNetworkCookie(BMessage* archive) archive->FindString(kArchivedCookieName, &fName); archive->FindString(kArchivedCookieValue, &fValue); - archive->FindString(kArchivedCookieComment, &fComment); - archive->FindString(kArchivedCookieCommentUrl, &fCommentUrl); archive->FindString(kArchivedCookieDomain, &fDomain); archive->FindString(kArchivedCookiePath, &fPath); archive->FindBool(kArchivedCookieSecure, &fSecure); - - if (archive->FindBool(kArchivedCookieDiscard, &fDiscard) == B_OK) - fHasDiscard = true; - - if (archive->FindInt8(kArchivedCookieVersion, &fVersion) == B_OK) - fHasVersion = true; + archive->FindBool(kArchivedCookieHttpOnly, &fHttpOnly); int32 expiration; if (archive->FindInt32(kArchivedCookieExpirationDate, &expiration) @@ -115,12 +84,6 @@ BNetworkCookie::BNetworkCookie(BMessage* archive) BNetworkCookie::BNetworkCookie() - : - fDiscard(false), - fExpiration(BDateTime::CurrentDateTime(B_GMT_TIME)), - fPath("/"), - fVersion(0), - fSessionCookie(true) { _Reset(); } @@ -169,28 +132,29 @@ BNetworkCookie::ParseCookieString(const BString& string) BNetworkCookie& -BNetworkCookie::SetComment(const BString& comment) +BNetworkCookie::SetName(const BString& name) { - fComment = comment; + fName = name; fRawFullCookieValid = false; + fRawCookieValid = false; return *this; } BNetworkCookie& -BNetworkCookie::SetCommentUrl(const BString& commentUrl) +BNetworkCookie::SetValue(const BString& value) { - fCommentUrl = commentUrl; + fValue = value; fRawFullCookieValid = false; + fRawCookieValid = false; return *this; } BNetworkCookie& -BNetworkCookie::SetDiscard(bool discard) +BNetworkCookie::SetPath(const BString& path) { - fDiscard = discard; - fHasDiscard = true; + fPath = path; fRawFullCookieValid = false; return *this; } @@ -249,15 +213,6 @@ BNetworkCookie::SetExpirationDate(BDateTime& expireDate) } -BNetworkCookie& -BNetworkCookie::SetPath(const BString& path) -{ - fPath = path; - fRawFullCookieValid = false; - return *this; -} - - BNetworkCookie& BNetworkCookie::SetSecure(bool secure) { @@ -268,31 +223,10 @@ BNetworkCookie::SetSecure(bool secure) BNetworkCookie& -BNetworkCookie::SetVersion(int8 version) +BNetworkCookie::SetHttpOnly(bool httpOnly) { - fVersion = version; - fHasVersion = true; - fRawCookieValid = false; - return *this; -} - - -BNetworkCookie& -BNetworkCookie::SetName(const BString& name) -{ - fName = name; + fHttpOnly = httpOnly; fRawFullCookieValid = false; - fRawCookieValid = false; - return *this; -} - - -BNetworkCookie& -BNetworkCookie::SetValue(const BString& value) -{ - fValue = value; - fRawFullCookieValid = false; - fRawCookieValid = false; return *this; } @@ -301,23 +235,16 @@ BNetworkCookie::SetValue(const BString& value) const BString& -BNetworkCookie::Comment() const +BNetworkCookie::Name() const { - return fComment; + return fName; } const BString& -BNetworkCookie::CommentUrl() const +BNetworkCookie::Value() const { - return fCommentUrl; -} - - -bool -BNetworkCookie::Discard() const -{ - return fDiscard; + return fValue; } @@ -328,10 +255,10 @@ BNetworkCookie::Domain() const } -int32 -BNetworkCookie::MaxAge() const +const BString& +BNetworkCookie::Path() const { - return fExpiration.Time_t() - BDateTime::CurrentDateTime(B_GMT_TIME).Time_t(); + return fPath; } @@ -356,13 +283,6 @@ BNetworkCookie::ExpirationString() const } -const BString& -BNetworkCookie::Path() const -{ - return fPath; -} - - bool BNetworkCookie::Secure() const { @@ -370,24 +290,10 @@ BNetworkCookie::Secure() const } -int8 -BNetworkCookie::Version() const +bool +BNetworkCookie::HttpOnly() const { - return fVersion; -} - - -const BString& -BNetworkCookie::Name() const -{ - return fName; -} - - -const BString& -BNetworkCookie::Value() const -{ - return fValue; + return fHttpOnly; } @@ -400,23 +306,16 @@ BNetworkCookie::RawCookie(bool full) const fRawFullCookie << fName << "=" << fValue; - if (HasCommentUrl()) - fRawFullCookie << "; Comment-Url=" << fCommentUrl; - if (HasComment()) - fRawFullCookie << "; Comment=" << fComment; - if (HasDiscard()) - fRawFullCookie << "; Discard=" << (fDiscard?"true":"false"); if (HasDomain()) fRawFullCookie << "; Domain=" << fDomain; if (HasExpirationDate()) - fRawFullCookie << "; Max-Age=" << MaxAge(); -// fRawFullCookie << "; Expires=" << ExpirationString(); + fRawFullCookie << "; Expires=" << ExpirationString(); if (HasPath()) fRawFullCookie << "; Path=" << fPath; - if (Secure() && fSecure) - fRawFullCookie << "; Secure=" << (fSecure?"true":"false"); - if (HasVersion()) - fRawFullCookie << ", Version=" << fVersion; + if (Secure()) + fRawFullCookie << "; Secure"; + if (HttpOnly()) + fRawFullCookie << "; HttpOnly"; } else if (!full && !fRawCookieValid) { fRawCookie.Truncate(0); @@ -425,7 +324,7 @@ BNetworkCookie::RawCookie(bool full) const fRawCookie << fName << "=" << fValue; } - return full?fRawFullCookie:fRawCookie; + return full ? fRawFullCookie : fRawCookie; } @@ -442,13 +341,14 @@ BNetworkCookie::IsSessionCookie() const bool BNetworkCookie::IsValid(bool strict) const { - return HasName() && HasValue() && (!strict || HasVersion()); + return HasName() && HasValue(); } bool BNetworkCookie::IsValidForUrl(const BUrl& url) const { + // TODO: Take secure attribute into account BString urlHost = url.Host(); BString urlPath = url.Path(); @@ -480,23 +380,16 @@ BNetworkCookie::IsValidForPath(const BString& path) const bool -BNetworkCookie::HasCommentUrl() const +BNetworkCookie::HasName() const { - return fCommentUrl.Length() > 0; + return fName.Length() > 0; } bool -BNetworkCookie::HasComment() const +BNetworkCookie::HasValue() const { - return fComment.Length() > 0; -} - - -bool -BNetworkCookie::HasDiscard() const -{ - return fHasDiscard; + return fValue.Length() > 0; } @@ -514,27 +407,6 @@ BNetworkCookie::HasPath() const } -bool -BNetworkCookie::HasVersion() const -{ - return fHasVersion; -} - - -bool -BNetworkCookie::HasName() const -{ - return fName.Length() > 0; -} - - -bool -BNetworkCookie::HasValue() const -{ - return fValue.Length() > 0; -} - - bool BNetworkCookie::HasExpirationDate() const { @@ -548,16 +420,14 @@ BNetworkCookie::HasExpirationDate() const bool BNetworkCookie::ShouldDeleteAtExit() const { - return (HasDiscard() && Discard()) - || (!IsSessionCookie() && ShouldDeleteNow()) - || IsSessionCookie(); + return IsSessionCookie() || ShouldDeleteNow(); } bool BNetworkCookie::ShouldDeleteNow() const { - if (!IsSessionCookie() && HasExpirationDate()) + if (HasExpirationDate()) return (BDateTime::CurrentDateTime(B_GMT_TIME) > fExpiration); return false; @@ -585,24 +455,6 @@ BNetworkCookie::Archive(BMessage* into, bool deep) const // We add optional fields only if they're defined - if (HasComment()) { - error = into->AddString(kArchivedCookieComment, fComment); - if (error != B_OK) - return error; - } - - if (HasCommentUrl()) { - error = into->AddString(kArchivedCookieCommentUrl, fCommentUrl); - if (error != B_OK) - return error; - } - - if (HasDiscard()) { - error = into->AddBool(kArchivedCookieDiscard, fDiscard); - if (error != B_OK) - return error; - } - if (HasDomain()) { error = into->AddString(kArchivedCookieDomain, fDomain); if (error != B_OK) @@ -628,8 +480,8 @@ BNetworkCookie::Archive(BMessage* into, bool deep) const return error; } - if (HasVersion()) { - error = into->AddInt8(kArchivedCookieVersion, fVersion); + if (HttpOnly()) { + error = into->AddBool(kArchivedCookieHttpOnly, fHttpOnly); if (error != B_OK) return error; } @@ -663,21 +515,16 @@ BNetworkCookie::operator=(const BNetworkCookie& other) fExpirationString = other.fExpirationString; fExpirationStringValid = other.fExpirationStringValid; - fComment = other.fComment; - fCommentUrl = other.fCommentUrl; - fDiscard = other.fDiscard; - fDomain = other.fDomain; - fExpiration = other.fExpiration; - fPath = other.fPath; - fSecure = other.fSecure; - fVersion = other.fVersion; fName = other.fName; fValue = other.fValue; + fDomain = other.fDomain; + fPath = other.fPath; + fExpiration = other.fExpiration; + fSecure = other.fSecure; + fHttpOnly = other.fHttpOnly; - fHasDiscard = other.fHasDiscard; fHasExpirationDate = other.fHasExpirationDate; fSessionCookie = other.fSessionCookie; - fHasVersion = other.fHasVersion; return *this; } @@ -708,21 +555,16 @@ BNetworkCookie::operator!=(const BNetworkCookie& other) void BNetworkCookie::_Reset() { - fComment.Truncate(0); - fCommentUrl.Truncate(0); fDomain.Truncate(0); fPath.Truncate(0); fName.Truncate(0); fValue.Truncate(0); - fDiscard = false; fSecure = false; - fVersion = 0; - fExpiration = 0; + fHttpOnly = false; + fExpiration = BDateTime(); - fHasDiscard = false; fHasExpirationDate = false; fSessionCookie = true; - fHasVersion = false; fRawCookieValid = false; fRawFullCookieValid = false; @@ -782,17 +624,8 @@ BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, name.Trim(); value.Trim(); - // Cookie comment - if (name == "comment") - SetComment(value); - // Cookie comment URL - else if (name == "comment-url") - SetCommentUrl(value); - // Cookie discard flag - else if (name == "discard") - SetDiscard(value.Length() == 0 || value.ToLower() == "true"); // Cookie max-age - else if (name == "maxage") + if (name == "maxage") SetMaxAge(atoi(value.String())); // Cookie expiration date else if (name == "expires") { @@ -807,9 +640,6 @@ BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, // Cookie secure flag else if (name == "secure") SetSecure(value.Length() == 0 || value.ToLower() == "true"); - // Cookie version - else if (name == "version") - SetVersion(atoi(value.String())); } From 33462ef54a6c481491c9716726619b5bb25c0dcf Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Thu, 31 Jan 2013 18:53:25 +0000 Subject: [PATCH 4/9] NetworkCookie: bring SetCookie parsing in line with RFC 6265 --- headers/os/net/NetworkCookie.h | 30 +- src/kits/network/libnetapi/NetworkCookie.cpp | 329 +++++++++++-------- 2 files changed, 202 insertions(+), 157 deletions(-) diff --git a/headers/os/net/NetworkCookie.h b/headers/os/net/NetworkCookie.h index 5a6fd0139f..7db429f19c 100644 --- a/headers/os/net/NetworkCookie.h +++ b/headers/os/net/NetworkCookie.h @@ -17,7 +17,6 @@ class BNetworkCookie : public BArchivable { public: BNetworkCookie(const char* name, const char* value); - BNetworkCookie(const BNetworkCookie& other); BNetworkCookie(const BString& cookieString); BNetworkCookie(const BString& cookieString, const BUrl& url); @@ -25,7 +24,7 @@ public: BNetworkCookie(); virtual ~BNetworkCookie(); - // Parse a "SetCookie" string, or "name=value" + // Parse a "SetCookie" string BNetworkCookie& ParseCookieStringFromUrl(const BString& string, const BUrl& url); @@ -53,6 +52,7 @@ public: bool HttpOnly() const; const BString& RawCookie(bool full) const; + bool IsHostOnly() const; bool IsSessionCookie() const; bool IsValid(bool strict = false) const; bool IsValidForUrl(const BUrl& url) const; @@ -76,34 +76,36 @@ public: static BArchivable* Instantiate(BMessage* archive); // Overloaded operators - BNetworkCookie& operator=(const BNetworkCookie& other); BNetworkCookie& operator=(const char* string); bool operator==(const BNetworkCookie& other); bool operator!=(const BNetworkCookie& other); private: void _Reset(); - void _ExtractNameValuePair( - const BString& cookieString, int16* index, - bool parseField = false); - void _SetDefaultPathForUrl(const BUrl& url); + int32 _ExtractNameValuePair(const BString& string, + BString& name, BString& value, + int32 index); + int32 _ExtractAttributeValuePair( + const BString& string, BString& name, + BString& value, int32 index); + BString _DefaultPathForUrl(const BUrl& url); private: mutable BString fRawCookie; mutable bool fRawCookieValid; mutable BString fRawFullCookie; mutable bool fRawFullCookieValid; - - BString fDomain; - BDateTime fExpiration; mutable BString fExpirationString; mutable bool fExpirationStringValid; - BString fPath; - bool fSecure; - bool fHttpOnly; + BString fName; BString fValue; + BString fDomain; + BString fPath; + BDateTime fExpiration; + bool fSecure; + bool fHttpOnly; - bool fHasExpirationDate; + bool fHostOnly; bool fSessionCookie; }; diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp b/src/kits/network/libnetapi/NetworkCookie.cpp index ca86851553..a06335a9e6 100644 --- a/src/kits/network/libnetapi/NetworkCookie.cpp +++ b/src/kits/network/libnetapi/NetworkCookie.cpp @@ -1,9 +1,10 @@ /* - * Copyright 2010 Haiku Inc. All rights reserved. + * Copyright 2010-2013 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. * * Authors: * Christophe Huriaux, c.huriaux@gmail.com + * Hamish Morrison, hamishm53@gmail.com */ @@ -13,11 +14,10 @@ #include #include +#include #include #include -#define PRINT(x) printf x; - using BPrivate::BHttpTime; static const char* kArchivedCookieName = "be:cookie.name"; @@ -27,22 +27,14 @@ static const char* kArchivedCookiePath = "be:cookie.path"; static const char* kArchivedCookieExpirationDate = "be:cookie.expirationdate"; static const char* kArchivedCookieSecure = "be:cookie.secure"; static const char* kArchivedCookieHttpOnly = "be:cookie.httponly"; +static const char* kArchivedCookieHostOnly = "be:cookie.hostonly"; BNetworkCookie::BNetworkCookie(const char* name, const char* value) - : - fName(name), - fValue(value), - fSessionCookie(true) { _Reset(); -} - - -BNetworkCookie::BNetworkCookie(const BNetworkCookie& other) -{ - _Reset(); - *this = other; + fName = name; + fValue = value; } @@ -62,8 +54,6 @@ BNetworkCookie::BNetworkCookie(const BString& cookieString, BNetworkCookie::BNetworkCookie(BMessage* archive) - : - fSessionCookie(true) { _Reset(); @@ -74,6 +64,7 @@ BNetworkCookie::BNetworkCookie(BMessage* archive) archive->FindString(kArchivedCookiePath, &fPath); archive->FindBool(kArchivedCookieSecure, &fSecure); archive->FindBool(kArchivedCookieHttpOnly, &fHttpOnly); + archive->FindBool(kArchivedCookieHostOnly, &fHostOnly); int32 expiration; if (archive->FindInt32(kArchivedCookieExpirationDate, &expiration) @@ -101,19 +92,61 @@ BNetworkCookie& BNetworkCookie::ParseCookieStringFromUrl(const BString& string, const BUrl& url) { - BString cookieString(string); - int16 index = 0; - _Reset(); - // Default values from url - SetDomain(url.Host()); - _SetDefaultPathForUrl(url); + BString name; + BString value; + int32 index = 0; - _ExtractNameValuePair(cookieString, &index); + // Parse the name and value of the cookie + index = _ExtractNameValuePair(string, name, value, index); + // The set-cookie-string is not valid + if (index == -1) + return *this; - while (index < cookieString.Length()) - _ExtractNameValuePair(cookieString, &index, true); + SetName(name); + SetValue(value); + + // Parse the remaining cookie attributes + while (index < string.Length()) { + ASSERT(string[index] == ';'); + index++; + + index = _ExtractAttributeValuePair(string, name, value, index); + + if (name.ICompare("secure") == 0) + SetSecure(true); + else if (name.ICompare("httponly") == 0) + SetHttpOnly(true); + + // The following attributes require a value + if (value.IsEmpty()) + continue; + + if (name.ICompare("max-age") == 0) { + // Validate the max-age value + char* end = NULL; + long maxAge = strtol(value.String(), &end, 10); + if (*end == '\0') + SetMaxAge((int)maxAge); + } else if (name.ICompare("expires") == 0) { + BHttpTime date(value); + SetExpirationDate(date.Parse()); + } else if (name.ICompare("domain") == 0) + SetDomain(value); + else if (name.ICompare("path") == 0) + SetPath(value); + } + + // If no domain was specified, we set a host-only domain from the URL + if (!HasDomain()) { + SetDomain(url.Host()); + fHostOnly = true; + } + + // If no path was specified we compute the default path from the URL + if (!HasPath()) + SetPath(_DefaultPathForUrl(url)); return *this; } @@ -164,6 +197,7 @@ BNetworkCookie& BNetworkCookie::SetDomain(const BString& domain) { fDomain = domain; + fHostOnly = false; // We always use pre-dotted domains for tail matching if (fDomain.ByteAt(0) != '.') @@ -200,13 +234,11 @@ BNetworkCookie::SetExpirationDate(BDateTime& expireDate) fSessionCookie = true; fExpirationStringValid = false; fRawFullCookieValid = false; - fHasExpirationDate = false; } else { fExpiration = expireDate; fSessionCookie = false; fExpirationStringValid = false; fRawFullCookieValid = false; - fHasExpirationDate = true; } return *this; @@ -331,6 +363,13 @@ BNetworkCookie::RawCookie(bool full) const // #pragma mark Cookie test +bool +BNetworkCookie::IsHostOnly() const +{ + return fHostOnly; +} + + bool BNetworkCookie::IsSessionCookie() const { @@ -410,7 +449,7 @@ BNetworkCookie::HasPath() const bool BNetworkCookie::HasExpirationDate() const { - return fHasExpirationDate; + return !IsSessionCookie(); } @@ -461,7 +500,7 @@ BNetworkCookie::Archive(BMessage* into, bool deep) const return error; } - if (fHasExpirationDate) { + if (HasExpirationDate()) { error = into->AddInt32(kArchivedCookieExpirationDate, fExpiration.Time_t()); if (error != B_OK) @@ -486,6 +525,12 @@ BNetworkCookie::Archive(BMessage* into, bool deep) const return error; } + if (IsHostOnly()) { + error = into->AddBool(kArchivedCookieHostOnly, true); + if (error != B_OK) + return error; + } + return B_OK; } @@ -504,32 +549,6 @@ BNetworkCookie::Instantiate(BMessage* archive) // #pragma mark Overloaded operators -BNetworkCookie& -BNetworkCookie::operator=(const BNetworkCookie& other) -{ - // Should we prefer to discard the cache ? - fRawCookie = other.fRawCookie; - fRawCookieValid = other.fRawCookieValid; - fRawFullCookie = other.fRawFullCookie; - fRawFullCookieValid = other.fRawFullCookieValid; - fExpirationString = other.fExpirationString; - fExpirationStringValid = other.fExpirationStringValid; - - fName = other.fName; - fValue = other.fValue; - fDomain = other.fDomain; - fPath = other.fPath; - fExpiration = other.fExpiration; - fSecure = other.fSecure; - fHttpOnly = other.fHttpOnly; - - fHasExpirationDate = other.fHasExpirationDate; - fSessionCookie = other.fSessionCookie; - - return *this; -} - - BNetworkCookie& BNetworkCookie::operator=(const char* string) { @@ -555,109 +574,133 @@ BNetworkCookie::operator!=(const BNetworkCookie& other) void BNetworkCookie::_Reset() { - fDomain.Truncate(0); - fPath.Truncate(0); fName.Truncate(0); fValue.Truncate(0); - fSecure = false; - fHttpOnly = false; - fExpiration = BDateTime(); + fDomain.Truncate(0); + fPath.Truncate(0); + fExpiration = BDateTime(); + fSecure = false; + fHttpOnly = false; - fHasExpirationDate = false; - fSessionCookie = true; + fSessionCookie = true; + fHostOnly = true; - fRawCookieValid = false; - fRawFullCookieValid = false; - fExpirationStringValid = false; + fRawCookieValid = false; + fRawFullCookieValid = false; + fExpirationStringValid = false; } -void -BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, - int16* index, bool parseField) +int32 +skip_whitespace_forward(const BString& string, int32 index) { - // Skip whitespaces - while (cookieString.ByteAt(*index) == ' ' - && *index < cookieString.Length()) - (*index)++; - - if (*index >= cookieString.Length()) - return; - - - // Look for a name=value pair - int16 firstSemiColon = cookieString.FindFirst(";", *index); - int16 firstEqual = cookieString.FindFirst("=", *index); - - BString name; - BString value; - - if (firstSemiColon == -1) { - if (firstEqual != -1) { - cookieString.CopyInto(name, *index, firstEqual - *index); - cookieString.CopyInto(value, firstEqual + 1, - cookieString.Length() - firstEqual - 1); - } else - cookieString.CopyInto(value, *index, - cookieString.Length() - *index); - - *index = cookieString.Length() + 1; - } else { - if (firstEqual != -1 && firstEqual < firstSemiColon) { - cookieString.CopyInto(name, *index, firstEqual - *index); - cookieString.CopyInto(value, firstEqual + 1, - firstSemiColon - firstEqual - 1); - } else - cookieString.CopyInto(value, *index, firstSemiColon - *index); - - *index = firstSemiColon + 1; - } - - // Cookie name/value pair - if (!parseField) { - SetName(name); - SetValue(value); - return; - } - - name.ToLower(); - name.Trim(); - value.Trim(); - - // Cookie max-age - if (name == "maxage") - SetMaxAge(atoi(value.String())); - // Cookie expiration date - else if (name == "expires") { - BHttpTime date(value); - SetExpirationDate(date.Parse()); - // Cookie valid domain - } else if (name == "domain") - SetDomain(value); - // Cookie valid path - else if (name == "path") - SetPath(value); - // Cookie secure flag - else if (name == "secure") - SetSecure(value.Length() == 0 || value.ToLower() == "true"); + while (index < string.Length() && (string[index] == ' ' + || string[index] == '\t')) + index++; + return index; } -void -BNetworkCookie::_SetDefaultPathForUrl(const BUrl& url) +int32 +skip_whitespace_backward(const BString& string, int32 index) +{ + while (index >= 0 && (string[index] == ' ' || string[index] == '\t')) + index--; + return index; +} + + +int32 +BNetworkCookie::_ExtractNameValuePair(const BString& cookieString, + BString& name, BString& value, int32 index) +{ + // Find our name-value-pair and the delimiter. + int32 firstEquals = cookieString.FindFirst('=', index); + int32 nameValueEnd = cookieString.FindFirst(';', index); + + // If the set-cookie-string lacks a semicolon, the name-value-pair + // is the whole string. + if (nameValueEnd == -1) + nameValueEnd = cookieString.Length(); + + // If the name-value-pair lacks an equals, the parse should fail. + if (firstEquals == -1 || firstEquals > nameValueEnd) + return -1; + + int32 first = skip_whitespace_forward(cookieString, index); + int32 last = skip_whitespace_backward(cookieString, firstEquals - 1); + + // If we lack a name, fail to parse. + if (first > last) + return -1; + + cookieString.CopyInto(name, first, last - first + 1); + + first = skip_whitespace_forward(cookieString, firstEquals + 1); + last = skip_whitespace_backward(cookieString, nameValueEnd - 1); + if (first <= last) + cookieString.CopyInto(value, first, last - first + 1); + else + value.SetTo(""); + + return nameValueEnd; +} + + +int32 +BNetworkCookie::_ExtractAttributeValuePair(const BString& cookieString, + BString& attribute, BString& value, int32 index) +{ + // Find the end of our cookie-av. + int32 cookieAVEnd = cookieString.FindFirst(';', index); + + // If the unparsed-attributes lacks a semicolon, then the cookie-av is the + // whole string. + if (cookieAVEnd == -1) + cookieAVEnd = cookieString.Length(); + + int32 attributeNameEnd = cookieString.FindFirst('=', index); + // If the cookie-av has no equals, the attribute-name is the entire + // cookie-av and the attribute-value is empty. + if (attributeNameEnd == -1 || attributeNameEnd > cookieAVEnd) + attributeNameEnd = cookieAVEnd; + + int32 first = skip_whitespace_forward(cookieString, index); + int32 last = skip_whitespace_backward(cookieString, attributeNameEnd - 1); + + if (first <= last) + cookieString.CopyInto(attribute, first, last - first + 1); + else + attribute.SetTo(""); + + if (attributeNameEnd == cookieAVEnd) { + value.SetTo(""); + return cookieAVEnd; + } + + first = skip_whitespace_forward(cookieString, attributeNameEnd + 1); + last = skip_whitespace_backward(cookieString, cookieAVEnd - 1); + if (first <= last) + cookieString.CopyInto(value, first, last - first + 1); + else + value.SetTo(""); + + return cookieAVEnd; +} + + +BString +BNetworkCookie::_DefaultPathForUrl(const BUrl& url) { const BString& path = url.Path(); - if (path.IsEmpty() || path.ByteAt(0) != '/') { - SetPath("/"); - return; - } + if (path.IsEmpty() || path.ByteAt(0) != '/') + return ""; int32 index = path.FindLast('/'); - if (index == 0) { - SetPath("/"); - return; - } + if (index == 0) + return ""; BString newPath = path; - SetPath(newPath.Truncate(index)); + newPath.Truncate(index); + return newPath; } From c8bc218363618030e5db44eb268a1da456596a9f Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Mon, 4 Feb 2013 00:06:42 +0000 Subject: [PATCH 5/9] NetworkCookie: fix domain/path matching, and validity checks --- headers/os/net/NetworkCookie.h | 2 +- src/kits/network/libnetapi/NetworkCookie.cpp | 49 +++++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/headers/os/net/NetworkCookie.h b/headers/os/net/NetworkCookie.h index 7db429f19c..dc9b78d9c1 100644 --- a/headers/os/net/NetworkCookie.h +++ b/headers/os/net/NetworkCookie.h @@ -54,7 +54,7 @@ public: bool IsHostOnly() const; bool IsSessionCookie() const; - bool IsValid(bool strict = false) const; + bool IsValid() const; bool IsValidForUrl(const BUrl& url) const; bool IsValidForDomain(const BString& domain) const; bool IsValidForPath(const BString& path) const; diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp b/src/kits/network/libnetapi/NetworkCookie.cpp index a06335a9e6..e4a5311dd5 100644 --- a/src/kits/network/libnetapi/NetworkCookie.cpp +++ b/src/kits/network/libnetapi/NetworkCookie.cpp @@ -378,16 +378,18 @@ BNetworkCookie::IsSessionCookie() const bool -BNetworkCookie::IsValid(bool strict) const +BNetworkCookie::IsValid() const { - return HasName() && HasValue(); + return HasName() && HasDomain() && HasPath(); } bool BNetworkCookie::IsValidForUrl(const BUrl& url) const { - // TODO: Take secure attribute into account + if (IsSecure() && url.Protocol() != "https") + return false; + BString urlHost = url.Host(); BString urlPath = url.Path(); @@ -398,20 +400,53 @@ BNetworkCookie::IsValidForUrl(const BUrl& url) const bool BNetworkCookie::IsValidForDomain(const BString& domain) const { - if (fDomain.Length() > domain.Length()) + // TODO: canonicalize both domains + const BString& cookieDomain = Domain(); + + int32 difference = domain.Length() - cookieDomain.Length(); + // If the cookie domain is longer than the domain string it cannot + // be valid. + if (difference < 0) return false; - return domain.FindLast(fDomain) == (domain.Length() - fDomain.Length()); + // If the cookie is host-only the domains must match exactly. + if (IsHostOnly()) + return domain == cookieDomain; + + // Otherwise, the domains must match exactly, or the cookie domain + // must be a suffix with the preceeding character being a dot. + const char* suffix = domain.String() + difference; + if (strcmp(suffix, cookieDomain.String()) == 0) { + if (difference == 0) + return true; + else if (domain[difference - 1] == '.') + return true; + } + + return false; } bool BNetworkCookie::IsValidForPath(const BString& path) const { - if (fPath.Length() > path.Length()) + const BString& cookiePath = Path(); + if (path.Length() < cookiePath.Length()) return false; - return path.FindFirst(fPath) == 0; + // The cookie path must be a prefix of the path string + if (path.Compare(cookiePath, cookiePath.Length()) != 0) + 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; } From 64a1f5a020afbbb203b72bfb07e1b08d38431bfe Mon Sep 17 00:00:00 2001 From: Hamish Morrison Date: Thu, 7 Feb 2013 17:59:43 +0000 Subject: [PATCH 6/9] NetworkCookieJar: various small fixes and updated tests --- headers/os/net/NetworkCookieJar.h | 27 ++- src/kits/network/libnetapi/NetworkCookie.cpp | 45 ++--- .../network/libnetapi/NetworkCookieJar.cpp | 155 ++++++++++-------- src/tests/kits/net/cookie/cookie_test.cpp | 123 ++++++++------ 4 files changed, 198 insertions(+), 152 deletions(-) diff --git a/headers/os/net/NetworkCookieJar.h b/headers/os/net/NetworkCookieJar.h index 31cd196a5b..9c3e775f7d 100644 --- a/headers/os/net/NetworkCookieJar.h +++ b/headers/os/net/NetworkCookieJar.h @@ -1,5 +1,5 @@ /* - * Copyright 2010 Haiku Inc. All rights reserved. + * Copyright 2010-2013 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. */ #ifndef _B_NETWORK_COOKIE_JAR_H_ @@ -24,7 +24,7 @@ public: class UrlIterator; struct PrivateIterator; struct PrivateHashMap; - + public: BNetworkCookieJar(); BNetworkCookieJar( @@ -34,16 +34,15 @@ public: BNetworkCookieJar(BMessage* archive); virtual ~BNetworkCookieJar(); - bool AddCookie(const BNetworkCookie& cookie); - bool AddCookie(BNetworkCookie* cookie); - bool AddCookies( - const BNetworkCookieList& cookies); + status_t AddCookie(const BNetworkCookie& cookie); + status_t AddCookie(BNetworkCookie* cookie); + status_t AddCookies(const BNetworkCookieList& cookies); uint32 DeleteOutdatedCookies(); uint32 PurgeForExit(); // BArchivable members - virtual status_t Archive(BMessage* into, + virtual status_t Archive(BMessage* into, bool deep = true) const; static BArchivable* Instantiate(BMessage* archive); @@ -54,20 +53,20 @@ public: virtual status_t Flatten(void* buffer, ssize_t size) const; virtual bool AllowsTypeCode(type_code code) const; - virtual status_t Unflatten(type_code code, + virtual status_t Unflatten(type_code code, const void* buffer, ssize_t size); // Iterators Iterator GetIterator() const; UrlIterator GetUrlIterator(const BUrl& url) const; - + private: void _DoFlatten() const; - + private: friend class Iterator; friend class UrlIterator; - + PrivateHashMap* fCookieHashMap; mutable BString fFlattened; }; @@ -117,7 +116,7 @@ private: UrlIterator(const BNetworkCookieJar* map, const BUrl& url); - bool _SupDomain(); + bool _SuperDomain(); void _FindNext(); void _FindDomain(); bool _FindPath(); @@ -131,10 +130,10 @@ private: BNetworkCookieList* fLastList; BNetworkCookie* fElement; BNetworkCookie* fLastElement; - + int32 fIndex; int32 fLastIndex; - + BUrl fUrl; }; diff --git a/src/kits/network/libnetapi/NetworkCookie.cpp b/src/kits/network/libnetapi/NetworkCookie.cpp index e4a5311dd5..25b5e5c36d 100644 --- a/src/kits/network/libnetapi/NetworkCookie.cpp +++ b/src/kits/network/libnetapi/NetworkCookie.cpp @@ -100,14 +100,15 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string, // Parse the name and value of the cookie index = _ExtractNameValuePair(string, name, value, index); - // The set-cookie-string is not valid - if (index == -1) + if (index == -1) { + // The set-cookie-string is not valid return *this; + } SetName(name); SetValue(value); - // Parse the remaining cookie attributes + // Parse the remaining cookie attributes. while (index < string.Length()) { ASSERT(string[index] == ';'); index++; @@ -119,12 +120,12 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string, else if (name.ICompare("httponly") == 0) SetHttpOnly(true); - // The following attributes require a value + // The following attributes require a value. if (value.IsEmpty()) continue; if (name.ICompare("max-age") == 0) { - // Validate the max-age value + // Validate the max-age value. char* end = NULL; long maxAge = strtol(value.String(), &end, 10); if (*end == '\0') @@ -132,20 +133,31 @@ BNetworkCookie::ParseCookieStringFromUrl(const BString& string, } else if (name.ICompare("expires") == 0) { BHttpTime date(value); SetExpirationDate(date.Parse()); - } else if (name.ICompare("domain") == 0) + } else if (name.ICompare("domain") == 0) { SetDomain(value); - else if (name.ICompare("path") == 0) + } else if (name.ICompare("path") == 0) { SetPath(value); + } } - // If no domain was specified, we set a host-only domain from the URL + // 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 *this; + } + // We should also reject cookies with domains that match public + // suffixes. } - // If no path was specified we compute the default path from the URL - if (!HasPath()) + // 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; @@ -187,6 +199,7 @@ BNetworkCookie::SetValue(const BString& value) BNetworkCookie& BNetworkCookie::SetPath(const BString& path) { + // TODO: canonicalize the path fPath = path; fRawFullCookieValid = false; return *this; @@ -196,13 +209,10 @@ BNetworkCookie::SetPath(const BString& path) BNetworkCookie& BNetworkCookie::SetDomain(const BString& domain) { + // TODO: canonicalize the domain fDomain = domain; fHostOnly = false; - // We always use pre-dotted domains for tail matching - if (fDomain.ByteAt(0) != '.') - fDomain.Prepend("."); - fRawFullCookieValid = false; return *this; } @@ -387,13 +397,10 @@ BNetworkCookie::IsValid() const bool BNetworkCookie::IsValidForUrl(const BUrl& url) const { - if (IsSecure() && url.Protocol() != "https") + if (Secure() && url.Protocol() != "https") return false; - BString urlHost = url.Host(); - BString urlPath = url.Path(); - - return IsValidForDomain(urlHost) && IsValidForPath(urlPath); + return IsValidForDomain(url.Host()) && IsValidForPath(url.Path()); } diff --git a/src/kits/network/libnetapi/NetworkCookieJar.cpp b/src/kits/network/libnetapi/NetworkCookieJar.cpp index 5b8a94583b..cfe3efdb6a 100644 --- a/src/kits/network/libnetapi/NetworkCookieJar.cpp +++ b/src/kits/network/libnetapi/NetworkCookieJar.cpp @@ -1,35 +1,37 @@ /* - * Copyright 2010 Haiku Inc. All rights reserved. + * Copyright 2010-2013 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. * * Authors: * Christophe Huriaux, c.huriaux@gmail.com + * Hamish Morrison, hamishm53@gmail.com */ -#include - #include #include #include #include #include + +#include + #include "NetworkCookieJarPrivate.h" -const char* kArchivedCookieMessageName = "be:cookie"; + +const char* kArchivedCookieMessageName = "be:cookie"; BNetworkCookieJar::BNetworkCookieJar() : - fCookieHashMap(new PrivateHashMap) + fCookieHashMap(new PrivateHashMap()) { } BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieJar&) : - BArchivable(), - fCookieHashMap(new PrivateHashMap) + fCookieHashMap(new PrivateHashMap()) { // TODO } @@ -37,7 +39,7 @@ BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieJar&) BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieList& otherList) : - fCookieHashMap(new PrivateHashMap) + fCookieHashMap(new PrivateHashMap()) { AddCookies(otherList); } @@ -45,19 +47,22 @@ BNetworkCookieJar::BNetworkCookieJar(const BNetworkCookieList& otherList) BNetworkCookieJar::BNetworkCookieJar(BMessage* archive) : - fCookieHashMap(new PrivateHashMap) + fCookieHashMap(new PrivateHashMap()) { BMessage extractedCookie; - for (int32 i = 0; - archive->FindMessage(kArchivedCookieMessageName, i, &extractedCookie) - == B_OK; - i++) { + for (int32 i = 0; archive->FindMessage(kArchivedCookieMessageName, i, + &extractedCookie) == B_OK; i++) { BNetworkCookie* heapCookie = new(std::nothrow) BNetworkCookie(&extractedCookie); - if (heapCookie == NULL || !AddCookie(heapCookie)) + if (heapCookie == NULL) break; + + if (!AddCookie(heapCookie)) { + delete heapCookie; + continue; + } } } @@ -66,7 +71,7 @@ BNetworkCookieJar::~BNetworkCookieJar() { BNetworkCookie* cookiePtr; - for (Iterator it(GetIterator()); (cookiePtr = it.Next()); ) + for (Iterator it = GetIterator(); (cookiePtr = it.Next()) != NULL;) delete it.Remove(); } @@ -74,49 +79,58 @@ BNetworkCookieJar::~BNetworkCookieJar() // #pragma mark Add cookie to cookie jar -bool +status_t BNetworkCookieJar::AddCookie(const BNetworkCookie& cookie) { BNetworkCookie* heapCookie = new(std::nothrow) BNetworkCookie(cookie); + if (heapCookie == NULL) + return B_NO_MEMORY; - if (!AddCookie(heapCookie)) { + status_t result = AddCookie(heapCookie); + if (result != B_OK) { delete heapCookie; - return false; + return result; } - return true; + return B_OK; } -bool +status_t BNetworkCookieJar::AddCookie(BNetworkCookie* cookie) { - if (cookie == NULL || cookie->ShouldDeleteNow()) - return false; + if (cookie == NULL) + return B_BAD_VALUE; HashString key(cookie->Domain()); - if (!fCookieHashMap->fHashMap.ContainsKey(key)) - fCookieHashMap->fHashMap.Put(key, new BList); - BNetworkCookieList* list = fCookieHashMap->fHashMap.Get(key); + if (list == NULL) { + list = new(std::nothrow) BNetworkCookieList(); + if (list == NULL || fCookieHashMap->fHashMap.Put(key, list) != B_OK) + return B_NO_MEMORY; + } for (int32 i = 0; i < list->CountItems(); i++) { BNetworkCookie* c = reinterpret_cast(list->ItemAt(i)); - if (c->Name() == cookie->Name()) { + if (c->Name() == cookie->Name() && c->Path() == cookie->Path()) { list->RemoveItem(i); break; } } - list->AddItem(cookie); - return true; + if (cookie->ShouldDeleteNow()) + delete cookie; + else + list->AddItem(cookie); + + return B_OK; } -bool +status_t BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies) { for (int32 i = 0; i < cookies.CountItems(); i++) { @@ -125,11 +139,12 @@ BNetworkCookieJar::AddCookies(const BNetworkCookieList& cookies) // Using AddCookie by reference in order to avoid multiple // cookie jar share the same cookie pointers - if (!AddCookie(*cookiePtr)) - return false; + status_t result = AddCookie(*cookiePtr); + if (result != B_OK) + return result; } - return true; + return B_OK; } @@ -437,7 +452,7 @@ BNetworkCookieJar::Iterator::NextDomain() return NULL; } - fList = *(fIterator->fCookieMapIterator.NextValue()); + fList = *fIterator->fCookieMapIterator.NextValue(); fIndex = 0; fElement = reinterpret_cast(fList->ItemAt(fIndex)); @@ -461,8 +476,8 @@ BNetworkCookieJar::Iterator::Remove() else fLastList->RemoveItem(fLastList->CountItems() - 1); } else { - fList->RemoveItem(fIndex-1); fIndex--; + fList->RemoveItem(fIndex); } fLastElement = NULL; @@ -474,12 +489,18 @@ BNetworkCookieJar::Iterator& BNetworkCookieJar::Iterator::operator=(const BNetworkCookieJar::Iterator& other) { fCookieJar = other.fCookieJar; - fIterator = other.fIterator; 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; } @@ -527,25 +548,22 @@ BNetworkCookieJar::UrlIterator::UrlIterator(const BNetworkCookieJar* cookieJar, fLastElement(NULL), fIndex(0), fLastIndex(0), - fUrl(const_cast(url)) + fUrl(url) { - BString domain(url.Host()); + BString domain = url.Host(); if (!domain.Length()) return; - if (domain[0] != '.') - domain.Prepend("."); - - // Prepending another dot since _FindNext is going to - // call _SupDomain() - domain.Prepend("."); - fIterator = new(std::nothrow) PrivateIterator( fCookieJar->fCookieHashMap->fHashMap.GetIterator()); - fIterator->fKey.SetTo(domain, domain.Length()); - _FindNext(); + if (fIterator != NULL) { + // Prepending a dot since _FindNext is going to call _SupDomain() + domain.Prepend("."); + fIterator->fKey.SetTo(domain, domain.Length()); + _FindNext(); + } } @@ -607,23 +625,28 @@ BNetworkCookieJar::UrlIterator::operator=( fLastElement = other.fLastElement; fIndex = other.fIndex; fLastIndex = other.fLastIndex; - fUrl = other.fUrl; - fIterator = other.fIterator; + + fIterator = new(std::nothrow) PrivateIterator(*other.fIterator); + if (fIterator == NULL) { + // Make the iterator unusable. + fElement = NULL; + fLastElement = NULL; + } + return *this; } bool -BNetworkCookieJar::UrlIterator::_SupDomain() +BNetworkCookieJar::UrlIterator::_SuperDomain() { - BString domain(fIterator->fKey.GetString()); - int32 nextDot = domain.FindFirst('.', 1); + const char* domain = fIterator->fKey.GetString(); + const char* nextDot = strchr(domain, '.'); - if (nextDot == -1) + if (nextDot == NULL) return false; - domain.Remove(0, nextDot); - fIterator->fKey.SetTo(domain.String(), domain.Length()); + fIterator->fKey.SetTo(nextDot + 1); return true; } @@ -633,19 +656,16 @@ BNetworkCookieJar::UrlIterator::_FindNext() { fLastIndex = fIndex; fLastElement = fElement; - - if (_FindPath()) - return; - fLastList = fList; - do { - if (!_SupDomain()) { + + while (!_FindPath()) { + if (!_SuperDomain()) { fElement = NULL; return; } _FindDomain(); - } while (!_FindPath()); + } } @@ -665,16 +685,13 @@ bool BNetworkCookieJar::UrlIterator::_FindPath() { fIndex++; - if (fList && fIndex < fList->CountItems()) { - do { - fElement - = reinterpret_cast(fList->ItemAt(fIndex)); + while (fList && fIndex < fList->CountItems()) { + fElement = reinterpret_cast(fList->ItemAt(fIndex)); - if (fElement->IsValidForPath(fUrl.Path())) - return true; + if (fElement->IsValidForPath(fUrl.Path())) + return true; - fIndex++; - } while (fList && fIndex < fList->CountItems()); + fIndex++; } return false; diff --git a/src/tests/kits/net/cookie/cookie_test.cpp b/src/tests/kits/net/cookie/cookie_test.cpp index ded674ce34..8a4417eb09 100644 --- a/src/tests/kits/net/cookie/cookie_test.cpp +++ b/src/tests/kits/net/cookie/cookie_test.cpp @@ -20,58 +20,81 @@ using std::cout; using std::endl; -typedef struct -{ +typedef struct { const char* cookieString; - + const char* url; struct { + bool valid; const char* name; const char* value; const char* domain; const char* path; bool secure; - bool discard; + bool httponly; bool session; - int32 maxAge; + BDateTime expire; } expected; } ExplodeTest; -const ExplodeTest kTestExplode[] = - // Cookie string - // Name Value Domain Path Secure Discard Session maxAge - // -------- --------- --------- --------- ------ ------- -------- ------- + +ExplodeTest kTestExplode[] = + // Cookie string URL + // ------------- ------------- + // Valid Name Value Domain Path Secure HttpOnly Session Expiration + // --------- -------- --------- ----------------- --------- -------- -------- ------- ---------- { - { "name=value", - { "name", "value", "", "", false, false, true, 0 } }, - { "name=value;secure=true", - { "name", "value", "", "", true, false, true, 0 } }, - { "name=value;secure=false;maxage=5", - { "name", "value", "", "", false, false, false, 5 } }, - { "name=value;discard=true", - { "name", "value", "", "", false, true, true, 0 } }, + // Normal cookies + { "name=value", "http://www.example.com/path/path", + { true, "name", "value", "www.example.com", "/path", false, false, true, BDateTime() } }, + { "name=value; domain=example.com; path=/; secure", "http://www.example.com/path/path", + { true, "name", "value", "example.com", "/" , true, false, true, BDateTime() } }, + { "name=value; httponly; secure", "http://www.example.com/path/path", + { true, "name", "value", "www.example.com", "/path", true, true, true, BDateTime() } }, + { "name=value; expires=Wed, 20 Feb 2013 20:00:00 UTC", "http://www.example.com/path/path", + { true, "name", "value", "www.example.com", "/path", false, false, false, + BDateTime(BDate(2012, 2, 20), BTime(20, 0, 0, 0)) } }, + // Valid cookie with bad form + { "name= ; domain =example.com ;path=/; secure = yup ; blahblah ;)", "http://www.example.com/path/path", + { true, "name", "", "example.com", "/" , true, false, true, BDateTime() } }, + // Invalid path, default path should be used instead + { "name=value; path=invalid", "http://www.example.com/path/path", + { true, "name", "value", "www.example.com", "/path", false, false, true, BDateTime() } }, + // Setting for other subdomain (invalid) + { "name=value; domain=subdomain.example.com", "http://www.example.com/path/path", + { false, "name", "value", "www.example.com", "/path", false, false, true, BDateTime() } }, + // Various invalid cookies + { "name", "http://www.example.com/path/path", + { false, "name", "value", "www.example.com", "/path", false, false, true, BDateTime() } }, + { "; domain=example.com", "http://www.example.com/path/path", + { false, "name", "value", "www.example.com", "/path", false, false, true, BDateTime() } } }; void explodeImplodeTest() { - uint8 testIndex; + uint32 testIndex; BNetworkCookie cookie; for (testIndex = 0; testIndex < (sizeof(kTestExplode) / sizeof(ExplodeTest)); testIndex++) { - cookie.ParseCookieString(kTestExplode[testIndex].cookieString); + BUrl url(kTestExplode[testIndex].url); + cookie.ParseCookieStringFromUrl(kTestExplode[testIndex].cookieString, url); - ASSERT(testIndex, BString(kTestExplode[testIndex].expected.name) == BString(cookie.Name())); - ASSERT(testIndex, BString(kTestExplode[testIndex].expected.value) == BString(cookie.Value())); - ASSERT(testIndex, BString(kTestExplode[testIndex].expected.domain) == BString(cookie.Domain())); - ASSERT(testIndex, BString(kTestExplode[testIndex].expected.path) == BString(cookie.Path())); - ASSERT(testIndex, kTestExplode[testIndex].expected.secure == cookie.Secure()); - ASSERT(testIndex, kTestExplode[testIndex].expected.discard == cookie.Discard()); - ASSERT(testIndex, kTestExplode[testIndex].expected.session == cookie.IsSessionCookie()); + ASSERT(testIndex, kTestExplode[testIndex].expected.valid == cookie.IsValid()); - if (!cookie.IsSessionCookie()) - ASSERT(testIndex, kTestExplode[testIndex].expected.maxAge == cookie.MaxAge()); + if (kTestExplode[testIndex].expected.valid) { + ASSERT(testIndex, BString(kTestExplode[testIndex].expected.name) == cookie.Name()); + ASSERT(testIndex, BString(kTestExplode[testIndex].expected.value) == cookie.Value()); + ASSERT(testIndex, BString(kTestExplode[testIndex].expected.domain) == cookie.Domain()); + ASSERT(testIndex, BString(kTestExplode[testIndex].expected.path) == cookie.Path()); + ASSERT(testIndex, kTestExplode[testIndex].expected.secure == cookie.Secure()); + ASSERT(testIndex, kTestExplode[testIndex].expected.httponly == cookie.HttpOnly()); + ASSERT(testIndex, kTestExplode[testIndex].expected.session == cookie.IsSessionCookie()); + + if (!cookie.IsSessionCookie()) + ASSERT(testIndex, kTestExplode[testIndex].expected.expire.Time_t() == cookie.ExpirationDate()); + } } } @@ -79,27 +102,27 @@ void explodeImplodeTest() void stressTest(int32 domainNumber, int32 totalCookies, char** flat, ssize_t* size) { char **domains = new char*[domainNumber]; - + cout << "Creating random domains" << endl; srand(time(NULL)); for (int32 i = 0; i < domainNumber; i++) { int16 charNum = (rand() % 16) + 1; - + domains[i] = new char[charNum + 5]; - + // Random domain - for (int32 c = 0; c < charNum; c++) + for (int32 c = 0; c < charNum; c++) domains[i][c] = (rand() % 26) + 'a'; - + domains[i][charNum] = '.'; - + // Random tld - for (int32 c = 0; c < 3; c++) + for (int32 c = 0; c < 3; c++) domains[i][charNum+1+c] = (rand() % 26) + 'a'; - + domains[i][charNum+4] = 0; } - + BNetworkCookieJar j; BStopWatch* watch = new BStopWatch("Cookie insertion"); for (int32 i = 0; i < totalCookies; i++) { @@ -107,21 +130,21 @@ void stressTest(int32 domainNumber, int32 totalCookies, char** flat, ssize_t* si int16 domain = (rand() % domainNumber); BString name("Foo"); name << i; - + c.SetName(name); c.SetValue("Bar"); c.SetDomain(domains[domain]); c.SetPath("/"); - + j.AddCookie(c); } delete watch; - + BNetworkCookie* c; int16 domain = (rand() % domainNumber); BString host("http://"); host << domains[domain] << "/"; - + watch = new BStopWatch("Cookie filtering"); BUrl url(host); int32 count = 0; @@ -131,17 +154,17 @@ void stressTest(int32 domainNumber, int32 totalCookies, char** flat, ssize_t* si } delete watch; cout << "Count for " << host << ": " << count << endl; - - + + cout << "Flat view of cookie jar is " << j.FlattenedSize() << " bytes large." << endl; *flat = new char[j.FlattenedSize()]; *size = j.FlattenedSize(); - + if (j.Flatten(*flat, j.FlattenedSize()) == B_OK) cout << "Flatten() success!" << endl; else cout << "Flatten() error!" << endl; - + delete[] domains; } @@ -152,22 +175,22 @@ main(int, char**) cout << "Running explodeImplodeTest:" << endl; explodeImplodeTest(); cout << endl << endl; - + cout << "Running stressTest:" << endl; char* flatJar; ssize_t size; stressTest(10000, 40000, &flatJar, &size); - + BNetworkCookieJar j; j.Unflatten(B_ANY_TYPE, flatJar, size); - + int32 count = 0; BNetworkCookie* c; for (BNetworkCookieJar::Iterator it(j.GetIterator()); (c = it.Next()); ) count++; cout << "Count : " << count << endl; - + delete[] flatJar; - + return EXIT_SUCCESS; } From 163a39483387c5db4156dfcf36e4b48156df30a6 Mon Sep 17 00:00:00 2001 From: Humdinger Date: Sun, 10 Feb 2013 07:28:38 +0100 Subject: [PATCH 7/9] Revert "Renamed Tracker option "Don't move files to Trash" (#9352)" This reverts commit 45f77dcd7029914ccd99178dfb333eb68c7af9e5. --- src/kits/tracker/Commands.h | 2 +- src/kits/tracker/ContainerWindow.cpp | 16 ++++++------ src/kits/tracker/FSUtils.cpp | 4 +-- src/kits/tracker/FilePanelPriv.cpp | 6 ++--- src/kits/tracker/PoseView.cpp | 4 +-- src/kits/tracker/PoseViewScripting.cpp | 2 +- src/kits/tracker/SettingsViews.cpp | 36 +++++++++++++------------- src/kits/tracker/SettingsViews.h | 4 +-- src/kits/tracker/TrackerSettings.cpp | 14 +++++----- src/kits/tracker/TrackerSettings.h | 4 +-- 10 files changed, 46 insertions(+), 46 deletions(-) diff --git a/src/kits/tracker/Commands.h b/src/kits/tracker/Commands.h index a98d6147e7..869270be48 100644 --- a/src/kits/tracker/Commands.h +++ b/src/kits/tracker/Commands.h @@ -140,7 +140,7 @@ const uint32 kUpdateVolumeSpaceBar = 'UpSB'; const uint32 kShowVolumeSpaceBar = 'ShSB'; const uint32 kSpaceBarColorChanged = 'SBcc'; -const uint32 kMoveFilesToTrashChanged = 'STdm'; +const uint32 kDontMoveFilesToTrashChanged = 'STdm'; const uint32 kAskBeforeDeleteFileChanged = 'STad'; } // namespace BPrivate diff --git a/src/kits/tracker/ContainerWindow.cpp b/src/kits/tracker/ContainerWindow.cpp index bf3a7e58e1..35f12a9be0 100644 --- a/src/kits/tracker/ContainerWindow.cpp +++ b/src/kits/tracker/ContainerWindow.cpp @@ -604,7 +604,7 @@ BContainerWindow::BContainerWindow(LockingList* list, app->StartWatching(this, kWindowsShowFullPathChanged); app->StartWatching(this, kSingleWindowBrowseChanged); app->StartWatching(this, kShowNavigatorChanged); - app->StartWatching(this, kMoveFilesToTrashChanged); + app->StartWatching(this, kDontMoveFilesToTrashChanged); app->Unlock(); } @@ -625,7 +625,7 @@ BContainerWindow::~BContainerWindow() app->StopWatching(this, kWindowsShowFullPathChanged); app->StopWatching(this, kSingleWindowBrowseChanged); app->StopWatching(this, kShowNavigatorChanged); - app->StopWatching(this, kMoveFilesToTrashChanged); + app->StopWatching(this, kDontMoveFilesToTrashChanged); app->Unlock(); } @@ -1687,10 +1687,10 @@ BContainerWindow::MessageReceived(BMessage* message) settings.SingleWindowBrowse()); break; - case kMoveFilesToTrashChanged: + case kDontMoveFilesToTrashChanged: { bool dontMoveToTrash - = settings.MoveFilesToTrash(); + = settings.DontMoveFilesToTrash(); BMenuItem* item = fFileContextMenu->FindItem(kMoveToTrash); @@ -1934,8 +1934,8 @@ BContainerWindow::AddFileMenu(BMenu* menu) menu->AddItem(new BMenuItem(B_TRANSLATE("Duplicate"), new BMessage(kDuplicateSelection), 'D')); - menu->AddItem(new BMenuItem(TrackerSettings().MoveFilesToTrash() - ? B_TRANSLATE("Move to Trash") : B_TRANSLATE("Delete"), + menu->AddItem(new BMenuItem(TrackerSettings().DontMoveFilesToTrash() + ? B_TRANSLATE("Delete") : B_TRANSLATE("Move to Trash"), new BMessage(kMoveToTrash), 'T')); menu->AddSeparatorItem(); @@ -2758,8 +2758,8 @@ BContainerWindow::AddFileContextMenus(BMenu* menu) } if (!IsTrash() && !InTrash()) { - menu->AddItem(new BMenuItem(TrackerSettings().MoveFilesToTrash() - ? B_TRANSLATE("Move to Trash") : B_TRANSLATE("Delete"), + menu->AddItem(new BMenuItem(TrackerSettings().DontMoveFilesToTrash() + ? B_TRANSLATE("Delete") : B_TRANSLATE("Move to Trash"), new BMessage(kMoveToTrash), 'T')); // add separator for copy to/move to items (navigation items) diff --git a/src/kits/tracker/FSUtils.cpp b/src/kits/tracker/FSUtils.cpp index f001b29e0b..8a75bbc1e8 100644 --- a/src/kits/tracker/FSUtils.cpp +++ b/src/kits/tracker/FSUtils.cpp @@ -2842,9 +2842,9 @@ status_t _DeleteTask(BObjectList* list, bool confirm) { if (confirm) { - bool MoveToTrash = TrackerSettings().MoveFilesToTrash(); + bool dontMoveToTrash = TrackerSettings().DontMoveFilesToTrash(); - if (MoveToTrash) { + if (!dontMoveToTrash) { BAlert* alert = new BAlert("", B_TRANSLATE_NOCOLLECT(kDeleteConfirmationStr), B_TRANSLATE("Cancel"), B_TRANSLATE("Move to Trash"), diff --git a/src/kits/tracker/FilePanelPriv.cpp b/src/kits/tracker/FilePanelPriv.cpp index 64cb1326ed..70d8cc3432 100644 --- a/src/kits/tracker/FilePanelPriv.cpp +++ b/src/kits/tracker/FilePanelPriv.cpp @@ -885,9 +885,9 @@ TFilePanel::AddFileContextMenus(BMenu* menu) new BMessage(kGetInfo), 'I')); menu->AddItem(new BMenuItem(B_TRANSLATE("Edit name"), new BMessage(kEditItem), 'E')); - menu->AddItem(new BMenuItem(TrackerSettings().MoveFilesToTrash() - ? B_TRANSLATE("Move to Trash") - : B_TRANSLATE("Delete"), + menu->AddItem(new BMenuItem(TrackerSettings().DontMoveFilesToTrash() + ? B_TRANSLATE("Delete") + : B_TRANSLATE("Move to Trash"), new BMessage(kMoveToTrash), 'T')); menu->AddSeparatorItem(); menu->AddItem(new BMenuItem(B_TRANSLATE("Cut"), diff --git a/src/kits/tracker/PoseView.cpp b/src/kits/tracker/PoseView.cpp index 87ea8b3bf4..cda2c0ee9a 100644 --- a/src/kits/tracker/PoseView.cpp +++ b/src/kits/tracker/PoseView.cpp @@ -2323,7 +2323,7 @@ BPoseView::MessageReceived(BMessage* message) { TrackerSettings settings; - if ((modifiers() & B_SHIFT_KEY) != 0 || !settings.MoveFilesToTrash()) + if ((modifiers() & B_SHIFT_KEY) != 0 || settings.DontMoveFilesToTrash()) DeleteSelection(true, settings.AskBeforeDeleteFile()); else MoveSelectionToTrash(); @@ -6461,7 +6461,7 @@ BPoseView::KeyDown(const char* bytes, int32 count) } else { TrackerSettings settings; - if ((modifiers() & B_SHIFT_KEY) != 0 || !settings.MoveFilesToTrash()) + if ((modifiers() & B_SHIFT_KEY) != 0 || settings.DontMoveFilesToTrash()) DeleteSelection(true, settings.AskBeforeDeleteFile()); else MoveSelectionToTrash(); diff --git a/src/kits/tracker/PoseViewScripting.cpp b/src/kits/tracker/PoseViewScripting.cpp index 94089a3e48..cb9d299acb 100644 --- a/src/kits/tracker/PoseViewScripting.cpp +++ b/src/kits/tracker/PoseViewScripting.cpp @@ -491,7 +491,7 @@ BPoseView::DeleteProperty(BMessage* _SCRIPTING_ONLY(specifier), if (error == B_OK) { TrackerSettings settings; - if (settings.MoveFilesToTrash()) { + if (!settings.DontMoveFilesToTrash()) { // move the list we build into trash, don't make the // trashing task select the next item MoveListToTrash(entryList, false, false); diff --git a/src/kits/tracker/SettingsViews.cpp b/src/kits/tracker/SettingsViews.cpp index d44d767792..5436978f49 100644 --- a/src/kits/tracker/SettingsViews.cpp +++ b/src/kits/tracker/SettingsViews.cpp @@ -956,12 +956,12 @@ TrashSettingsView::TrashSettingsView() : SettingsView("TrashSettingsView") { - fMoveFilesToTrashCheckBox = new BCheckBox("", - B_TRANSLATE("Move deleted files to Trash first"), - new BMessage(kMoveFilesToTrashChanged)); + fDontMoveFilesToTrashCheckBox = new BCheckBox("", + B_TRANSLATE("Don't move files to Trash"), + new BMessage(kDontMoveFilesToTrashChanged)); fAskBeforeDeleteFileCheckBox = new BCheckBox("", - B_TRANSLATE("Ask before deleting for good"), + B_TRANSLATE("Ask before delete"), new BMessage(kAskBeforeDeleteFileChanged)); const float spacing = be_control_look->DefaultItemSpacing(); @@ -970,7 +970,7 @@ TrashSettingsView::TrashSettingsView() layout->SetOrientation(B_VERTICAL); layout->SetSpacing(0); BGroupLayoutBuilder(layout) - .Add(fMoveFilesToTrashCheckBox) + .Add(fDontMoveFilesToTrashCheckBox) .Add(fAskBeforeDeleteFileCheckBox) .AddGlue() .SetInsets(spacing, spacing, spacing, spacing); @@ -981,7 +981,7 @@ TrashSettingsView::TrashSettingsView() void TrashSettingsView::AttachedToWindow() { - fMoveFilesToTrashCheckBox->SetTarget(this); + fDontMoveFilesToTrashCheckBox->SetTarget(this); fAskBeforeDeleteFileCheckBox->SetTarget(this); } @@ -995,11 +995,11 @@ TrashSettingsView::MessageReceived(BMessage* message) TrackerSettings settings; switch (message->what) { - case kMoveFilesToTrashChanged: - settings.SetMoveFilesToTrash( - fMoveFilesToTrashCheckBox->Value() == 1); + case kDontMoveFilesToTrashChanged: + settings.SetDontMoveFilesToTrash( + fDontMoveFilesToTrashCheckBox->Value() == 1); - tracker->SendNotices(kMoveFilesToTrashChanged); + tracker->SendNotices(kDontMoveFilesToTrashChanged); Window()->PostMessage(kSettingsContentsModified); break; @@ -1023,7 +1023,7 @@ TrashSettingsView::SetDefaults() { TrackerSettings settings; - settings.SetMoveFilesToTrash(true); + settings.SetDontMoveFilesToTrash(false); settings.SetAskBeforeDeleteFile(true); ShowCurrentSettings(); @@ -1036,7 +1036,7 @@ TrashSettingsView::IsDefaultable() const { TrackerSettings settings; - return settings.MoveFilesToTrash() != true + return settings.DontMoveFilesToTrash() != false || settings.AskBeforeDeleteFile() != true; } @@ -1046,7 +1046,7 @@ TrashSettingsView::Revert() { TrackerSettings settings; - settings.SetMoveFilesToTrash(fMoveFilesToTrash); + settings.SetDontMoveFilesToTrash(fDontMoveFilesToTrash); settings.SetAskBeforeDeleteFile(fAskBeforeDeleteFile); ShowCurrentSettings(); @@ -1061,7 +1061,7 @@ TrashSettingsView::_SendNotices() if (!tracker) return; - tracker->SendNotices(kMoveFilesToTrashChanged); + tracker->SendNotices(kDontMoveFilesToTrashChanged); tracker->SendNotices(kAskBeforeDeleteFileChanged); } @@ -1071,7 +1071,7 @@ TrashSettingsView::ShowCurrentSettings() { TrackerSettings settings; - fMoveFilesToTrashCheckBox->SetValue(settings.MoveFilesToTrash()); + fDontMoveFilesToTrashCheckBox->SetValue(settings.DontMoveFilesToTrash()); fAskBeforeDeleteFileCheckBox->SetValue(settings.AskBeforeDeleteFile()); } @@ -1081,7 +1081,7 @@ TrashSettingsView::RecordRevertSettings() { TrackerSettings settings; - fMoveFilesToTrash = settings.MoveFilesToTrash(); + fDontMoveFilesToTrash = settings.DontMoveFilesToTrash(); fAskBeforeDeleteFile = settings.AskBeforeDeleteFile(); } @@ -1089,8 +1089,8 @@ TrashSettingsView::RecordRevertSettings() bool TrashSettingsView::IsRevertable() const { - return fMoveFilesToTrash - != (fMoveFilesToTrashCheckBox->Value() > 0) + return fDontMoveFilesToTrash + != (fDontMoveFilesToTrashCheckBox->Value() > 0) || fAskBeforeDeleteFile != (fAskBeforeDeleteFileCheckBox->Value() > 0); } diff --git a/src/kits/tracker/SettingsViews.h b/src/kits/tracker/SettingsViews.h index f6c21beb44..bc3474bb26 100644 --- a/src/kits/tracker/SettingsViews.h +++ b/src/kits/tracker/SettingsViews.h @@ -181,10 +181,10 @@ class TrashSettingsView : public SettingsView { private: void _SendNotices(); - BCheckBox* fMoveFilesToTrashCheckBox; + BCheckBox* fDontMoveFilesToTrashCheckBox; BCheckBox* fAskBeforeDeleteFileCheckBox; - bool fMoveFilesToTrash; + bool fDontMoveFilesToTrash; bool fAskBeforeDeleteFile; typedef SettingsView _inherited; diff --git a/src/kits/tracker/TrackerSettings.cpp b/src/kits/tracker/TrackerSettings.cpp index 5ee3ed202c..7eebfa4e21 100644 --- a/src/kits/tracker/TrackerSettings.cpp +++ b/src/kits/tracker/TrackerSettings.cpp @@ -82,7 +82,7 @@ class TTrackerState : public Settings { HexScalarValueSetting* fFreeSpaceColor; HexScalarValueSetting* fWarningSpaceColor; - BooleanValueSetting* fMoveFilesToTrash; + BooleanValueSetting* fDontMoveFilesToTrash; BooleanValueSetting* fAskBeforeDeleteFile; Benaphore fInitLock; @@ -202,8 +202,8 @@ TTrackerState::LoadSettingsIfNeeded() Add(fWarningSpaceColor = new HexScalarValueSetting("WarningSpaceColor", 0xc0cb0000, "", "")); - Add(fMoveFilesToTrash - = new BooleanValueSetting("MoveFilesToTrash", true)); + Add(fDontMoveFilesToTrash + = new BooleanValueSetting("DontMoveFilesToTrash", false)); Add(fAskBeforeDeleteFile = new BooleanValueSetting("AskBeforeDeleteFile", true)); @@ -509,16 +509,16 @@ TrackerSettings::SetRecentFoldersCount(int32 count) bool -TrackerSettings::MoveFilesToTrash() +TrackerSettings::DontMoveFilesToTrash() { - return gTrackerState.fMoveFilesToTrash->Value(); + return gTrackerState.fDontMoveFilesToTrash->Value(); } void -TrackerSettings::SetMoveFilesToTrash(bool enabled) +TrackerSettings::SetDontMoveFilesToTrash(bool enabled) { - gTrackerState.fMoveFilesToTrash->SetValue(enabled); + gTrackerState.fDontMoveFilesToTrash->SetValue(enabled); } diff --git a/src/kits/tracker/TrackerSettings.h b/src/kits/tracker/TrackerSettings.h index da762ebb7b..78f2f335e5 100644 --- a/src/kits/tracker/TrackerSettings.h +++ b/src/kits/tracker/TrackerSettings.h @@ -118,8 +118,8 @@ class TrackerSettings { bool ClockIs24Hr(); void SetClockTo24Hr(bool); - bool MoveFilesToTrash(); - void SetMoveFilesToTrash(bool); + bool DontMoveFilesToTrash(); + void SetDontMoveFilesToTrash(bool); bool AskBeforeDeleteFile(); void SetAskBeforeDeleteFile(bool); From 1baa2211739cd5b4d45a56868aedd57dedbe782c Mon Sep 17 00:00:00 2001 From: Humdinger Date: Sun, 10 Feb 2013 11:02:38 +0100 Subject: [PATCH 8/9] Removed Trash options from Tracker prefs. As discussed [1][2], using a Trash is in the interest of the vast majority of users. The setting is still present in the Tracker settings file. SHIFT+DELETE will still bypass the Trash. [1] http://www.freelists.org/post/haiku-commits/haiku-hrev45134-srckitstracker,4 [2] http://www.freelists.org/post/haiku/Removing-Trackers-Trash-options Please enter the commit message for your changes. Lines starting --- src/kits/tracker/SettingsViews.cpp | 148 --------------------- src/kits/tracker/SettingsViews.h | 27 ---- src/kits/tracker/TrackerSettingsWindow.cpp | 2 - 3 files changed, 177 deletions(-) diff --git a/src/kits/tracker/SettingsViews.cpp b/src/kits/tracker/SettingsViews.cpp index 5436978f49..3e645d433e 100644 --- a/src/kits/tracker/SettingsViews.cpp +++ b/src/kits/tracker/SettingsViews.cpp @@ -947,151 +947,3 @@ SpaceBarSettingsView::IsRevertable() const || fFreeSpaceColor != settings.FreeSpaceColor() || fWarningSpaceColor != settings.WarningSpaceColor(); } - - -// #pragma mark - - - -TrashSettingsView::TrashSettingsView() - : - SettingsView("TrashSettingsView") -{ - fDontMoveFilesToTrashCheckBox = new BCheckBox("", - B_TRANSLATE("Don't move files to Trash"), - new BMessage(kDontMoveFilesToTrashChanged)); - - fAskBeforeDeleteFileCheckBox = new BCheckBox("", - B_TRANSLATE("Ask before delete"), - new BMessage(kAskBeforeDeleteFileChanged)); - - const float spacing = be_control_look->DefaultItemSpacing(); - - BGroupLayout* layout = GroupLayout(); - layout->SetOrientation(B_VERTICAL); - layout->SetSpacing(0); - BGroupLayoutBuilder(layout) - .Add(fDontMoveFilesToTrashCheckBox) - .Add(fAskBeforeDeleteFileCheckBox) - .AddGlue() - .SetInsets(spacing, spacing, spacing, spacing); - -} - - -void -TrashSettingsView::AttachedToWindow() -{ - fDontMoveFilesToTrashCheckBox->SetTarget(this); - fAskBeforeDeleteFileCheckBox->SetTarget(this); -} - - -void -TrashSettingsView::MessageReceived(BMessage* message) -{ - TTracker* tracker = dynamic_cast(be_app); - if (!tracker) - return; - TrackerSettings settings; - - switch (message->what) { - case kDontMoveFilesToTrashChanged: - settings.SetDontMoveFilesToTrash( - fDontMoveFilesToTrashCheckBox->Value() == 1); - - tracker->SendNotices(kDontMoveFilesToTrashChanged); - Window()->PostMessage(kSettingsContentsModified); - break; - - case kAskBeforeDeleteFileChanged: - settings.SetAskBeforeDeleteFile( - fAskBeforeDeleteFileCheckBox->Value() == 1); - - tracker->SendNotices(kAskBeforeDeleteFileChanged); - Window()->PostMessage(kSettingsContentsModified); - break; - - default: - _inherited::MessageReceived(message); - break; - } -} - - -void -TrashSettingsView::SetDefaults() -{ - TrackerSettings settings; - - settings.SetDontMoveFilesToTrash(false); - settings.SetAskBeforeDeleteFile(true); - - ShowCurrentSettings(); - _SendNotices(); -} - - -bool -TrashSettingsView::IsDefaultable() const -{ - TrackerSettings settings; - - return settings.DontMoveFilesToTrash() != false - || settings.AskBeforeDeleteFile() != true; -} - - -void -TrashSettingsView::Revert() -{ - TrackerSettings settings; - - settings.SetDontMoveFilesToTrash(fDontMoveFilesToTrash); - settings.SetAskBeforeDeleteFile(fAskBeforeDeleteFile); - - ShowCurrentSettings(); - _SendNotices(); -} - - -void -TrashSettingsView::_SendNotices() -{ - TTracker* tracker = dynamic_cast(be_app); - if (!tracker) - return; - - tracker->SendNotices(kDontMoveFilesToTrashChanged); - tracker->SendNotices(kAskBeforeDeleteFileChanged); -} - - -void -TrashSettingsView::ShowCurrentSettings() -{ - TrackerSettings settings; - - fDontMoveFilesToTrashCheckBox->SetValue(settings.DontMoveFilesToTrash()); - fAskBeforeDeleteFileCheckBox->SetValue(settings.AskBeforeDeleteFile()); -} - - -void -TrashSettingsView::RecordRevertSettings() -{ - TrackerSettings settings; - - fDontMoveFilesToTrash = settings.DontMoveFilesToTrash(); - fAskBeforeDeleteFile = settings.AskBeforeDeleteFile(); -} - - -bool -TrashSettingsView::IsRevertable() const -{ - return fDontMoveFilesToTrash - != (fDontMoveFilesToTrashCheckBox->Value() > 0) - || fAskBeforeDeleteFile - != (fAskBeforeDeleteFileCheckBox->Value() > 0); -} - diff --git a/src/kits/tracker/SettingsViews.h b/src/kits/tracker/SettingsViews.h index bc3474bb26..68ce03484f 100644 --- a/src/kits/tracker/SettingsViews.h +++ b/src/kits/tracker/SettingsViews.h @@ -163,33 +163,6 @@ class SpaceBarSettingsView : public SettingsView { typedef SettingsView _inherited; }; - -class TrashSettingsView : public SettingsView { - public: - TrashSettingsView(); - - virtual void MessageReceived(BMessage* message); - virtual void AttachedToWindow(); - - virtual void SetDefaults(); - virtual bool IsDefaultable() const; - virtual void Revert(); - virtual void ShowCurrentSettings(); - virtual void RecordRevertSettings(); - virtual bool IsRevertable() const; - - private: - void _SendNotices(); - - BCheckBox* fDontMoveFilesToTrashCheckBox; - BCheckBox* fAskBeforeDeleteFileCheckBox; - - bool fDontMoveFilesToTrash; - bool fAskBeforeDeleteFile; - - typedef SettingsView _inherited; -}; - } // namespace BPrivate using namespace BPrivate; diff --git a/src/kits/tracker/TrackerSettingsWindow.cpp b/src/kits/tracker/TrackerSettingsWindow.cpp index 66267a60d3..754ae65bc3 100644 --- a/src/kits/tracker/TrackerSettingsWindow.cpp +++ b/src/kits/tracker/TrackerSettingsWindow.cpp @@ -114,8 +114,6 @@ TrackerSettingsWindow::TrackerSettingsWindow() new DesktopSettingsView())); fSettingsTypeListView->AddItem(new SettingsItem(B_TRANSLATE("Windows"), new WindowsSettingsView())); - fSettingsTypeListView->AddItem(new SettingsItem(B_TRANSLATE("Trash"), - new TrashSettingsView())); fSettingsTypeListView->AddItem(new SettingsItem( B_TRANSLATE("Volume icons"), new SpaceBarSettingsView())); From 344de4ccd12eb0fe63d7d3c8aece5138fe1909bd Mon Sep 17 00:00:00 2001 From: Siarzhuk Zharski Date: Sun, 10 Feb 2013 13:19:27 +0100 Subject: [PATCH 9/9] Fix Terminal localization issues in Custom color scheme * Localized Label of the colors menu entry was used as the key during loading corresponding color value from the preferences file. It was obviously observed only on non-English locales; * Fixes #7209 #8256. --- src/apps/terminal/AppearPrefView.cpp | 42 +++++++++++++++------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/apps/terminal/AppearPrefView.cpp b/src/apps/terminal/AppearPrefView.cpp index 3c3d23bc78..fa58dae14e 100644 --- a/src/apps/terminal/AppearPrefView.cpp +++ b/src/apps/terminal/AppearPrefView.cpp @@ -73,12 +73,12 @@ AppearancePrefView::AppearancePrefView(const char* name, fTerminalMessenger(messenger) { const char* kColorTable[] = { - B_TRANSLATE("Text"), - B_TRANSLATE("Background"), - B_TRANSLATE("Cursor"), - B_TRANSLATE("Text under cursor"), - B_TRANSLATE("Selected text"), - B_TRANSLATE("Selected background"), + B_TRANSLATE_MARK("Text"), + B_TRANSLATE_MARK("Background"), + B_TRANSLATE_MARK("Cursor"), + B_TRANSLATE_MARK("Text under cursor"), + B_TRANSLATE_MARK("Selected text"), + B_TRANSLATE_MARK("Selected background"), NULL }; @@ -272,11 +272,14 @@ AppearancePrefView::MessageReceived(BMessage* msg) case MSG_COLOR_CHANGED: { - rgb_color oldColor = PrefHandler::Default()->getRGB( - fColorField->Menu()->FindMarked()->Label()); + const BMessage* itemMessage + = fColorField->Menu()->FindMarked()->Message(); + const char* label = NULL; + if (itemMessage->FindString("label", &label) != B_OK) + break; + rgb_color oldColor = PrefHandler::Default()->getRGB(label); if (oldColor != fColorControl->ValueAsColor()) { - PrefHandler::Default()->setRGB( - fColorField->Menu()->FindMarked()->Label(), + PrefHandler::Default()->setRGB(label, fColorControl->ValueAsColor()); modified = true; } @@ -300,9 +303,12 @@ AppearancePrefView::MessageReceived(BMessage* msg) } case MSG_COLOR_FIELD_CHANGED: - fColorControl->SetValue(PrefHandler::Default()->getRGB( - fColorField->Menu()->FindMarked()->Label())); + { + const char* label = NULL; + if (msg->FindString("label", &label) == B_OK) + fColorControl->SetValue(PrefHandler::Default()->getRGB(label)); break; + } case MSG_BLINK_CURSOR_CHANGED: if (PrefHandler::Default()->getBool(PREF_BLINK_CURSOR) @@ -514,23 +520,21 @@ AppearancePrefView::_MakeMenu(uint32 msg, const char** items, { BPopUpMenu* menu = new BPopUpMenu(""); - int32 i = 0; while (*items) { if (strcmp((*items), "") == 0) menu->AddSeparatorItem(); else { BMessage* message = new BMessage(msg); - menu->AddItem(new BMenuItem((*items), message)); + message->AddString("label", *items); + BMenuItem* item = new BMenuItem(B_TRANSLATE(*items), message); + menu->AddItem(item); + if (strcmp(*items, defaultItemName) == 0) + item->SetMarked(true); } items++; - i++; } - BMenuItem* defaultItem = menu->FindItem(defaultItemName); - if (defaultItem) - defaultItem->SetMarked(true); - return menu; }