ext/curl: set curl post size using CURLOPT_POSTFIELDSIZE_LARGE - #22842
ext/curl: set curl post size using CURLOPT_POSTFIELDSIZE_LARGE#22842Sjord wants to merge 4 commits into
Conversation
This is a 64-bit number on all platforms. This improves support of posting files larger than 2GB. - https://curl.se/libcurl/c/CURLOPT_POSTFIELDSIZE.html - https://curl.se/libcurl/c/CURLOPT_POSTFIELDSIZE_LARGE.html
| zend_string *str = zval_get_tmp_string(zvalue, &tmp_str); | ||
| /* with curl 7.17.0 and later, we can use COPYPOSTFIELDS, but we have to provide size before */ | ||
| error = curl_easy_setopt(ch->cp, CURLOPT_POSTFIELDSIZE, ZSTR_LEN(str)); | ||
| error = curl_easy_setopt(ch->cp, CURLOPT_POSTFIELDSIZE_LARGE, (curl_off_t) ZSTR_LEN(str)); |
There was a problem hiding this comment.
might be worth trying to make a test out of it (with an appropriate SKIPIF section)
There was a problem hiding this comment.
This would require to post a payload larger than 2 GB which takes a lot of time and memory, right? I am not sure I understand what you mean.
There was a problem hiding this comment.
if you ever wrote such test you would at least put RUN_RESOURCE_HEAVY_TEST check under the SKIPIF condition but I ll let the maintainers decide on it as for me the C part looks good regardless. cc @Ayesh
|
@Ayesh do you think it is worth it to create a test for this that posts >2GB? |
|
I have created 2GB+ tests for ext/tidy, so yes I think it make sense to have one. |
|
I've added a test. It would be nice if it could also run on Windows (as that is the one platform that would profit from this), but the test currently relies on fork. I'll look into this some more later. |
This is a 64-bit number on all platforms. This improves support of posting files larger than 2GB.