14145 - Overdose of cookies

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Stephen P. Morse]

The sequence of cookies that necko is passing to the cookie module does not
appear to be correct.  Some cookies are being passed over and over whereas other
cookies are not being passed at all.  The particular test involved the netcenter
site but I would assume that other sites would display similar problems.

First I wanted to determine just what cookies netcenter is sending so I used the
4.x browser with the warn-me preference enabled.  I always answered CANCEL in
the nag box.  I noticed a difference between the first netcenter access per
session and all other accesses.  So I am looking only at the first access in
this test.

In 4.x I observed netcenter or its agents attempting to set the following
cookies.:

   netcenter: UIDC=...
   netcenter: UIDC=...
   netcenter: NGUserID=...
   adforce: JEB2
   netcenter: c=1

total cookie nags = 5

In 5.0 I saw the cookie nag box 21 times instead of the 5 I saw above.  Since I
could instrument 5.0, I did so and observed the following cookie requests being
passed from necko to the cookie module (note: some requests pass more than one
cookie, therefore we have 12 requests but 21 cookies).

url = http://home.netscape.com/
1. cookie = UIDC

url = http://home.netscape.com/images/ns_netcenter_bar.gif
2. cookie = UIDC
3. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
4. cookie = UIDC

url = http://home.netscape.com/images/pixel.gif
5. cookie = UIDC
6. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
7. cookie = UIDC
8. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
9. cookie = UIDC
10. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
11. cookie = UIDC
12. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
13. cookie = UIDC
14. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
15. cookie = UIDC
16. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
17. cookie = UIDC
18. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
19. cookie = UIDC
20. cookie = NGUserID

url = http://home.netscape.com/images/pixel.gif
21. cookie = UIDC

In summary, the cookies from the image pixel.jif is passed from necko to
the cookie module repeatedly whereas the foreign cookie from adforce is never
passed to the cookie module.  Also the c=1 cookie from netcenter is never being
passed either.

Coping Pam on this because the excessive cookies seem to be coming from a gif.

Comment 1

[Stephen P. Morse]

Here's something that might be of interest.  The server timestamps on these
repeated cookie requests are not all the same.  So it appears that necko might
actually be getting repeated cookie requests from the server.  In which case we
need to figure out what the browser is doing to cause the server to respond many
time.

In particular, from the server time stamps I noticed the following:

Cookie 1 was sent at 12:27:46
Cookies 2 thru 8 were sent .02 seconds later
Cookies 9 thru 16 were sent .01 second after that
Cookies 17 and 18 were sent .17 seconds after that
Cookies 19, 20, and 21 were actually sent .01 seconds before 17,18

Comment 2

[pnunn]

The image in question, pixel.gif, is one of those 1x1 images used as
spacers. Each time a different height or width tag is attached to an
image request, the image is stored _after_ it is sized in the image cache.
So each image, though using the same original image is viewed as a
different request.

Alot of users use this method, though using CSS is preferred.
-pn

Comment 3

[Stephen P. Morse]

I understand a bit more about this bug and see that at least part of the problem
is in the cookie module itself (see below).  So I'm assigning this bug back to
myself.

There may be three separate bugs here, I'm not sure yet.  One is the repeated
rquest to set certain cookies, one is the failure of the foreign cookie from
adforce to be set, and the third is that the netcenter cookie c=1 is never
getting set.

I know what is happening with the adforce cookie.  In that case, the header
being sent back in the http response does not contain a date entry.  That date
entry is supposed to be the server date and is used to adjust for time-zone
effects on the cookie expiration time.  And the reason that lack of a date field
is preventing the cookie from being set is because of the following code in
nsCookieHTTPNotify.cpp:

  if(pDate) {
    COOKIE_SetCookieStringFromHttp((char*)(const char*)url, cookie, pDate);
    nsCRT::free(pDate);
  }

So if the date is null, the SetCookieStringFromHttp routine never gets called.
The fix is to move the call to SetCookieStringFromHttp outside of the if
statement as follows:

  COOKIE_SetCookieStringFromHttp((char*)(const char*)url, cookie, pDate);
  if(pDate) {
    nsCRT::free(pDate);
  }

This alone is not sufficient because the SetCookieStringFromHttp routine expects
a non-null value for the date.  This can be corrected by changing the following
code to the SetCookieStringFromHttp routine from:

  sDate = cookie_ParseDate(server_date);

to:

  if (server_date) {
    sDate = cookie_ParseDate(server_date);
  } else {
    sDate = time(NULL);
  }

Ironically, although this fixes item 2, it makes item 1 worse.  For now the
adforce cookies are attempting to get set over and over again and, in fact, it
appears to be repeating an infinite number of times.  So before I go asking
permission to check this fix in, I need to find and fix the cause of item 1.

Changing the tfv to M12 because setting of cookies is definitely a beta issue.
Furthermore, repeated attempts at setting cookies unnecessarily is certainly a
performance issue.

Comment 4

[Stephen P. Morse]

I understand item 1 (repeated requests to set certain cookies) a bit better now.
Recall that 4.x was setting the UIDC cookie only twice whereas in 5.0 it was
being set 12 times.  And all but the first setting of this cookie was coming
from the 1x1 pixel image that Pam described.

Although I couldn't instrument 4.x to see what was happening (the code base has
bit-rotted and it is no longer possible for mere mortals to build it), I was
still able to deduce things about it.  In particular, I discovered that if I
cleared out my cache before visiting netcenter, I got numerous (much more than
two) requests to set the UIDC cookie.  So I presume that the image winds up in
the cache and as long as it is being refetched from the cache there are no
cookie-setting requests made on its behalf.  And since 5.0 doesn't yet have a
cache, this explains why 5.0 always has so many requests to set UIDC.  So item
#1 is not a bug but just a consequence of cache not yet being implemented.

Now the only outstanding part of this bug report is item 3 -- c=1 cookie never
gets set.  This is a javascript cookie so maybe all javascript cookies are
broken.  I'm still invesigating.

Comment 5

[pnunn]

Steve:
Please note that the image is cached by its size. A new width or height will
cause the image to be downloaded again from the server. So, even when the
netwerk cache is implemented, you will still get multiple calls for the image.

This is how images worked in 4.x, too.

-pn

Comment 6

[Stephen P. Morse]

Guess what.  With a tree that I pulled today at noon, I am no longer seeing item
#3 (c=1 cookie not being set).  So let's summarize the status of this bug
report:

1. Repeated request to set the netcenter UIDC cookie
   status: this is due to cache not yet implemented.  So ignore this item

2. Failure of foreign cookies from adforce to be set
   status: coding error involving sites that don't have an HTTP date entry
           I have fix ready to be checked in if I'm given permission to do so

3. Failure to set the netcenter c=1 cookie.
   status: This has suddenly started working as of today's builds.

Comment 7

[Stephen P. Morse]

Fix for item 2 has now been checked in.  Item 1 is a cache issue and covered in
other bug reports.  And item 3 suddenly started working by itself.  Closing out
this bug as fixed.

Comment 8

[leger]

Bulk move of all Necko (to be deleted component) bugs to new Networking

component.

Comment 9

[Tom Everingham]

verified:  NT 2000012520

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c5b710c6391
[fenix] For https://github.com/mozilla-mobile/fenix/issues/14145 - Add pinned badge for pinned sites