diff --git a/ChangeLog b/ChangeLog index 57e83c0d..8666b094 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2008-12-16 Tatsuhiro Tsujikawa + + 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 Fixed the bug that causes corrupted downloads if HTTP pipelining diff --git a/src/Cookie.cc b/src/Cookie.cc index 8caeca70..924bbec2 100644 --- a/src/Cookie.cc +++ b/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; } diff --git a/test/CookieParserTest.cc b/test/CookieParserTest.cc index e07056c6..816daed3 100644 --- a/test/CookieParserTest.cc +++ b/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()); diff --git a/test/CookieStorageTest.cc b/test/CookieStorageTest.cc index 2bd05cac..77b791d6 100644 --- a/test/CookieStorageTest.cc +++ b/test/CookieStorageTest.cc @@ -1,12 +1,15 @@ #include "CookieStorage.h" + +#include +#include + +#include + #include "Exception.h" #include "Util.h" #include "TimeA2.h" #include "CookieParser.h" #include "RecoverableException.h" -#include -#include -#include 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()); diff --git a/test/CookieTest.cc b/test/CookieTest.cc index 35795c06..cc1c67d4 100644 --- a/test/CookieTest.cc +++ b/test/CookieTest.cc @@ -1,9 +1,12 @@ #include "Cookie.h" + +#include + +#include + #include "Exception.h" #include "Util.h" #include "TimeA2.h" -#include -#include 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 diff --git a/test/NsCookieParserTest.cc b/test/NsCookieParserTest.cc index c52d86ff..055e9c14 100644 --- a/test/NsCookieParserTest.cc +++ b/test/NsCookieParserTest.cc @@ -1,8 +1,11 @@ #include "NsCookieParser.h" + +#include + +#include + #include "RecoverableException.h" #include "Util.h" -#include -#include namespace aria2 { @@ -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() diff --git a/test/Sqlite3MozCookieParserTest.cc b/test/Sqlite3MozCookieParserTest.cc index 8460e944..afb446ec 100644 --- a/test/Sqlite3MozCookieParserTest.cc +++ b/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());