sapi/cli: support Expect 100-continue in PHP dev server - #23245
Conversation
When posting large payloads, curl checks whether the server is ready for the body. It sends an `Expect: 100-continue` header and expects `HTTP/1.1 100 Continue` as the response before sending the body. The PHP development server did not support this, causing a timeout in curl. This made such requests take one second longer. - https://everything.curl.dev/http/post/expect100.html - php#23242
This is not a curl test, but a test of the behavior of the PHP development server. It should thus not be in the curl directory but in the sapi/cli directory.
1b0cb8d to
0052e12
Compare
|
@php/release-managers-86 @mbeccati Is this something you want in PHP 8.6? |
|
@Sjord Seems like a useful fix to me. I will discuss with the team. EDIT: we agree it's perfectly fine for 8.6 |
|
@iluuu1994 @Girgias Could you review this please? |
|
ok will try my best to review, I have some questions ... |
| append_http_status_line(&buffer, client->parser.http_major * 100 + client->parser.http_minor, 100, 0); | ||
| smart_str_appendl(&buffer, "\r\n", 2); | ||
| smart_str_0(&buffer); | ||
| php_cli_server_client_send_through(client, ZSTR_VAL(buffer.s), ZSTR_LEN(buffer.s)); |
There was a problem hiding this comment.
Is the dev server meant to exit when the peer aborts here? If I m not mistaken, php_cli_server_client_send_through() call is reached from a place where a failed send() is fatal to the whole process, right ?
There was a problem hiding this comment.
When reading the code I come to the same conclusion, but I cannot reproduce it. php_cli_server_client_send_through is also used in sapi_cli_server_send_headers, so I thought that this would be the appropriate function to use.
There was a problem hiding this comment.
right right ... note that sapi_cli_server_send_headers is wrapped in zend_try (and is a sapi handler) ; this is where I would like Gina opinion.
There was a problem hiding this comment.
I got a bit further with this.
- The test now sets the SO_LINGER option on the socket (if possible), which makes the
sendinphp_cli_server_client_send_throughfail. This indeed made the server exit. - In the calling code
php_cli_server_client_send_throughis now wrapped within zend_try to handle the error.
So this seems solved, but I am also a little bit out of my depth here so it would be nice to get another set of eyes on this.
If php_cli_server_client_send_through fails while sending `HTTP/1.1 100 Continue` we don't want the server to exit.
3 participants
When posting large payloads, curl checks whether the server is ready for the body. It sends an
Expect: 100-continueheader and expectsHTTP/1.1 100 Continueas the response before sending the body. The PHP development server did not support this, causing a timeout in curl. This made such requests take one second longer.