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

fixed a bug where max-age=0 cookies would not expires until next second #969

Merged
merged 1 commit into from Jun 12, 2016

Conversation

jamadam
Copy link
Contributor

@jamadam jamadam commented Jun 12, 2016

They say set-cookie with Max-Age=0 means it expires immidiately but current Mojo::UserAgent doesn't handle it that way.

I found it in real world use case. PHP applications sometimes send cookies as follows for session renewal or something.

HTTP/1.1 302 Found
Content-Type: text/html; charset=UTF-8
Server: nginx
X-Powered-By: PHP/5.6.16
Vary: Accept-Encoding
Date: Sun, 12 Jun 2016 13:33:55 GMT
Set-Cookie: PHPSESSIDADMIN=deleted; expires=Thu, 01-Jan-1970 00:00:01 GMT; Max-Age=0; path=/
Set-Cookie: PHPSESSIDADMIN=qkm88lcmdt1bmls1ngb3e57ci3; expires=Sun, 12-Jun-2016 19:33:54 GMT; Max-Age=21600; path=/admin; HttpOnly
Location: http://example.com/admin/

Recieving the response the ua unexpectedly sends back the Max-Age=0 cookie again within the same second in favor of redirection.

I checked RFC6265

5.2.2.  The Max-Age Attribute

If delta-seconds is less than or equal to zero (0), let expiry-time
be the earliest representable date and time.  Otherwise, let the
expiry-time be the current date and time plus delta-seconds seconds.

If I'm getting it right, "earliest representable date and time" should be epoch 0.
Since negative values are suggested to be ignored earlier, delta-seconds can't be less than zero.

@kraih kraih merged commit 8a810cd into mojolicious:master Jun 12, 2016
@kraih
Copy link
Member

kraih commented Jun 12, 2016

Thanks, applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants