Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request.set_cookie(expires) requires a UTC datetime but doesn't document this #254

Closed
dobesv opened this issue Jun 3, 2016 · 2 comments
Closed

Comments

@dobesv
Copy link

@dobesv dobesv commented Jun 3, 2016

Currently Response.set_cookie will accept a datetime instance to specify the time at which the cookie should expire.

However, this datetime instance is expected to be a utc datetime, for example datetime.utcnow() + timedelta(seconds=3600), not a local datetime like datetime.now() + timedelta(seconds=3600).

Also, the provided datetime instance must NOT be timezone-aware or you will get an error. For example passing datetime.utcnow(tz=pytz.utc) + timedelta(seconds=3600).

If callers are not aware of these requirements, their application might fail unexpectedly if the app moves between time zones.

@bertjwregeer
Copy link
Member

@bertjwregeer bertjwregeer commented Jun 3, 2016

expires on the Response.set_cookie is something I am going to deprecate. Setting a max_age automatically calculates the expiration correctly, and the API that is used to actually make the cookie no longer supports an expiration parameter.

Things that need to get fixed for now:

  • If date time is timezone aware, convert it to UTC, without TZ
  • Change date time from local to UTC

WebOb 1.9:

  • Drop expires from the API entirely.
@bertjwregeer bertjwregeer modified the milestones: 1.7.0, 1.6.2 Jun 3, 2016
bertjwregeer added a commit that referenced this issue Nov 18, 2016
This will convert the TZ aware datetime to one that is not timezone
aware and to UTC. This can then be used in math to generate the
timedelta for max_age.

Also documents that using a local datetime object is not supported,
unfortunately there is no good way to find out whether a datetime is
local or not, so there is no way to warn the user about bad input.

Closes #254
@bertjwregeer
Copy link
Member

@bertjwregeer bertjwregeer commented Nov 18, 2016

There is no way to determine if a datetime is a local one or not. It's documented as long accepting a UTC datetime.

At this point, timezone aware datetime's are converted to UTC, and those work correctly now.

bertjwregeer added a commit that referenced this issue Nov 18, 2016
This will convert the TZ aware datetime to one that is not timezone
aware and to UTC. This can then be used in math to generate the
timedelta for max_age.

Also documents that using a local datetime object is not supported,
unfortunately there is no good way to find out whether a datetime is
local or not, so there is no way to warn the user about bad input.

Closes #254
bertjwregeer added a commit that referenced this issue Nov 18, 2016
This will convert the TZ aware datetime to one that is not timezone
aware and to UTC. This can then be used in math to generate the
timedelta for max_age.

Also documents that using a local datetime object is not supported,
unfortunately there is no good way to find out whether a datetime is
local or not, so there is no way to warn the user about bad input.

Closes #254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants