livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix for EXT-6756: google apps auth doesn't work right with shared media cookies 

Added "HttpOnly" to the allowed field names in LLPluginCookieStore::Cookie::parse().  (This was the actual cause of the failure -- cookies with this field in them were silently failing to parse.)

Added some LL_WARNS logging on this sort of cookie parse failure, which will make similar problems much easier to track down in future.

Also added tags to most of the logging in llplugincookiestore.cpp to make it easier to selectively enable it when debugging.

Added a cookie with all allowable field names to the unit test.

Reviewed by Sam at http://codereview.lindenlab.com/1247014
master
Monroe Linden 2010-04-07 18:15:56 -07:00
parent 463c0778ea
commit 2ba90ca871
2 changed files with 24 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
indra/llplugin/llplugincookiestore.cpp
View File

@ -99,7 +99,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
std::string::size_type cookie_end = mCookie.size();
std::string::size_type field_start = 0;
lldebugs << "parsing cookie: " << mCookie << llendl;
LL_DEBUGS("CookieStoreParse") << "parsing cookie: " << mCookie << LL_ENDL;
while(field_start < cookie_end)
{
// Finding the start of the next field requires honoring special quoting rules
@ -167,10 +167,10 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
++value_end;
}
lldebugs
LL_DEBUGS("CookieStoreParse")
<< " field name: \"" << mCookie.substr(name_start, name_end - name_start)
<< "\", value: \"" << mCookie.substr(value_start, value_end - value_start) << "\""
<< llendl;
<< LL_ENDL;
// See whether this field is one we know
if(first_field)
@ -195,13 +195,13 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
#if 1
time_t date = curl_getdate(date_string.c_str(), NULL );
mDate.secondsSinceEpoch((F64)date);
lldebugs << " expire date parsed to: " << mDate.asRFC1123() << llendl;
LL_DEBUGS("CookieStoreParse") << " expire date parsed to: " << mDate.asRFC1123() << LL_ENDL;
#else
// This doesn't work (rfc1123-format dates cause it to fail)
if(!mDate.fromString(date_string))
{
// Date failed to parse.
llwarns << "failed to parse cookie's expire date: " << date << llendl;
LL_WARNS("CookieStoreParse") << "failed to parse cookie's expire date: " << date << LL_ENDL;
return false;
}
#endif
@ -232,9 +232,14 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
{
// We don't care about the value of this field (yet)
}
else if(matchName(name_start, name_end, "httponly"))
{
// We don't care about the value of this field (yet)
}
else
{
// An unknown field is a parse failure
LL_WARNS("CookieStoreParse") << "unexpected field name: " << mCookie.substr(name_start, name_end - name_start) << LL_ENDL;
return false;
}
@ -270,7 +275,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
mCookie += host;
mDomainEnd = mCookie.size();
lldebugs << "added domain (" << mDomainStart << " to " << mDomainEnd << "), new cookie is: " << mCookie << llendl;
LL_DEBUGS("CookieStoreParse") << "added domain (" << mDomainStart << " to " << mDomainEnd << "), new cookie is: " << mCookie << LL_ENDL;
}
// If the cookie doesn't have a path, add "/".
@ -288,7 +293,7 @@ bool LLPluginCookieStore::Cookie::parse(const std::string &host)
mCookie += "/";
mPathEnd = mCookie.size();
lldebugs << "added path (" << mPathStart << " to " << mPathEnd << "), new cookie is: " << mCookie << llendl;
LL_DEBUGS("CookieStoreParse") << "added path (" << mPathStart << " to " << mPathEnd << "), new cookie is: " << mCookie << LL_ENDL;
}
@ -566,7 +571,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
Cookie *cookie = Cookie::createFromString(s, cookie_start, cookie_end, host);
if(cookie)
{
lldebugs << "setting cookie: " << cookie->getCookie() << llendl;
LL_DEBUGS("CookieStoreUpdate") << "setting cookie: " << cookie->getCookie() << LL_ENDL;
// Create a key for this cookie
std::string key = cookie->getKey();
@ -579,7 +584,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
{
// If we're marking cookies as changed, we should keep it anyway since we'll need to send it out with deltas.
cookie->setDead(true);
lldebugs << " marking dead" << llendl;
LL_DEBUGS("CookieStoreUpdate") << " marking dead" << LL_ENDL;
}
else
{
@ -590,7 +595,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
delete cookie;
cookie = NULL;
lldebugs << " removing" << llendl;
LL_DEBUGS("CookieStoreUpdate") << " removing" << LL_ENDL;
}
}
@ -607,7 +612,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
delete cookie;
cookie = NULL;
lldebugs << " unchanged" << llendl;
LL_DEBUGS("CookieStoreUpdate") << " unchanged" << LL_ENDL;
}
else
{
@ -619,7 +624,7 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
if(mark_changed)
mHasChangedCookies = true;
lldebugs << " replacing" << llendl;
LL_DEBUGS("CookieStoreUpdate") << " replacing" << LL_ENDL;
}
}
else
@ -631,10 +636,15 @@ void LLPluginCookieStore::setOneCookie(const std::string &s, std::string::size_t
if(mark_changed)
mHasChangedCookies = true;
lldebugs << " adding" << llendl;
LL_DEBUGS("CookieStoreUpdate") << " adding" << LL_ENDL;
}
}
}
else
{
LL_WARNS("CookieStoreUpdate") << "failed to parse cookie: " << s.substr(cookie_start, cookie_end - cookie_start) << LL_ENDL;
}
}
void LLPluginCookieStore::clearCookies()

2
indra/llplugin/tests/llplugincookiestore_test.cpp
View File

@ -127,7 +127,7 @@ namespace tut
// Valid, distinct cookies:
std::string cookie01 = "cookieA=value; domain=example.com; path=/";
std::string cookie02 = "cookieB=value; domain=example.com; path=/"; // different name
std::string cookie02 = "cookieB=value; Domain=example.com; Path=/; Max-Age=10; Secure; Version=1; Comment=foo!; HTTPOnly"; // cookie with every supported field, in different cases.
std::string cookie03 = "cookieA=value; domain=foo.example.com; path=/"; // different domain
std::string cookie04 = "cookieA=value; domain=example.com; path=/bar/"; // different path
std::string cookie05 = "cookieC; domain=example.com; path=/"; // empty value