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

2 files changed, 24 insertions, 14 deletions

diff --git a/indra/llplugin/llplugincookiestore.cpp b/indra/llplugin/llplugincookiestore.cpp
index 85b1e70d78..da770c5f2e 100644
--- a/indra/llplugin/llplugincookiestore.cpp
+++ b/indra/llplugin/llplugincookiestore.cpp
@@ -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()
diff --git a/indra/llplugin/tests/llplugincookiestore_test.cpp b/indra/llplugin/tests/llplugincookiestore_test.cpp
index 020d9c1977..c903464c64 100644
--- a/indra/llplugin/tests/llplugincookiestore_test.cpp
+++ b/indra/llplugin/tests/llplugincookiestore_test.cpp
@@ -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