Opened 8 years ago

Closed 6 years ago

#7770 closed (fixed)

HttpResponse.set_cookie should accept expires=datetime

Reported by: Jeremy Dunck Owned by: Chris Beaven
Component: HTTP handling Version: master
Severity: Keywords:
Cc: goliath.mailinglist@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

set_cookie accepts a keyword argument, expires, which is expected to be an RFC-formatted string for datetime.

It should (also?) accept a datetime and possibly epoch-seconds.

In general, set_cookie may need to be reviewed for usefulness.

See also: #6657

Attachments (4)

cookie-datetime.diff (1.2 KB) - added by Ben Firshman 8 years ago.
7770.diff (3.3 KB) - added by Chris Beaven 8 years ago.
7770.2.diff (4.5 KB) - added by Chris Beaven 6 years ago.
7770.3.diff (4.6 KB) - added by Chris Beaven 6 years ago.

Download all attachments as: .zip

Change History (16)

Changed 8 years ago by Ben Firshman

Attachment: cookie-datetime.diff added

comment:1 Changed 8 years ago by Sung-jin Hong

Has patch: set
Triage Stage: UnreviewedAccepted

Looks good to me.

comment:2 Changed 8 years ago by David Danier <goliath.mailinglist@…>

Perhaps max_age could accept a timedelta-object as well? Perhaps even a datetime-object would be possible for max_age (converted to timedelta, converted to seconds)?

comment:3 Changed 8 years ago by David Danier <goliath.mailinglist@…>

Cc: goliath.mailinglist@… added

comment:4 Changed 8 years ago by Ben Firshman

Perhaps it should generate both from one argument? They are essentially the same thing. It would be nice if there were default settings for domain, path, secure and expires (as a timedelta?) too.

comment:5 Changed 8 years ago by Malcolm Tredinnick

milestone: 1.0 alpha

Removing milestone. If we get to it before 1.0, well and good. If not, it's not backwards incompatible, so it could wait until afterwards.

comment:6 Changed 8 years ago by Chris Beaven

Needs tests: set

Yep, it should definitely generate the max-age (if it wasn't explicitly given).

Changed 8 years ago by Chris Beaven

Attachment: 7770.diff added

comment:7 Changed 8 years ago by Chris Beaven

Needs tests: unset
Triage Stage: AcceptedReady for checkin

Ok, here's the patch with tests.

I should have done this work when I wrote the custom session age stuff...

comment:8 Changed 8 years ago by Chris Beaven

Owner: changed from nobody to Chris Beaven
Status: newassigned

comment:9 Changed 6 years ago by Chris Beaven

Needs documentation: set
Triage Stage: Ready for checkinAccepted

(cough)

Changed 6 years ago by Chris Beaven

Attachment: 7770.2.diff added

comment:10 Changed 6 years ago by Chris Beaven

Needs documentation: unset

Added docs, and changed to expect datetime.datetime objects in UTC.

comment:11 Changed 6 years ago by Chris Beaven

Triage Stage: AcceptedReady for checkin

The latest patch also fixes #13548, and is simplified in that expires only takes a datetime.datetime. If you want to use a datetime.timedelta then you can pass max_age the equivalent seconds.

I'm happy with it. Bumping to RFC

Changed 6 years ago by Chris Beaven

Attachment: 7770.3.diff added

comment:12 Changed 6 years ago by Malcolm Tredinnick

Resolution: fixed
Status: assignedclosed

(In [13809]) Allow setting HttpResponse cookie expiry times with datetime objects.

Patch from SmileyChris. Fixed #7770.

Note: See TracTickets for help on using tickets.
Back to Top