[HttpClient] Stream request body in HttplugClient and Psr18Client #58856
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
|
(please rebase, not merge) |
|
Sorry, should be fixed Any idea what's up with the tests? I'd say I'm reasonably confident I'm not responsible for all the failures here... |
|
Note that this means the content-length won't be sent anymore and the chunked encoding will be used instead. |
|
The Content-Length header should now be added if |
|
Not sure if we'd want to do something about the duplicate code in Psr18Client and HttplugClient |
|
Thank you @KurtThiemann. |
I think that the documentation should say |
…ct HTTP method in CurlHttpClient (KurtThiemann) This PR was submitted for the 7.2 branch but it was squashed and merged into the 6.4 branch instead. Discussion ---------- [HttpClient] Always set CURLOPT_CUSTOMREQUEST to the correct HTTP method in CurlHttpClient | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix #59043 | License | MIT Currently, when sending a `HEAD` request and using a closure as the request body using `CurlHttpClient`, a `PUT` request instead of a HEAD request will be sent. This is because `CURLOPT_NOBODY` is set to make a `HEAD` request, but is overwritten by `CURLOPT_UPLOAD`, which is necessary to make curl call `CURLOPT_READFUNCTION`, but also sets the HTTP method to `PUT`. The solution I am suggesting is to just always set `CURLOPT_CUSTOMREQUEST`, so the correct HTTP method will be used, no matter what other options are added. This is mainly an issue in the PSR-18 wrapper, since PSR requests always have a request body stream, even if the body is empty. Since #58856, `Psr18Client` always passes a closure as the request body to the underlying HTTP client instead of just reading the entire request body stream into a string, as that made it impossible to upload large files. While it would be possible to add an exception to `Psr18Client` to only add the request body if the method used commonly has a body, I think the better solution is to ensure that `CurlHttpClient` always actually sends the type of request that it is asked to send. Commits ------- bfdf5d9 [HttpClient] Always set CURLOPT_CUSTOMREQUEST to the correct HTTP method in CurlHttpClient
Currently,
Psr18ClientandHttplugClientwill simply callgetContents()on the stream, loading the entire stream content into a string before passing it to the underlyingHttpClient. When sending a request with a large body, this can lead to the PHP process running out of memory.This pull request changes both clients to instead pass a Closure that reads from the request body stream to the underlying
HttpClient.The HttpClient documentation states that "each time, the closure should return a string smaller than the amount requested as argument", which would mean that if the closure was called requesting a size of 10 bytes, you are only allowed to return at most 9 bytes. I assume that is an inaccuracy in the docs, but please correct me if I'm wrong here.
I'm also not fully sure whether this would be considered a new feature or a bug fix.