Skip to content

ext/curl: Fix curl_reset() and CURLOPT_POSTFIELDS keeping old request data - #25000

Open
binaryfire wants to merge 3 commits into
php:PHP-8.4from
binaryfire:fix/curl-reset-references
Open

binaryfire wants to merge 3 commits into
php:PHP-8.4from
binaryfire:fix/curl-reset-references

Conversation

@binaryfire

@binaryfire binaryfire commented Oct 28, 2026 •

Copy link
Copy Markdown

I ran into this while working on Swoole's copy of ext/curl, which we keep in line with php-src.

The description below was written by Claude at my request, and I've reviewed it.

curl_reset() resets the write, header and read handlers but doesn't release their callbacks. It also keeps the CURLOPT_PRIVATE value and the CURLOPT_POSTFIELDS array, so all of these stay referenced until the handle is freed:

  • Callback objects and closures, and anything they capture, stay alive while a reset handle is pooled for reuse.
  • curl_getinfo($ch, CURLINFO_PRIVATE) still returns the old value after a reset.
  • curl_copy_handle() and clone of a reset handle rebuild the old file post fields.

Guzzle's CurlFactory works around the first one by setting each callback to null before it calls curl_reset() on a handle it pools. Its code comment says the callbacks "can hold onto references and are not cleaned up by curl_reset".

Separately, when a CURLOPT_POSTFIELDS array containing a CURLFile or CURLStringFile is replaced with another value, the old array isn't released. The original handle sends the new body, but curl_copy_handle() and clone rebuild the old multipart body on the copy.

The same array is also kept when CURLOPT_HTTPGET, CURLOPT_NOBODY, CURLOPT_POST, CURLOPT_UPLOAD or CURLOPT_PUT changes the request method afterwards. For example, after CURLOPT_HTTPGET the original handle sends a GET, but the copy goes back to a multipart POST with the old file.

This PR releases the three callbacks, the private value and the post fields array in curl_reset(). It also drops the stored array when CURLOPT_POSTFIELDS is set again, or when one of those method options is set.

A destructor that calls curl_setopt() for the same option while curl_reset() is releasing it can still cause a use-after-free. That's already possible for the progress, xferinfo and other callbacks released there, so I've left it to the general destructor work in #20001.

curl_reset() reset the write, header and read handlers but kept their
callbacks, and it kept the CURLOPT_PRIVATE value and the CURLOPT_POSTFIELDS
array. All of them stayed referenced until the handle was freed,
CURLINFO_PRIVATE still returned the old value, and copies of a reset handle
rebuilt the old file post fields.
Setting CURLOPT_POSTFIELDS again kept a previous array containing CURLFile
or CURLStringFile objects, so curl_copy_handle() and clone rebuilt the old
multipart body on the copy while the original handle sent the new value.
Switching the request method with CURLOPT_HTTPGET, CURLOPT_NOBODY,
CURLOPT_POST, CURLOPT_UPLOAD or CURLOPT_PUT kept a CURLOPT_POSTFIELDS array
containing CURLFile or CURLStringFile objects, so curl_copy_handle() and
clone rebuilt the old multipart POST on the copy while the original handle
used the new method.
@anun333

anun333 commented Oct 29, 2026

Copy link
Copy Markdown

Quoted below: written with Claude, an LLM, as CONTRIBUTING.md asks us to mark. The builds and test runs behind it are on my machine; the commands to repeat them are under the fold.

Tested on today's PHP-8.4 (c7ea9b17582) with this applied, debug builds:

PHP-8.4 this PR
the three new tests FAIL PASS
ext/curl/tests 150 passed, 3 failed (the new ones), 17 skipped 153 passed, 0 failed, 17 skipped
a write callback capturing an object, released when the handle is freed at curl_reset()
curl_getinfo($ch, CURLINFO_PRIVATE) after curl_reset() 'request 1' false

So a reset handle no longer keeps what the request before it set, which is what curl_easy_reset() promises for the handle itself.

How this was tested

Linux x86_64, gcc 13.3, libcurl 8.5.0, debug builds of origin/PHP-8.4 and of it with this PR's commit cherry-picked:

./buildconf --force
./configure --enable-debug --disable-cgi --disable-phpdbg --disable-fileinfo --enable-pcntl --enable-zend-test --enable-fpm --with-curl --enable-mbstring --disable-mbregex
make -j4

The callback check: curl_setopt($ch, CURLOPT_WRITEFUNCTION, function ($c, $d) use ($o) { return strlen($d); }) with $o an object whose __destruct() echoes, CURLOPT_PRIVATE set to 'request 1', then unset($o), curl_reset($ch), curl_getinfo($ch, CURLINFO_PRIVATE), unset($ch). Tests: TEST_PHP_EXECUTABLE=$PWD/sapi/cli/php sapi/cli/php run-tests.php -n -q -j4 ext/curl/tests (the new tests copied into the PHP-8.4 tree for its run).

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.

2 participants