Skip to content

Update to unmodified curl 7.64.1#12

Closed
cmb69 wants to merge 1 commit intowinlibs:masterfrom
cmb69:curl-7.64.1
Closed

Update to unmodified curl 7.64.1#12
cmb69 wants to merge 1 commit intowinlibs:masterfrom
cmb69:curl-7.64.1

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Mar 27, 2019

No description provided.

@Jan-E
Copy link
Contributor

Jan-E commented Apr 30, 2019

@weltling @cmb69
After merging this PR, the test script for bug 76675 fails. Is that the reason, why this PR is not merged yet?

The test script does not segfault (as was the original bug), but fails anyway. That might be because the script uses pipelining:
https://github.com/php/php-src/blob/master/ext/curl/tests/bug76675.phpt#L26

Pipelining has been removed from Curl now: curl/curl@2f44e94

Long story, short: the test script is obsolete in the next version of Curl. In fact, maybe the bug itself was caused by pipelining of which @bagder said that

the pipelining code had a few rather nasty known issues

Quote from curl/curl@2f44e94#commitcomment-33220187

Would not it be better to ignore bug 76675 and remove it from the PHP curl code? See my comment in the bug report as well: https://bugs.php.net/bug.php?id=76675

@weltling
Copy link

@Jan-E thanks for the ping. There is no plan to upgrade to this cURL version. Probably we should abandon this PR. If the upcoming versions of cURL don't support pipelining, it might need additional fixes on the PHP side, lets see. But the test should be kept for those versions that support pipelining.

Thansk.

@cmb69
Copy link
Member Author

cmb69 commented May 3, 2019

This test does not use HTTP/1 pipelining (i.e. CURLPIPE_HTTP1), but rather HTTP/2 multiplexing (i.e. CURLPIPE_MULTIPLEX), which is not deprecated. See https://curl.haxx.se/libcurl/c/CURLMOPT_PIPELINING.html.

Anyhow, since we're not planning to update to 7.64.1, I'm closing this PR.

@cmb69 cmb69 closed this May 3, 2019
@cmb69 cmb69 deleted the curl-7.64.1 branch May 3, 2019 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants