pressflow / 6

Each version of Pressflow is API-compatible with the same major Drupal version. For example, Pressflow 6 is compatible with all Drupal 6 modules. Pressflow 6 also integrates the SimpleTest system from Drupal 7 and the CDN support patch.
http://pressflow.org/
GNU General Public License v2.0
234 stars 89 forks source link

Fix RFC1123 formating in Last-Modified header #80

Closed neclimdul closed 4 years ago

neclimdul commented 10 years ago

Fixes #78

pwolanin commented 10 years ago

So PHP core has it wrong? Is there a bug report there?

pwolanin commented 10 years ago

looks like the d.o issue sugests: gm_date('D, d M Y H:i:s').' GMT', which is what Symfony currently does.

https://drupal.org/node/1918820

neclimdul commented 10 years ago

Yeah, this has the GMT inside the gm_date call and escaped rather then concatenated. Same code.

neclimdul commented 10 years ago

And re:php being wrong, sort of. RFC 2616 specifies dates be specified in a subset of the RFC1123 standard. The date must be in GMT and specifically with GMT as the timezone identifier. http://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.3

achton commented 9 years ago

+1 for this cleanup. This has been fixed in D7 since Nov 2014.

pwolanin commented 9 years ago

Drupal 6 core has e.g.

  header("Last-Modified: ". gmdate("D, d M Y H:i:s") ." GMT");

Though I agree the formatting doesn't really matter.

I'm also curious why this is using REQUEST_TIME - that's just an optimization to avoid calling out to time()?

achton commented 9 years ago

I guess the $_SERVER['REQUEST_TIME'] part is another backport from D7, although it's really (int) $_SERVER['REQUEST_TIME']. No idea if PHP is really calling time() if you omit the 2nd argument, or if it has access to some internal pointer which directly gives you the time() value. I guess there could be slight difference between the value of time() and REQUEST_TIME as well. In any case, it can't make a huge difference, methinks.

neclimdul commented 9 years ago

Probably. We got strict about this in Drupal 7 and I assume David was following that here 586e6d9 when he added it.

neclimdul commented 4 years ago

I can't see a reason for this to still be open :)