Fix CLI server date format

[kelunik]

Fix the date format to be compliant with https://tools.ietf.org/html/rfc7231#section-7.1.1.2

[KalleZ]

This should be a no-brainer as the CLI webserver is only meant for testing anyway, I take it no tests depends on this output and needs to be adjusted

[petk]

+1 for following standards and standard recommendations. PHP is already too permissive up on too many places. However something more needs to be fixed here I think. With current implementation I get this:

Build: PHP 7.2.18-dev
Systems tested: Alpine 3.9 and macOS Mojave 10.14.3

./buildconf && ./configure --disable-all && make
curl -I http://localhost:8084/

HTTP/1.1 404 Not Found
Host: localhost:8084
Date: Sun GMT
Connection: close
Content-Type: text/html; charset=UTF-8
Content-Length: 533

This is intended and ok?

[kelunik]

@petk Yes, just noticed this while also fixing using GMT instead of local time. It was the length I forgot to change.

[staabm]

Just for context: this change is motivated by this tweet - which contains a example of the php-cli server misbehaving in comparison to nginx
https://twitter.com/arneblankerts/status/1109857090946957319

[krakjoe]

I'm in bed so can't review in its entirety, but the other day I noticed a bug report about static content responder crashing because it uses date functions that aren't initialized in static response ... Is this in any way going to effect that? Cheers Joe

…

On Sun, 24 Mar 2019, 21:46 Markus Staab, ***@***.***> wrote: Just for context: this change is motivated by this tweet - which contains a example of the php-cli server misbehaving in comparison to nginx https://twitter.com/arneblankerts/status/1109857090946957319 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3982 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACIe6mlXjlNZijsx1tQZDZ3FDeD3mF8Lks5vZ-QmgaJpZM4cFrLK> .

[petk]

Additionally, it would be really good to have a bugs.php.net ID for this if this is going in the PHP-7.2 so we can properly mark it in the NEWS files. Technically, this should go to PHP-7.4+ but if this is not affecting BC then ok.

[krakjoe]

So append_essential_headers is invoked by the static responder, can you test this with only static content or static content first, I'm quite sure it's going to fault still ... obviously this was an existing problem, but if you're familiar with this, maybe you could have a look at solving that problem ? (it doesn't/shouldn't block this pr)

[KalleZ]

@petk The builtin webserver is not intended for anything but testing & development and I would categorize it as a bug fix, so it should go into the lowest possible branch (probably 7.2 as noted).

I agree it should have a ticket associated for NEWS

[kelunik]

Created https://bugs.php.net/bug.php?id=77794 for reference and fixed what has been mentioned.

@krakjoe I'm not familiar with the code, just saw the tweet and searched for where the date is formatted, sorry.

[kelunik]

@petk This is a bugfix, so it should go into PHP 7.2 (lowest supported version). Without this fix, the implementation violates the RFC and that causes issues in browsers (see https://gist.github.com/theseer/dff2d65ac80df66afc0a4cca68f4030c).

[petk]

Applied via 7f98723 to PHP-7.2+. Thank you @kelunik and the reviewers...