Skip to content

setcookie has an obsolete expires date format #9200

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

Closed
GuySartorelli opened this issue Jul 30, 2022 · 10 comments
Closed

setcookie has an obsolete expires date format #9200

GuySartorelli opened this issue Jul 30, 2022 · 10 comments

Comments

@GuySartorelli
Copy link

Description

setcookie() gives an expires date format that is non-standard as per MDN and HTTP RFC 2616 - the latter of which explicitly refers to the format PHP's setcookie() outputs as "obsolete".

The following code:

<?php
setcookie('expires-format-test', 'test', strtotime('now + 1 minute'));

Resulted in this header output:
Set-Cookie | expires-format-test=test; expires=Sat, 30-Jul-2022 05:12:54 GMT; Max-Age=60

But I expected this header output instead:
Set-Cookie | expires-format-test=test; expires=Sat, 30 Jul 2022 05:12:54 GMT; Max-Age=60

PHP Version

PHP 8.0.21

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Jul 30, 2022

The recently approved RFC 9110 states:

A recipient that parses a timestamp value in an HTTP field MUST accept all three HTTP-date formats. When a sender generates a field that contains one or more timestamps defined as HTTP-date, the sender MUST generate those timestamps in the IMF-fixdate format.

So while PHP does not comply to the RFC, it shouldn't cause issues. And given that the RFC 850 format is obsolete for a long time, I don't see the urgent need to fix this; i.e. I'd leave it for PHP 8.0 and 8.1 for BC reasons.

@GuySartorelli
Copy link
Author

GuySartorelli commented Jul 30, 2022

If the format doesn't cause problems regardless of the format then I don't see how BC comes into it, but by the same token if it doesn't cause problems then it's fine to leave as is.

It would be nice if PHP used the "preferred" format as stated in the RFC but it seems unlikely the obsolete format will cause any problems for many years to come.

@cmb69
Copy link
Member

cmb69 commented Jul 30, 2022

If the format doesn't cause problems regardless of the format then I don't see how BC comes into it

I was mostly thinking about test expectations (at least some PHPTs in php-src would be affected).

@TimWolla
Copy link
Member

I don't see the urgent need to fix this;

I agree here, this is not urgent to fix. The date parsing algorithm is explicitly defined in RFC 6265 (“Cookie RFC”), section 5.1.1 and it supports this legacy format (and various other questionable formats).

i.e. I'd leave it for PHP 8.0 and 8.1 for BC reasons.

I agree with that, this is not something to backport into existing releases.

I'd like to see this fixed for PHP 8.2, though. Anything compatible with 2011 RFC 6265 will understand both formats and we should not try to attempt compatibility with more than a decade old systems if we're technically in violation of the spec.

@TimWolla
Copy link
Member

This also affects ext/date's DATE_FORMAT_COOKIE.

@cmb69
Copy link
Member

cmb69 commented Jul 30, 2022

And also the session cookie. Grepping for D, d-M-Y H:i:s T finds these occurrences (there should be only one); DATE_FORMAT_COOKIE uses l, d-M-Y H:i:s T, though. I'm not sure that we should change DATE_FORMAT_COOKIE; maybe it's better to introduce a new constant for this.

@TimWolla
Copy link
Member

DATE_FORMAT_COOKIE uses l, d-M-Y H:i:s T

Ugh, indeed, full day-of-week name instead of abbreviation.

The correct format is already available as DATE_RFC7231. It might be sense to simply refer to that constant in all the other locations. The explicit \G\M\T is required, T is incorrect.

@GuySartorelli
Copy link
Author

GuySartorelli commented Jul 31, 2022

Should everywhere that uses (or as we're discussing, should use) the DATE_RFC7231 format use that constant?

@derickr
Copy link
Member

derickr commented Aug 10, 2022

Nobody has reported that the current format causes any problems, and until somebody does, I am not in favour of changing this.

@cmb69
Copy link
Member

cmb69 commented Aug 10, 2022

The date format we're talking about has been introduced by RFC 850, which has been obsoleted by RFC 1036 in 1987! Furthermore, the format 30-Jul-2022 makes no sense; nobody would write it that way. While I still don't see an urgent need to switch to a non obsolete format, I wouldn't want to keep it forever.

derickr added a commit to derickr/php-src that referenced this issue Aug 11, 2022
TimWolla added a commit to TimWolla/php-src that referenced this issue Aug 11, 2022
TimWolla added a commit that referenced this issue Aug 12, 2022
* Update expires format for session cookie

see GH-9200
see 15e3fcb

* Add ext/session/tests/gh9200.phpt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants