2008-12-16 Tatsuhiro Tsujikawa <t-tujikawa@users.sourceforge.net>

	Fixed the cookie implementation based on RFC2965. 
	Now if a value in domain field is not started with dot, then
	prepend dot. That means a cookie with domain=sf.net is sent to
	wiki.sf.net.
	* src/Cookie.cc
	* test/CookieParserTest.cc
	* test/CookieStorageTest.cc
	* test/CookieTest.cc
	* test/NsCookieParserTest.cc
	* test/Sqlite3MozCookieParserTest.cc

ChangeLog

@@ -1,3 +1,16 @@
+2008-12-16  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
+
+	Fixed the cookie implementation based on RFC2965. 
+	Now if a value in domain field is not started with dot, then
+	prepend dot. That means a cookie with domain=sf.net is sent to
+	wiki.sf.net.
+	* src/Cookie.cc
+	* test/CookieParserTest.cc
+	* test/CookieStorageTest.cc
+	* test/CookieTest.cc
+	* test/NsCookieParserTest.cc
+	* test/Sqlite3MozCookieParserTest.cc
+
 2008-12-16  Tatsuhiro Tsujikawa  <t-tujikawa@users.sourceforge.net>
 
 	Fixed the bug that causes corrupted downloads if HTTP pipelining

src/Cookie.cc

@@ -42,6 +42,30 @@
 
 namespace aria2 {
 
+static std::string prependDotIfNotExists(const std::string& domain)
+{
+  // From RFC2965:
+  // * Domain=value
+  //   OPTIONAL.  The value of the Domain attribute specifies the domain
+  //   for which the cookie is valid.  If an explicitly specified value
+  //   does not start with a dot, the user agent supplies a leading dot.
+  return (!domain.empty() && domain[0] != '.') ? "."+domain : domain;
+}
+
+static std::string normalizeDomain(const std::string& domain)
+{
+  if(domain.empty() || Util::isNumbersAndDotsNotation(domain)) {
+    return domain;
+  }
+  std::string md = prependDotIfNotExists(domain);
+  // TODO use Util::split to strict verification
+  std::string::size_type p = md.find_last_of(".");
+  if(p == 0 || p == std::string::npos) {
+    md += ".local";
+  }
+  return Util::toLower(prependDotIfNotExists(md));
+}
+
 Cookie::Cookie(const std::string& name,
 	       const std::string& value,
 	       time_t  expiry,
@@ -52,7 +76,7 @@ Cookie::Cookie(const std::string& name,
   _value(value),
   _expiry(expiry),
   _path(path),
-  _domain(Util::toLower(domain)),
+  _domain(normalizeDomain(domain)),
   _secure(secure) {}
 
 Cookie::Cookie(const std::string& name,
@@ -64,7 +88,7 @@ Cookie::Cookie(const std::string& name,
   _value(value),
   _expiry(0),
   _path(path),
-  _domain(Util::toLower(domain)),
+  _domain(normalizeDomain(domain)),
   _secure(secure) {}
 
 Cookie::Cookie():_expiry(0), _secure(false) {}
@@ -97,23 +121,28 @@ static bool pathInclude(const std::string& requestPath, const std::string& path)
   return true;
 }
 
-static bool domainMatch(const std::string& requestHost,
+static bool domainMatch(const std::string& normReqHost,
 			const std::string& domain)
 {
-  if(*domain.begin() == '.') {
-    return Util::endsWith("."+requestHost, domain);
-  } else {
-    return requestHost == domain;
-  }
+  // RFC2965 stated that:
+  //
+  // A Set-Cookie2 with Domain=ajax.com will be accepted, and the
+  // value for Domain will be taken to be .ajax.com, because a dot
+  // gets prepended to the value.
+  //
+  // Also original Netscape implementation behaves exactly the same.
+
+  // _domain always starts ".". See Cookie::Cookie().
+  return Util::endsWith(normReqHost, domain);
 }
 
 bool Cookie::match(const std::string& requestHost,
 		   const std::string& requestPath,
 		   time_t date, bool secure) const
 {
-  std::string lowerRequestHost = Util::toLower(requestHost);
+  std::string normReqHost = normalizeDomain(requestHost);
   if((secure || (!_secure && !secure)) &&
-     domainMatch(lowerRequestHost, _domain) &&
+     domainMatch(normReqHost, _domain) &&
      pathInclude(requestPath, _path) &&
      (isSessionCookie() || (date < _expiry))) {
     return true;
@@ -123,10 +152,12 @@ bool Cookie::match(const std::string& requestHost,
 }
 
 bool Cookie::validate(const std::string& requestHost,
-		      const std::string& requestPath) const
+                      const std::string& requestPath) const
 {
-  std::string lowerRequestHost = Util::toLower(requestHost);
-  if(lowerRequestHost != _domain) {
+  std::string normReqHost = normalizeDomain(requestHost);
+  // If _domain is IP address, then it should be matched to requestHost.
+  // In other words, _domain == normReqHost
+  if(normReqHost != _domain) {
     // domain must start with '.'
     if(*_domain.begin() != '.') {
       return false;
@@ -139,24 +170,23 @@ bool Cookie::validate(const std::string& requestHost,
     if(_domain.size() < 4 || _domain.find(".", 1) == std::string::npos) {
       return false;
     }
-    if(!Util::endsWith(lowerRequestHost, _domain)) {
+    if(!Util::endsWith(normReqHost, _domain)) {
       return false;
     }
-    // From RFC2109
-    // * The request-host is a FQDN (not IP address) and has the form HD,
+    // From RFC2965 3.3.2 Rejecting Cookies
+    // * The request-host is a HDN (not IP address) and has the form HD,
     //   where D is the value of the Domain attribute, and H is a string
     //   that contains one or more dots.
-    if(std::count(lowerRequestHost.begin(),
-		  lowerRequestHost.begin()+
-		  (lowerRequestHost.size()-_domain.size()), '.')
-       > 0) {
+    size_t dotCount = std::count(normReqHost.begin(),
+                                 normReqHost.begin()+
+                                 (normReqHost.size()-_domain.size()), '.');
+    if(dotCount > 1 || dotCount == 1 && normReqHost[0] != '.') {
       return false;
     } 
   }
   if(requestPath != _path) {
-    // From RFC2109
-    // * The value for the Path attribute is not a prefix of the request-
-    //   URI.
+    // From RFC2965 3.3.2 Rejecting Cookies
+    // * The value for the Path attribute is not a prefix of the request-URI.
     if(!pathInclude(requestPath, _path)) {
       return false;
     }

test/CookieParserTest.cc

@@ -33,7 +33,7 @@ void CookieParserTest::testParse()
   CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)1181473200, c.getExpiry());  
   CPPUNIT_ASSERT_EQUAL(std::string("/"), c.getPath());
-  CPPUNIT_ASSERT_EQUAL(std::string("localhost"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
   CPPUNIT_ASSERT_EQUAL(true, c.isSecureCookie());
   CPPUNIT_ASSERT_EQUAL(false, c.isSessionCookie());
 
@@ -43,7 +43,7 @@ void CookieParserTest::testParse()
   CPPUNIT_ASSERT_EQUAL(std::string("JSESSIONID"), c.getName());
   CPPUNIT_ASSERT_EQUAL(std::string("123456789"), c.getValue());
   CPPUNIT_ASSERT_EQUAL((time_t)0, c.getExpiry());
-  CPPUNIT_ASSERT_EQUAL(std::string("default.domain"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".default.domain"), c.getDomain());
   CPPUNIT_ASSERT_EQUAL(std::string("/default/path"), c.getPath());
   CPPUNIT_ASSERT_EQUAL(false, c.isSecureCookie());
   CPPUNIT_ASSERT_EQUAL(true, c.isSessionCookie());

test/CookieStorageTest.cc

@@ -1,12 +1,15 @@
 #include "CookieStorage.h"
+
+#include <iostream>
+#include <algorithm>
+
+#include <cppunit/extensions/HelperMacros.h>
+
 #include "Exception.h"
 #include "Util.h"
 #include "TimeA2.h"
 #include "CookieParser.h"
 #include "RecoverableException.h"
-#include <iostream>
-#include <algorithm>
-#include <cppunit/extensions/HelperMacros.h>
 
 namespace aria2 {
 
@@ -175,7 +178,7 @@ void CookieStorageTest::testLoad()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
   CPPUNIT_ASSERT(c.isSecureCookie());
 
   c = *(st.begin()+1);
@@ -183,7 +186,7 @@ void CookieStorageTest::testLoad()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
 
   c = *(st.begin()+2);
@@ -191,7 +194,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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".overflow.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
 
   c = *(st.begin()+3);
@@ -199,7 +202,7 @@ void CookieStorageTest::testLoad()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
 }
 
@@ -215,7 +218,7 @@ void CookieStorageTest::testLoad_sqlite3()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), c.getDomain());
   CPPUNIT_ASSERT(c.isSecureCookie());
   CPPUNIT_ASSERT(!c.isSessionCookie());
 
@@ -224,7 +227,7 @@ void CookieStorageTest::testLoad_sqlite3()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
   CPPUNIT_ASSERT(c.isSessionCookie());
 
@@ -233,7 +236,7 @@ void CookieStorageTest::testLoad_sqlite3()
   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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".overflow_time_t.local"), c.getDomain());
   CPPUNIT_ASSERT(!c.isSecureCookie());
   CPPUNIT_ASSERT(!c.isSessionCookie());
     

test/CookieTest.cc

@@ -1,9 +1,12 @@
 #include "Cookie.h"
+
+#include <iostream>
+
+#include <cppunit/extensions/HelperMacros.h>
+
 #include "Exception.h"
 #include "Util.h"
 #include "TimeA2.h"
-#include <iostream>
-#include <cppunit/extensions/HelperMacros.h>
 
 namespace aria2 {
 
@@ -14,6 +17,7 @@ class CookieTest:public CppUnit::TestFixture {
   CPPUNIT_TEST(testOperatorEqual);
   CPPUNIT_TEST(testMatch);
   CPPUNIT_TEST(testIsExpired);
+  CPPUNIT_TEST(testNormalizeDomain);
   CPPUNIT_TEST_SUITE_END();
 public:
   void setUp() {}
@@ -24,6 +28,7 @@ public:
   void testOperatorEqual();
   void testMatch();
   void testIsExpired();
+  void testNormalizeDomain();
 };
 
 
@@ -36,10 +41,10 @@ void CookieTest::testValidate()
     CPPUNIT_ASSERT(defaultDomainPath.validate("localhost", "/"));
   }
   {
-    Cookie domainStartsWithDot("k", "v", "/", "aria2.org", false);
-    CPPUNIT_ASSERT(!domainStartsWithDot.validate("www.aria2.org", "/"));
-    Cookie success("k", "v", "/", ".aria2.org", false);
-    CPPUNIT_ASSERT(success.validate("www.aria2.org", "/"));
+    Cookie domainStartsWithoutDot("k", "v", "/", "aria2.org", false);
+    CPPUNIT_ASSERT(domainStartsWithoutDot.validate("www.aria2.org", "/"));
+    Cookie domainStartsWithDot("k", "v", "/", ".aria2.org", false);
+    CPPUNIT_ASSERT(domainStartsWithDot.validate("www.aria2.org", "/"));
   }
   {
     Cookie domainWithoutEmbeddedDot("k", "v", "/", ".org", false);
@@ -105,12 +110,22 @@ void CookieTest::testOperatorEqual()
   CPPUNIT_ASSERT(!(a == wrongDomain));
   CPPUNIT_ASSERT(!(a == wrongName));
   CPPUNIT_ASSERT(!(a == caseSensitiveName));
+  // normalize
+  Cookie n1("k", "v", "/", "localhost", false);
+  Cookie n2("k", "v", "/", ".localhost", false);
+  Cookie n3("k", "v", "/", "localhost.local", false);
+  Cookie n4("k", "v", "/", ".localhost.local", false);
+  CPPUNIT_ASSERT(n1 == n2);
+  CPPUNIT_ASSERT(n1 == n3);
+  CPPUNIT_ASSERT(n1 == n4);
 }
 
 void CookieTest::testMatch()
 {
   Cookie c("k", "v", "/downloads", ".aria2.org", false);
   Cookie c2("k", "v", "/downloads/", ".aria2.org", false);
+  Cookie c3("k", "v", "/downloads/", "aria2.org", false);
+  Cookie c4("k", "v", "/downloads/", "localhost", false);
   CPPUNIT_ASSERT(c.match("www.aria2.org", "/downloads", 0, false));
   CPPUNIT_ASSERT(c.match("www.aria2.org", "/downloads/", 0, false));
   CPPUNIT_ASSERT(c2.match("www.aria2.org", "/downloads", 0, false));
@@ -121,12 +136,14 @@ void CookieTest::testMatch()
   CPPUNIT_ASSERT(c.match("www.aria2.org", "/downloads/latest", 0, false));
   CPPUNIT_ASSERT(!c.match("www.aria2.org", "/downloadss/latest", 0, false));
   CPPUNIT_ASSERT(!c.match("www.aria2.org", "/DOWNLOADS", 0, false));
+  CPPUNIT_ASSERT(c3.match("www.aria2.org", "/downloads", 0, false));
+  CPPUNIT_ASSERT(c4.match("localhost", "/downloads", 0, false));
 
   Cookie secureCookie("k", "v", "/", "secure.aria2.org", true);
   CPPUNIT_ASSERT(secureCookie.match("secure.aria2.org", "/", 0, true));
   CPPUNIT_ASSERT(!secureCookie.match("secure.aria2.org", "/", 0, false));
   CPPUNIT_ASSERT(!secureCookie.match("ssecure.aria2.org", "/", 0, true));
-  CPPUNIT_ASSERT(!secureCookie.match("www.secure.aria2.org", "/", 0, true));
+  CPPUNIT_ASSERT(secureCookie.match("www.secure.aria2.org", "/", 0, true));
 
   Cookie expireTest("k", "v", 1000, "/", ".aria2.org", false);
   CPPUNIT_ASSERT(expireTest.match("www.aria2.org", "/", 999, false));
@@ -147,7 +164,16 @@ void CookieTest::testIsExpired()
   Cookie sessionCookie("k", "v", "/", "localhost", false);
   CPPUNIT_ASSERT(!sessionCookie.isExpired());
   Cookie sessionCookie2("k", "v", 0, "/", "localhost", false);
-  CPPUNIT_ASSERT(!sessionCookie2.isExpired());  
+  CPPUNIT_ASSERT(!sessionCookie2.isExpired());
+
+}
+
+void CookieTest::testNormalizeDomain()
+{
+  Cookie dot("k", "v", "/", ".", false);
+  CPPUNIT_ASSERT_EQUAL(std::string("..local"), dot.getDomain());
+  Cookie ip("k", "v", "/", "192.168.1.1", false);
+  CPPUNIT_ASSERT_EQUAL(std::string("192.168.1.1"), ip.getDomain());
 }
 
 } // namespace aria2

test/NsCookieParserTest.cc

@@ -1,9 +1,12 @@
 #include "NsCookieParser.h"
-#include "RecoverableException.h"
-#include "Util.h"
+
 #include <iostream>
+
 #include <cppunit/extensions/HelperMacros.h>
 
+#include "RecoverableException.h"
+#include "Util.h"
+
 namespace aria2 {
 
 class NsCookieParserTest:public CppUnit::TestFixture {
@@ -35,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"), 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"), 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"), 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"), 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"), c.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".localhost.local"), 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"), 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"), nullValue.getDomain());
+  CPPUNIT_ASSERT_EQUAL(std::string(".null_value.local"), 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"),
+  CPPUNIT_ASSERT_EQUAL(std::string(".overflow_time_t.local"),
 		       overflowTime.getDomain());
   CPPUNIT_ASSERT_EQUAL(std::string("/path/to"), overflowTime.getPath());
   CPPUNIT_ASSERT_EQUAL(std::string("foo"), overflowTime.getName());