2010-01-29 Tatsuhiro Tsujikawa <[email protected]>

	Fixed the bug that cookie for domain a.b.c is not sent to the host
	a.b.c if no other cookies are stored.  Fixed the bug that
	Cookie::markOriginServerOnly() is not called when cookies are
	loaded from file/sqlite3 database.
	* src/Cookie.h
	* src/CookieStorage.cc
	* src/NsCookieParser.cc
	* src/Sqlite3MozCookieParser.cc
	* test/CookieStorageTest.cc
	* test/NsCookieParserTest.cc
	* test/Sqlite3MozCookieParserTest.cc
	* test/cookies.sqlite
	* test/nscookietest.txt

ChangeLog

@@ -1,3 +1,19 @@
+2010-01-29  Tatsuhiro Tsujikawa  <[email protected]>
+
+	Fixed the bug that cookie for domain a.b.c is not sent to the host
+	a.b.c if no other cookies are stored.  Fixed the bug that
+	Cookie::markOriginServerOnly() is not called when cookies are
+	loaded from file/sqlite3 database.
+	* src/Cookie.h
+	* src/CookieStorage.cc
+	* src/NsCookieParser.cc
+	* src/Sqlite3MozCookieParser.cc
+	* test/CookieStorageTest.cc
+	* test/NsCookieParserTest.cc
+	* test/Sqlite3MozCookieParserTest.cc
+	* test/cookies.sqlite
+	* test/nscookietest.txt
+
 2010-01-29  Tatsuhiro Tsujikawa  <[email protected]>
 
 	Removed RequestGroup::initSegmentMan(). Guaranteed that either

src/Cookie.h

@@ -57,6 +57,9 @@ private:
 public:
   /*
    * If expires = 0 is given, then the cookie becomes session cookie.
+   * domain is normalized using normalizeDomain() function and
+   * assigned to _domain.  If domain is not specified in cookie, call
+   * markOriginServerOnly() after construction.
    */
   Cookie(const std::string& name,
          const std::string& value,
@@ -66,8 +69,11 @@ public:
          bool secure);
 
   /*
-   * Creates session cookie. This is equivalent to
-   * Cookie(name, value, 0, path, domain, secure);
+   * Creates session cookie. This is equivalent to Cookie(name, value,
+   * 0, path, domain, secure); domain is normalized using
+   * normalizeDomain() function and assigned to _domain.  If domain is
+   * not specified in cookie, call markOriginServerOnly() after
+   * construction.
    */
   Cookie(const std::string& name,
          const std::string& value,

src/CookieStorage.cc

@@ -266,13 +266,9 @@ std::deque<Cookie> CookieStorage::criteriaFind(const std::string& requestHost,
     for(std::vector<std::string>::const_iterator di =
           domainComponents.begin()+1; di != domainComponents.end(); ++di) {
       domain = strconcat(A2STR::DOT_C, *di, domain);
-      const size_t prenum = res.size();
       searchCookieByDomainSuffix(domain, _domains.begin(), _domains.end(),
                                  std::back_inserter(res),
                                  normRequestHost, requestPath, date, secure);
-      if(prenum == res.size()) {
-        break;
-      } 
     }
   }
   std::vector<CookiePathDivider> divs;

src/NsCookieParser.cc

@@ -70,7 +70,9 @@ static Cookie parseNsCookie(const std::string& nsCookieStr)
            vs[2], // path
            vs[0], // domain
            vs[3] == C_TRUE ? true : false);
-
+  if(!util::startsWith(vs[0], A2STR::DOT_C)) {
+    c.markOriginServerOnly();
+  }
   return c;
 }
 

src/Sqlite3MozCookieParser.cc

@@ -65,12 +65,12 @@ static int cookieRowMapper(void* data, int rowIndex,
                            char** values, char** names)
 {
   try {
+    std::deque<Cookie>& cookies = *reinterpret_cast<std::deque<Cookie>*>(data);
     int64_t expireDate = util::parseLLInt(toString(values[3]));
     // TODO assuming time_t is int32_t...
     if(expireDate > INT32_MAX) {
       expireDate = INT32_MAX;
     }
-
     Cookie c(toString(values[4]), // name
              toString(values[5]), // value
              expireDate, // expires
@@ -78,9 +78,11 @@ static int cookieRowMapper(void* data, int rowIndex,
              toString(values[0]), // domain
              strcmp(toString(values[2]).c_str(), "1") == 0 ? true:false //secure
              );
-                      
+    if(!util::startsWith(values[0], A2STR::DOT_C)) {
+      c.markOriginServerOnly();
+    }
     if(c.good()) {
-      reinterpret_cast<std::deque<Cookie>*>(data)->push_back(c);
+      cookies.push_back(c);
     }
   } catch(RecoverableException& e) {
     //failed to parse expiry.

test/CookieStorageTest.cc

@@ -138,7 +138,7 @@ void CookieStorageTest::testCriteriaFind()
                false);
   Cookie charlie("charlie", "CHARLIE", "/", ".aria2.org", true);
   Cookie delta("delta", "DELTA", "/foo/bar", ".aria2.org", false);
-  Cookie echo("echo", "ECHO", "/", "www.aria2.org", false);
+  Cookie echo("echo", "ECHO", "/", "www.dl.aria2.org", false);
   Cookie foxtrot("foxtrot", "FOXTROT", "/", ".sf.net", false);
   Cookie golf("golf", "GOLF", "/", "192.168.1.1", false);
   Cookie hotel1("hotel", "HOTEL1", "/", "samename.x", false);
@@ -168,7 +168,7 @@ void CookieStorageTest::testCriteriaFind()
   CPPUNIT_ASSERT(st.store(juliet1));
   CPPUNIT_ASSERT(st.store(juliet2));
 
-  std::deque<Cookie> aria2Slash = st.criteriaFind("www.aria2.org", "/",
+  std::deque<Cookie> aria2Slash = st.criteriaFind("www.dl.aria2.org", "/",
                                                   0, false);
   CPPUNIT_ASSERT_EQUAL((size_t)2, aria2Slash.size());
   CPPUNIT_ASSERT(std::find(aria2Slash.begin(), aria2Slash.end(), alpha)
@@ -176,7 +176,7 @@ void CookieStorageTest::testCriteriaFind()
   CPPUNIT_ASSERT(std::find(aria2Slash.begin(), aria2Slash.end(), echo)
                  != aria2Slash.end());
 
-  std::deque<Cookie> aria2SlashFoo = st.criteriaFind("www.aria2.org", "/foo",
+  std::deque<Cookie> aria2SlashFoo = st.criteriaFind("www.dl.aria2.org", "/foo",
                                                      0, false);
   CPPUNIT_ASSERT_EQUAL((size_t)3, aria2SlashFoo.size());
   CPPUNIT_ASSERT_EQUAL(std::string("bravo"), aria2SlashFoo[0].getName());
@@ -185,7 +185,7 @@ void CookieStorageTest::testCriteriaFind()
   CPPUNIT_ASSERT(std::find(aria2SlashFoo.begin(), aria2SlashFoo.end(), echo)
                  != aria2SlashFoo.end());
 
-  std::deque<Cookie> aria2Expires = st.criteriaFind("www.aria2.org", "/foo",
+  std::deque<Cookie> aria2Expires = st.criteriaFind("www.dl.aria2.org", "/foo",
                                                     Time().getTime()+120,
                                                     false);
   CPPUNIT_ASSERT_EQUAL((size_t)2, aria2Expires.size());
@@ -217,6 +217,10 @@ void CookieStorageTest::testCriteriaFind()
                        defaultDomainCookies[0].getValue());
   CPPUNIT_ASSERT_EQUAL(std::string("INDIA2"),
                        defaultDomainCookies[1].getValue());
+  defaultDomainCookies =
+    st.criteriaFind("sub.default.domain", "/foo", 0, false);
+  CPPUNIT_ASSERT_EQUAL((size_t)1, defaultDomainCookies.size());
+
 
   // localhost.local case
   std::deque<Cookie> localDomainCookies =
@@ -240,27 +244,27 @@ void CookieStorageTest::testLoad()
   dumpCookie(cookies, st);
 
   Cookie c = cookies[0];
-  CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), c.getName());
-  CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
+  CPPUNIT_ASSERT_EQUAL(std::string("novalue"), c.getName());
+  CPPUNIT_ASSERT_EQUAL(std::string(""), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
-  CPPUNIT_ASSERT(c.isSecureCookie());
+  CPPUNIT_ASSERT_EQUAL(std::string(".example.org"), c.getDomain());
+  CPPUNIT_ASSERT(!c.isSecureCookie());
 
   c = cookies[1];
-  CPPUNIT_ASSERT_EQUAL(std::string("novalue"), c.getName());
-  CPPUNIT_ASSERT_EQUAL(std::string(""), c.getValue());
+  CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), c.getName());
+  CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
-  CPPUNIT_ASSERT(!c.isSecureCookie());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT(c.isSecureCookie());
 
   c = cookies[2];
   CPPUNIT_ASSERT_EQUAL(std::string("passwd"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("secret"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/cgi-bin"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
 
   c = cookies[3];
@@ -268,7 +272,7 @@ void CookieStorageTest::testLoad()
   CPPUNIT_ASSERT_EQUAL(std::string("1000"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".overflow.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("overflow.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
 }
 
@@ -281,31 +285,31 @@ void CookieStorageTest::testLoad_sqlite3()
   std::vector<Cookie> cookies;
   dumpCookie(cookies, st);
   Cookie c = cookies[0];
-  CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), c.getName());
-  CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
-  CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
-  CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
-  CPPUNIT_ASSERT(c.isSecureCookie());
-  CPPUNIT_ASSERT(!c.isSessionCookie());
-
-  c = cookies[1];
   CPPUNIT_ASSERT_EQUAL(std::string("uid"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string(""), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)0, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/path/to"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.com"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
   CPPUNIT_ASSERT(c.isSessionCookie());
 
-  c = cookies[2];
+  c = cookies[1];
   CPPUNIT_ASSERT_EQUAL(std::string("foo"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("bar"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/path/to"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".overflow_time_t.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".overflow.time_t.org"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
   CPPUNIT_ASSERT(!c.isSessionCookie());
+
+  c = cookies[2];
+  CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), c.getName());
+  CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
+  CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
+  CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT(c.isSecureCookie());
+  CPPUNIT_ASSERT(!c.isSessionCookie());
     
 #else // !HAVE_SQLITE3
   CPPUNIT_ASSERT(!st.load("cookies.sqlite"));

test/NsCookieParserTest.cc

@@ -38,35 +38,35 @@ void NsCookieParserTest::testParse()
   CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), c.getDomain());
 
   c = cookies[1];
   CPPUNIT_ASSERT_EQUAL(std::string("user"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("me"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)1181473200, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".expired.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("expired.local"), c.getDomain());
 
   c = cookies[2];
   CPPUNIT_ASSERT_EQUAL(std::string("passwd"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("secret"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/cgi-bin"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), c.getDomain());
 
   c = cookies[3];
   CPPUNIT_ASSERT_EQUAL(std::string("TAX"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("1000"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".overflow.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("overflow.local"), c.getDomain());
 
   c = cookies[4];
   CPPUNIT_ASSERT_EQUAL(std::string("novalue"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string(""), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)2147483647, c.getExpiry());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".example.org"), c.getDomain());
 }
 
 void NsCookieParserTest::testParse_fileNotFound()

test/Sqlite3MozCookieParserTest.cc

@@ -36,7 +36,7 @@ void Sqlite3MozCookieParserTest::testParse()
   CPPUNIT_ASSERT_EQUAL((size_t)3, cookies.size());
 
   const Cookie& localhost = cookies[0];
-  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), localhost.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string("localhost.local"), localhost.getDomain());
   CPPUNIT_ASSERT_EQUAL(std::string("/"), localhost.getPath());
   CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), localhost.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("123456789"), localhost.getValue());
@@ -44,7 +44,7 @@ void Sqlite3MozCookieParserTest::testParse()
   CPPUNIT_ASSERT_EQUAL(true, localhost.isSecureCookie());
 
   const Cookie& nullValue = cookies[1];
-  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.local"), nullValue.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.com"), nullValue.getDomain());
   CPPUNIT_ASSERT_EQUAL(std::string("/path/to"), nullValue.getPath());
   CPPUNIT_ASSERT_EQUAL(std::string("uid"), nullValue.getName());
   CPPUNIT_ASSERT_EQUAL(std::string(""), nullValue.getValue());
@@ -54,7 +54,7 @@ void Sqlite3MozCookieParserTest::testParse()
   // See row id=3 has no name, so it is skipped.
 
   const Cookie& overflowTime = cookies[2];
-  CPPUNIT_ASSERT_EQUAL(std::string(".overflow_time_t.local"),
+  CPPUNIT_ASSERT_EQUAL(std::string(".overflow.time_t.org"),
                        overflowTime.getDomain());
   CPPUNIT_ASSERT_EQUAL(std::string("/path/to"), overflowTime.getPath());
   CPPUNIT_ASSERT_EQUAL(std::string("foo"), overflowTime.getName());

test/nscookietest.txt

@@ -6,4 +6,4 @@ expired	FALSE	/	FALSE	1181473200	user	me
 localhost	FALSE	/cgi-bin	FALSE	2147483647	passwd	secret
 badformat
 overflow	FALSE	/	FALSE	9223372036854775807	TAX	1000
-localhost	FALSE	/	FALSE	2147483648	novalue	
+.example.org	TRUE	/	FALSE	2147483648	novalue