In an attempt to get my bug list in order again, marking all the bugs I have currently as ASSIGNED.
Prashant, could you confirm that this is still a problem? I can't seem to find a Mac build tonight and, since the bug is fairly hope, I'm hoping it's fixed itself. If it is fixed, please close it. Thanks.
Putting on PDT+ radar for beta1.
Well, I tried to debug this on a Mac with no luck. For whatever reason, I can't seem to step into some of the code in COOKIE_GetCookie(). I'm passing this along to the owner of cookies for further investigation - I've convinced myself that the DOM invocation of the cookie code is correct.
scott could you help steve with this.
Getting cookies works fine. Setting cookies usually works fine. However, setting cookies that expire runs into trouble. Problem is: the date gets converted incorrectly. For a nice string like "Tue, 13 Feb 2001 15:16:23 GMT", | PR_ParseTimeString| returns 982077383. This number is supposed to be microseconds since the epoch, but since that is barely 12 days worth of microseconds, this seems incorrect. The cookie is expired before it is even added to the list. That can't be good. So, my analysis is that |PR_ParseTimeString| is broken on the Mac. Investigating further into that; though now it just might be time to re-assign the bug into the NSPR team.
The HTTP code path is different. In that code path, we call |PR_ParseTimeString| twice, once for the servers current time, once for the time it wants the cookie to expire ... then we add that to the current time on the client machine. That ends up making a cookie that expires in the future, but not very far in the future. An interesting thing to note about the wrong time given above is that it is off by a factor of 1000. As suggested by <firstname.lastname@example.org> some platform routines may return seconds instead of microseconds. That suggestion fits the observed behavior. <email@example.com> has asked me to re-assign this bug into the NSPR group; but my current understanding is they are extremely understaffed and not taking new bugs ... so I'm going to look deeper and see if I can't come up with a real fix and submit a patch with this bug to make it easier for them. Steve Morse is also looking at at |PR_ParseTimeString| to help find the real problem.
No. This isn't right somehow. |PR_ParseTimeString| is returning the right number of microseconds. And this number is getting converted into seconds in | cookie_ParseDate|, so seconds (not microseconds) must be the canonical form for cookies. Still, the value is significantly smaller than other cookie expires dates, but it adds up to the right date in seconds. Hmmm. I just watched it do everything including add the cookie to the list, but saving cookies didn't write the new cookie out to the file. What does this mean?
Then, I see as I get the cookie, that this cookie is deleted during the search because it is considered expired. What the hell is going on? Steve? Do you have any clue what this is?
It means that you can now use the cookie viewer to see what the expiration date/time is rather than having to calculate it by hand. Once the cookie is added to the cookie list, the cookie viewer will display it. It's under tasks/autofill/display-cookies.
One thing to be aware of is that the cookie expiration time is recorded as seconds since 1-1-70 on all platforms. In 4.7 the mac used to record it as seconds since 1-1-1900 instead (because I believe that was how the native clock in the mac kept time) whereas the other platforms in 4.x used the 1-1-70 baseline. Could this be relevant here?
That's what it is, Steve. The line time_t cur_time = time(NULL); at the top of |COOKIE_GetCookie| on the Mac is returning the time since 1900. And it's always expired. The right fix might be to call |PR_Now| and then convert it an unsigned long. What do you think?
Or we could subtract out the number of seconds between 1900 and 1970 (for mac-only of course). That number is 2,208,988,800 -- I've already calculated that in bug 15111.
OK, you actually use the bad |time(NULL)| function in several places. Here's my patch which appears to work. I replaced several cases so it definitely needs review.
OK, I presume that the PR_NOW() function on all platforms returns the current time in microseconds since 1970 whereas time(NULL) on the mac reterns it as seconds since 1900. Is that correct? In that case your change looks fine. You might want to add a comment to your get_current_time routine that gives the reason you are not using time(NULL) because someone looking at this code in the future might be tempted to simplify it to use time(NULL). Of course you also could have had get_current_time use time(NULL) and then ifdef the mac and subtract out my magic number in that case. Then you wouldn't have needed all the LL_ routines. But either way is fine, so go for it. Thanks for finding this problem Scott.
Fix checked in with "nsCookie.cpp" version 1.53
Verified with 2000-02-14-09.
Marked verified by mistake. Meant to verify other bug. reopening and again marking it fixed.
Working fine now. Tested with 02-17.