HTTP header date formats

Hm. HTTP/1.1 does specify the date format, and it's pretty rigid. It is not RFC1123, but only a subset that hardcodes "GMT" as the timezone identifier.

RFC850 being completely obsolete, let's not go back to that.

The current definition of DATE_RFC1132 is "D, d M Y H:i:s O". Replacing it by "D, d M Y H:i:s T" should do the trick.

That, or gm_date('D, d M Y H:i:s').' GMT', which is what Symfony currently does.

Ok, this is present in Drupal 8 also, so it needs to be fixed there first.

relevant 11 year old post on php.net http://php.net/gmdate#25031

@0x534B41 I think it would be a great test bed with D8 to test this out and then hopefully backport to D7!

REDbot complains about Drupal.org too.
https://redbot.org/?uri=https%3A%2F%2Fdrupal.org

The Last-Modified header's value isn't a valid date.

HTTP/1.1 200 OK
    Server: nginx/1.6.0
    Date: Wed, 28 May 2014 19:25:39 GMT
    Content-Type: text/html; charset=utf-8
    Transfer-Encoding: chunked
    Connection: keep-alive
    Vary: Accept-Encoding
    X-Drupal-Cache: HIT
    Content-Language: en
    X-Generator: Drupal 7 (http://drupal.org)
    Link: <https://drupal.org/>;
        rel="shortlink",<https://drupal.org/>; rel="canonical"
    Cache-Control: public, max-age=0
    Last-Modified: Wed, 28 May 2014 19:18:17 +0000
    Expires: Sun, 19 Nov 1978 05:00:00 GMT
    Vary: Cookie,Accept-Encoding
    X-Varnish: 1611016738
    Age: 0
    Via: 1.1 varnish
    X-Cache-Svr: www6.drupal.org
    X-Cache: MISS
    Front-End-Https: on
    Content-Encoding: gzip

First pass for 8.x

Added in a the other occurrences of RFC1123. At least, I assume all of them should be replaced

Ouch, that was embarrassing. Let's try that again with the brackets in the right places.

Changing all the dates is probably appropriate. Relevant documentation from the RFC in the summary.

a fixed-length subset of that defined by RFC 1123 [8] (an update to RFC 822 [9]) ... representing HTTP-date values in header fields.

Helper constant on DateTimePlus seems a handy way to handle this in D8 so it is easier to reuse this. While making the modifications I noticed a couple calls to RFC822 dates. These are, for the purposes of this issue, the same as RFC1123 so likely wrong as well.

Woops, there was a stray character in bootstrap.inc

Now I'm embarrassed. Missing use statement.

Sorry for the noise guys

W00T! Works, Redbot test passed!

That's more elegant, thanks necimdul.

+++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
@@ -31,6 +31,13 @@ class DateTimePlus extends \DateTime {
+   * A RFC2616 compliant subset of RFC1123.
+   *
+   * Example: Sun, 06 Nov 1994 08:49:37 GMT
+   */
+  const RFC2616 = 'D, d M Y H:i:s \G\M\T';
+
+  /**

My only concern is the naming of this constant. It seems like it should be more specific. Like DATE_RFC2616.

Thoughts?

I named it similar to the php.net date constants http://us3.php.net/manual/en/class.datetime.php#datetime.constants.types.

re-roll. going to be bold and just push this back to rtbc.

Namespace conflict. No changes still.

Considering updating the RFC. http://tools.ietf.org/html/rfc7231#section-7.1.1.1 Format is the same but the documentation is more up to date. Thoughts?

8: 1918820-8.patch queued for re-testing.

11: 1918820-11.patch queued for re-testing.

Yes @ 30.

Been a bug so long a new RFC came out describing it. Time to get this committed ;).

Totes.

automated re-roll.

almost automated re-roll. Namespace conflict in toolbar.module. Still no changes.

Patch in #40 is for another issue :)

Bah, lazy patch naming bit me clearly. Well here's a re-roll so we're sure its up to date.

Committed 5dd3776 and pushed to 8.x. Thanks!

Hope to set the ball rolling on the D7 backport.

Could someone please test this and mark RTBC?

#46 looks good to me. After applying, the Last Modified header is in the correct format.

I checked for other uses of DATE_RFC1123 and couldn't find any others in core that aren't fixed by the patch. I did note, however, that the admin_menu module outputs a header using DATE_RFC1123. So maybe this will need a change record so contrib maintainers will be aware of the change. I don't know if that's appropriate or necessary.

Anyway, #46 seems RTBC to me.

Committed to 7.x - thanks!

Looks like this also makes Drupal more internally consistent, since the "Expires" header was already in this format.

As for a change notice, I'm not sure it necessarily needs one given that this is a relatively minor change - though it certainly wouldn't hurt and it would be a way to inform contrib modules about this. For now, I did put it in CHANGELOG.txt and the release notes.

This broke Varnish page caching, see #2381839: Changed date format for Last-Modified header breaks caching for Varnish/Nginx configs.