Fix #79019: Copied cURL handles upload empty file #5045
Conversation
|
Good question! I've just sent a mail to the curl-library list. Let's see. |
|
After some helpful discussion on the curl mailing list, I came up with a more comprehensive solution which is supposed to work with It has to be noted that this solution uses a hack to keep BC for PHP-7.4; after merging into master that hack should be removed. Also, it is important to point out that the current solution uses an anonymous union and struct, so it is easier to port to a better solution in master. Anonymous unions and struct actually require C11 (or an appropriate subset thereof), but I think that we're using anonymous unions/struct already in PHP-7.4. Please correct me if I'm wrong. |
As suggested by Nikita[1]. [1] <#5045 (comment)>
To cater to `curl_copy_handle()` of cURL handles with attached `CURLFile`s, we must not attach the opened stream, because the stream may not be seekable, so that we could rewind, when the same stream is going to be uploaded multiple times. Instead, we're opening the stream lazily in the read or seek callback. Since `curl_multi_perfom()` processes easy handles asynchronously, we have no control of the operation sequence. Since duplicated cURL handles may be used with multi handles, we cannot use a single arg structure, but actually have to rebuild the whole mime structure on handle duplication and attach this to the new handle. This requires to store the zval that has been originally provided for `CURLOPT_POSTFIELDS`. Ideally, we would store it in an additional member of `php_curl`, but since that would constitute a BC break, we work around by actually storing a union in `to_free->stream`, and we retrieve the appropriate zval by sequentially searching the list. In order to better test this behavior, we extend the test responder to print the size of the upload, and patch the existing tests accordingly.
|
If there are no objections, I'll merge this PR in a week (and will re-classify bug 79013 as doc issue; IOW, stick with the chunked upload). |
|
An update regarding https://bugs.php.net: whether libcurl uses chunked transfer encoding obviously depends on the HTTP version (and maybe other factors), so I don't think it's up to us to document this. |
According to the `curl_mime_data_cb()` man page[1], the seek callback is only called when resending due to redirection. In this case the stream would already have been opened, so there is no need to try to open the stream in `seek_cb()`. [1] <https://curl.haxx.se/libcurl/c/curl_mime_data_cb.html>
cmb69
added
Waiting on Review
Waiting on Author
and removed
Waiting on Author
Waiting on Review
labels
|
Thanks! Applied as 2d0dec9. |
To cater to
curl_copy_handle()of cURL handles with attachedCURLFiles, we must not attach the opened stream, because the streammay not be seekable, so that we could rewind, when the same stream is
going to be uploaded multiple times. Instead, we're opening the stream
lazily in the read or seek callback, and close it when reading finished
(either successfully or not) and when seeking failed, respectively.
This behavior is basically the same as for libcurl < 7.56.0, where we
attach the file name, and libcurl reads and uploads the file contents.
To be able to test this behavior, we extend the test responder to print
the size of the upload, and patch the existing tests accordingly.
I have targeted PHP-7.4, although the CURLFile stream support has recently been back-ported to PHP-7.3, so this branch is affected by bug 79019 as well. However, particularly after this patch, I don't think that we could reasonably fix bug 79013, so I suggest to revert commit 17a9f14 (note that this commit has not yet been released) but keep e120273, and stick with the chunked upload for PHP-7.4+ and change bug 79013 to documentation problem.