Closed Bug 407845 Opened 12 years ago Closed 12 years ago

nsCookieService::FindCookie() shouldn't return expired cookies

Categories

(Core :: Networking: Cookies, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: dwitte, Assigned: dwitte)

Details

Attachments

(1 file)

i was pondering cookie code the other day, and happened upon a little bug:

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/netwerk/cookie/src/nsCookieService.cpp&rev=1.84#1381

if that includes expired cookies in its search, then the |foundCookie == PR_TRUE| branch of the following |if| block will do some wacky things, namely denying httpOnly cookies the right to be set etc. the original idea here was to avoid list traversals (the |else| branch) by using the expired cookie's spot, but we need to be a little more panache about it.
Assignee: nobody → dwitte
Attached patch patch v1Splinter Review
sanest to forget about the micro-optimization - fixes FindCookie to not return expired cookies, and as a side effect, fixes Remove to still blacklist in case of expiry. includes tests for the new behavior too!
Attachment #308170 - Flags: superreview?(cbiesinger)
Attachment #308170 - Flags: review?(cbiesinger)
Attachment #308170 - Flags: superreview?(cbiesinger)
Attachment #308170 - Flags: superreview+
Attachment #308170 - Flags: review?(cbiesinger)
Attachment #308170 - Flags: review+
Comment on attachment 308170 [details] [diff] [review]
patch v1

requesting approval - very low-risk patch that fixes a correctness issue (see comment 0); includes tests.
Attachment #308170 - Flags: approval1.9?
Comment on attachment 308170 [details] [diff] [review]
patch v1

Thanks for the risk explanation and for including tests!

a1.9+=damons
Attachment #308170 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Checking in netwerk/cookie/src/nsCookieService.cpp;
/cvsroot/mozilla/netwerk/cookie/src/nsCookieService.cpp,v  <--  nsCookieService.cpp
new revision: 1.94; previous revision: 1.93
done
Checking in netwerk/cookie/src/nsCookieService.h;
/cvsroot/mozilla/netwerk/cookie/src/nsCookieService.h,v  <--  nsCookieService.h
new revision: 1.40; previous revision: 1.39
done
Checking in netwerk/test/TestCookie.cpp;
/cvsroot/mozilla/netwerk/test/TestCookie.cpp,v  <--  TestCookie.cpp
new revision: 1.24; previous revision: 1.23
done
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.