74112 - Cookie following deleted cookie doesn't get sent out.

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[Stephen P. Morse]

This bug is a fallout of bug 73964.  The following is a copy of the pertanant 
discussion from that report.

... there's a bug in the cookie module as follows.  When cookies are being 
sent out to the server, we loop through the cookie list looking for cookies that 
belong to that server and send them out.  While looping we also check each 
cookie to see if it expired and we delete it from the list if it is.  But we do 
not alter the index which we are using to loop through the list.  So, for 
example if we determine cookie at position 3 has expired, we delete it and then 
go and test cookie at position 4.  But the act of deletion has moved all cookies 
higher than #3 down one position.  So the cookie at position 4 is really the 
cookie that used to be at position 5.  Note that we completely skipped over the 
cookie that was originally at position 4 and never tested it.  And if that 
cookie belongs to the server in question, it will not get sent although it is 
suposed to.

Note that many things have to all come together in order to hit this problem.  
Specifically the list contains a cookie that has expired followed immediately by 
a cookie that is supposed to get sent to the server.  It's not a likely event 
and normally not a very reproducible one (the expiring cookie) if it does occur.  
But if the host itself expires its own cookies (for the purpose of deleting 
them), then this will be reproducible.  And this is exactly what compuserve was 
doing.

Well the fix for that is easy.  I'll post it right here, inline, instead of 
making an attachment.  In my next comment I'll discuss the next problem.

Index: nsCookies.cpp
===================================================================
RCS file: /cvsroot/mozilla/extensions/cookie/nsCookies.cpp,v
retrieving revision 1.1
diff -u -r1.1 nsCookies.cpp
--- nsCookies.cpp       2001/03/29 01:53:21     1.1
+++ nsCookies.cpp       2001/03/30 04:33:03
@@ -496,7 +496,7 @@
       /* check for expired cookies */
       if( cookie_s->expires && (cookie_s->expires < cur_time) ) {
         /* expire and remove the cookie */
-        cookie_list->RemoveElementAt(i);
+        cookie_list->RemoveElementAt(i--); /* decr i so next cookie isn't skipp
ed */
         deleteCookie((void*)cookie_s, nsnull);
         cookie_changed = PR_TRUE;
         continue;

Comment 1

[Stephen P. Morse]

This bug already received an r=valeski (from the comments in bug 73964).  So now 
it needs a super-review.  cc'ing alecf for that.

Comment 2

[Alec Flett]

sr=alecf

Comment 3

[Stephen P. Morse]

Fix checked in.

Comment 4

[Tom Everingham]

verified