Skip to content

ext/curl: set curl post size using CURLOPT_POSTFIELDSIZE_LARGE - #22842

Open
Sjord wants to merge 5 commits into
php:masterfrom
Sjord:curl-large-post
Open

ext/curl: set curl post size using CURLOPT_POSTFIELDSIZE_LARGE#22842
Sjord wants to merge 5 commits into
php:masterfrom
Sjord:curl-large-post

Conversation

@Sjord

@Sjord Sjord commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This is a 64-bit number on all platforms. This improves support of posting files larger than 2GB.

Comment thread ext/curl/interface.c Outdated
@Sjord
Sjord marked this pull request as ready for review July 21, 2026 15:14
@Sjord
Sjord requested a review from adoy as a code owner July 21, 2026 15:14
Comment thread ext/curl/interface.c
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));

@devnexen devnexen Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be worth trying to make a test out of it (with an appropriate SKIPIF section)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@Sjord

Sjord commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@Ayesh do you think it is worth it to create a test for this that posts >2GB?

@Sjord Sjord changed the title Set curl post size using CURLOPT_POSTFIELDSIZE_LARGE ext/curl: set curl post size using CURLOPT_POSTFIELDSIZE_LARGE Jul 30, 2026
@Girgias

Girgias commented Aug 7, 2026

Copy link
Copy Markdown
Member

I have created 2GB+ tests for ext/tidy, so yes I think it make sense to have one.

@Sjord

Sjord commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

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.

The test (curl_post_large_string) posts data slightly bigger than fits
in a signed 32-bit number. It uses Caddy and not server.inc, because PHP
has limits on maximum upload size. It doesn't actually check whether it
uploads the correct number of bytes, but it does check the
Content-Length header, which is sufficient indication whether curl
understands the correct size.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants