-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix the bug where clone curl does not copy POST fields. #16556
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR!
I think this also affects older PHP branches; should likely target PHP-8.2.
ext/curl/interface.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the actual problem that we don't clone the ->postfields in the first place? I.e. ZVAL_COPY(&clone_ch->postfields, &ch->postfields).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code should behave consistently with PHP_FUNCTION(curl_copy_handle), with the only difference being the return value. This PR primarily aims to fix this bug, and a subsequent PR for code optimization can be submitted to implement a reusable function.
Line 1598 in 5d10a33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you clone an object, you should clone all its properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have modified the code to use ZVAL_COPY for postfields and updated PHP_FUNCTION(curl_copy_handle) to ensure consistency with the clone operation.
5d10a33 to
a0dd445
Compare
Updated
No description provided.