Short-lived cookies get "upgraded" to session cookies

VERIFIED FIXED in mozilla0.9.9

Status

()

Core
Networking: Cookies
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Chris Underhill, Assigned: Stephen P. Morse)

Tracking

Trunk
mozilla0.9.9
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

16 years ago
Mozilla 2001121308-trunk on Linux: Red Hat 7.1+updates

When using P3P or the preferences to downgrade cookies sent by the site to
session cookies, short-lived cookies no longer expire. This stops sites which
use this mechanism to force idle users to relogin from doing so.

See also comments 11 and 12 on bugid 114709.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED
OS: Linux → All
Target Milestone: --- → mozilla0.9.9
(Assignee)

Updated

16 years ago
Keywords: nsbeta1
(Assignee)

Comment 1

16 years ago
OK, the way to fix this is to yank out all the current tests for expiring the 
cookie if status is downgrade.  Instead add a test at the time that the cookie 
is being written to the file, and if the cookie has a status of downgrade, don't 
write it.

Attaching a composite patch pertaining to bug 121311, bug 121685, and this one 
(because they all modify the same file).  The portions of the patch that are 
specific to this bug are:

1. removing the test for nsICookie::STATUS_DOWNGRADED before setting expires
   to cookie_ParseDate(date)

2. removing the test for nsICookie::STATUS_DOWNGRADED which results in setting
   gmtCookieExpires to 0

3. adding the test for nsICookie::STATUS_DOWNGRADED before writing the cookie
   to the file.
(Assignee)

Comment 2

16 years ago
Created attachment 68499 [details] [diff] [review]
composite patch for this bug, bug 121311, and bug 121685
(Assignee)

Comment 3

16 years ago
alecf, sgehani, please review

Comment 4

16 years ago
Comment on attachment 68499 [details] [diff] [review]
composite patch for this bug, bug 121311, and bug 121685

there's got to be a better way to get the max int..did you look in the NSPR
header files?

Comment 5

16 years ago
Comment on attachment 68499 [details] [diff] [review]
composite patch for this bug, bug 121311, and bug 121685

r=sgehani for the non-121311 (part Alec has a concern with) and non-121685
portions of this patch.
Attachment #68499 - Flags: review+
(Assignee)

Comment 6

16 years ago
In prlong.h there is defined LL_MAXINT.  But that is defined as an int64 whereas 
I need it for a long.  When I tried to use it I got the warning message:

  warning C4244: '=' : conversion from '__int64' to 'long',
                       possible loss of data

The value being returned by LL_MAXINT is 0x7fffffff, 0xffffffff.  The value that 
I am generating is a leading 0 followed by all 1's, independent of the 
number of bits.

Is there something else that you know about that I should use?

Comment 7

16 years ago
Comment on attachment 68499 [details] [diff] [review]
composite patch for this bug, bug 121311, and bug 121685

my concern is that you're relying on the bit format of time_t, rather than
treating it as an abstract type. What if time_t is signed? or unsigned? or what
if it IS actually 64-bit?

I think this reveals a fundamental flaw in the use of cookie_ParseDate()

you should be using the guaranteed-to-be-the-same-on-all-platforms PRTime

I just read the comments in get_current_time() and I see you've basically
painted yourself into a corner by not using PRTime "since the value is 64bits
and too hard to manipulate" (quoted fromt he comments at the top of
get_current_time)



I'm not sure what to say here, except that you're relying on assumptions about
time_t that simply won't hold true on all platforms.
(Assignee)

Comment 8

16 years ago
Created attachment 68966 [details] [diff] [review]
Separate patch, no longer a composite.
(Assignee)

Comment 9

16 years ago
Alec, samir, please review.  The patch is no longer a composite patch but 
applies to this bug report only.

Comment 10

16 years ago
Comment on attachment 68966 [details] [diff] [review]
Separate patch, no longer a composite.

r=sgehani
Attachment #68966 - Flags: review+

Comment 11

16 years ago
Comment on attachment 68966 [details] [diff] [review]
Separate patch, no longer a composite.

sr=alecf
Attachment #68966 - Flags: superreview+
(Assignee)

Comment 12

16 years ago
This was checked in yesterday.  Marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 13

16 years ago
verified:  03/13/02 builds
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.