From 10b31fba6c3e3373039358d6bb8febbf31d5dbaa Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Tue, 9 Feb 2010 13:07:16 -0800 Subject: [PATCH] Bug 542974 - Make the cookie parser's handling of " characters more like IE. r=dwitte --- netwerk/cookie/src/nsCookieService.cpp | 61 ++++++++------------------ netwerk/test/TestCookie.cpp | 38 +++++++++------- 2 files changed, 40 insertions(+), 59 deletions(-) diff --git a/netwerk/cookie/src/nsCookieService.cpp b/netwerk/cookie/src/nsCookieService.cpp index 9492be6fba3b..83e95977c5b4 100644 --- a/netwerk/cookie/src/nsCookieService.cpp +++ b/netwerk/cookie/src/nsCookieService.cpp @@ -1727,11 +1727,7 @@ nsCookieService::AddInternal(const nsCString &aBaseDomain, ** Begin BNF: token = 1* - value = token-value | quoted-string - token-value = 1* - quoted-string = ( <"> *( qdtext | quoted-pair ) <"> ) - qdtext = > ; CR | LF removed by necko - quoted-pair = "\" ; CR | LF removed by necko + value = 1* separators = ";" | "=" value-sep = ";" cookie-sep = CR | LF @@ -1766,7 +1762,6 @@ nsCookieService::AddInternal(const nsCString &aBaseDomain, // helper functions for GetTokenValue static inline PRBool iswhitespace (char c) { return c == ' ' || c == '\t'; } static inline PRBool isterminator (char c) { return c == '\n' || c == '\r'; } -static inline PRBool isquoteterminator(char c) { return isterminator(c) || c == '"'; } static inline PRBool isvalueseparator (char c) { return isterminator(c) || c == ';'; } static inline PRBool istokenseparator (char c) { return isvalueseparator(c) || c == '='; } @@ -1806,39 +1801,16 @@ nsCookieService::GetTokenValue(nsASingleFragmentCString::const_char_iterator &aI start = aIter; - if (*aIter == '"') { - // process - // (note: cookie terminators, CR | LF, can't happen: - // they're removed by necko before the header gets here) - // assume value mangled if no terminating '"', return - while (++aIter != aEndIter && !isquoteterminator(*aIter)) { - // if (backwhacked char), skip over it. this allows '\"' in . - // we increment once over the backwhack, nullcheck, then continue to the 'while', - // which increments over the backwhacked char. one exception - we don't allow - // CR | LF here either (see above about necko) - if (*aIter == '\\' && (++aIter == aEndIter || isterminator(*aIter))) - break; - } + // process + // just look for ';' to terminate ('=' allowed) + while (aIter != aEndIter && !isvalueseparator(*aIter)) + ++aIter; - if (aIter != aEndIter && !isterminator(*aIter)) { - // include terminating quote in attribute string - aTokenValue.Rebind(start, ++aIter); - // skip to next ';' - while (aIter != aEndIter && !isvalueseparator(*aIter)) - ++aIter; - } - } else { - // process - // just look for ';' to terminate ('=' allowed) - while (aIter != aEndIter && !isvalueseparator(*aIter)) - ++aIter; - - // remove trailing ; first check we're not at the beginning - if (aIter != start) { - lastSpace = aIter; - while (--lastSpace != start && iswhitespace(*lastSpace)); - aTokenValue.Rebind(start, ++lastSpace); - } + // remove trailing ; first check we're not at the beginning + if (aIter != start) { + lastSpace = aIter; + while (--lastSpace != start && iswhitespace(*lastSpace)); + aTokenValue.Rebind(start, ++lastSpace); } } @@ -1900,10 +1872,6 @@ nsCookieService::ParseAttributes(nsDependentCString &aCookieHeader, if (!tokenValue.IsEmpty()) { tokenValue.BeginReading(tempBegin); tokenValue.EndReading(tempEnd); - if (*tempBegin == '"' && *--tempEnd == '"') { - // our parameter is a quoted-string; remove quotes for later parsing - tokenValue.Rebind(++tempBegin, tempEnd); - } } // decide which attribute we have, and copy the string @@ -2272,8 +2240,15 @@ nsCookieService::GetExpiry(nsCookieAttributes &aCookieAttributes, PRTime tempExpires; PRInt64 expires; + // For Expires, we trim leading and trailing " characters to maximize + // the compatibility of our date parsing. In principle, this processsing + // should be done in our date parser. + nsCString& expiresAttr = aCookieAttributes.expires; + if (!expiresAttr.IsEmpty() && expiresAttr.First() == '"' && expiresAttr.Last() == '"') + expiresAttr = Substring(expiresAttr.BeginReading() + 1, expiresAttr.EndReading() - 1); + // parse expiry time - if (PR_ParseTimeString(aCookieAttributes.expires.get(), PR_TRUE, &tempExpires) == PR_SUCCESS) { + if (PR_ParseTimeString(expiresAttr.get(), PR_TRUE, &tempExpires) == PR_SUCCESS) { expires = tempExpires / PR_USEC_PER_SEC; } else { return PR_TRUE; diff --git a/netwerk/test/TestCookie.cpp b/netwerk/test/TestCookie.cpp index f16438f66b75..8f9cee0de81f 100644 --- a/netwerk/test/TestCookie.cpp +++ b/netwerk/test/TestCookie.cpp @@ -462,39 +462,45 @@ main(PRInt32 argc, char *argv[]) SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; expires=Thu, 10 Apr 1980 16:33:12 GMT", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[2] = CheckResult(cookie.get(), MUST_BE_NULL); - - SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=60", nsnull); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; expires=\"Thu, 10 Apr 1980 16:33:12 GMT", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[3] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry"); - SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=-20", nsnull); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; expires=\"Thu, 10 Apr 1980 16:33:12 GMT\"", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[4] = CheckResult(cookie.get(), MUST_BE_NULL); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=60", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[5] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry"); - SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; expires=Thu, 10 Apr 1980 16:33:12 GMT", nsnull); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=-20", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[6] = CheckResult(cookie.get(), MUST_BE_NULL); SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=60", nsnull); + GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); + rv[7] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry"); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; expires=Thu, 10 Apr 1980 16:33:12 GMT", nsnull); + GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); + rv[8] = CheckResult(cookie.get(), MUST_BE_NULL); + SetACookie(cookieService, "http://expireme.org/", nsnull, "test=expiry; max-age=60", nsnull); SetACookie(cookieService, "http://expireme.org/", nsnull, "newtest=expiry; max-age=60", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); - rv[7] = CheckResult(cookie.get(), MUST_CONTAIN, "test=expiry"); - rv[8] = CheckResult(cookie.get(), MUST_CONTAIN, "newtest=expiry"); + rv[9] = CheckResult(cookie.get(), MUST_CONTAIN, "test=expiry"); + rv[10] = CheckResult(cookie.get(), MUST_CONTAIN, "newtest=expiry"); SetACookie(cookieService, "http://expireme.org/", nsnull, "test=differentvalue; max-age=0", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); - rv[9] = CheckResult(cookie.get(), MUST_EQUAL, "newtest=expiry"); + rv[11] = CheckResult(cookie.get(), MUST_EQUAL, "newtest=expiry"); SetACookie(cookieService, "http://expireme.org/", nsnull, "newtest=evendifferentvalue; max-age=0", nsnull); GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); - rv[10] = CheckResult(cookie.get(), MUST_BE_NULL); - - SetACookie(cookieService, "http://foo.expireme.org/", nsnull, "test=expiry; domain=.expireme.org; max-age=60", nsnull); - GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); - rv[11] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry"); - SetACookie(cookieService, "http://bar.expireme.org/", nsnull, "test=differentvalue; domain=.expireme.org; max-age=0", nsnull); - GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); rv[12] = CheckResult(cookie.get(), MUST_BE_NULL); - allTestsPassed = PrintResult(rv, 13) && allTestsPassed; + SetACookie(cookieService, "http://foo.expireme.org/", nsnull, "test=expiry; domain=.expireme.org; max-age=60", nsnull); + GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); + rv[13] = CheckResult(cookie.get(), MUST_EQUAL, "test=expiry"); + SetACookie(cookieService, "http://bar.expireme.org/", nsnull, "test=differentvalue; domain=.expireme.org; max-age=0", nsnull); + GetACookie(cookieService, "http://expireme.org/", nsnull, getter_Copies(cookie)); + rv[14] = CheckResult(cookie.get(), MUST_BE_NULL); + + allTestsPassed = PrintResult(rv, 15) && allTestsPassed; // *** multiple cookie tests @@ -535,7 +541,7 @@ main(PRInt32 argc, char *argv[]) rv[1] = CheckResult(cookie.get(), MUST_BE_NULL); SetACookie(cookieService, "http://parser.test/", nsnull, "test=\"fubar! = foo;bar\\\";\" parser; domain=.parser.test; max-age=6\nfive; max-age=2.63,", nsnull); GetACookie(cookieService, "http://parser.test/", nsnull, getter_Copies(cookie)); - rv[2] = CheckResult(cookie.get(), MUST_CONTAIN, "test=\"fubar! = foo;bar\\\";\""); + rv[2] = CheckResult(cookie.get(), MUST_CONTAIN, "test=\"fubar! = foo"); rv[3] = CheckResult(cookie.get(), MUST_CONTAIN, "five"); SetACookie(cookieService, "http://parser.test/", nsnull, "test=kill; domain=.parser.test; max-age=0 \n five; max-age=0", nsnull); GetACookie(cookieService, "http://parser.test/", nsnull, getter_Copies(cookie));