Skip to content
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

Force closing keep-alive connections on old cURL #211

Merged
merged 1 commit into from
May 13, 2016

Conversation

rmccue
Copy link
Collaborator

@rmccue rmccue commented May 13, 2016

cURL <7.22 seems to hold the request open if the connection is
keep-alive and the buffer hasn't been filled. Changing to single
connections is slightly less efficient (although unnoticable in most
uses), but should fix this issue.

If you need keep-alive, set the Connection header manually.

cURL <7.22 seems to hold the request open if the connection is
keep-alive and the buffer hasn't been filled. Changing to single
connections is slightly less efficient (although unnoticable in most
uses), but should fix this issue.

If you need keep-alive, set the Connection header manually.
@rmccue rmccue added this to the 1.7 milestone May 13, 2016
@codecov-io
Copy link

Current coverage is 92.25%

Merging #211 into master will increase coverage by +<.01%

@@             master       #211   diff @@
==========================================
  Files            21         21          
  Lines          1766       1768     +2   
  Methods         156        156          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1629       1631     +2   
  Misses          137        137          
  Partials          0          0          

Powered by Codecov. Last updated by 8b6a253...fd34acb

@rmccue rmccue merged commit 95518ce into master May 13, 2016
@rmccue rmccue deleted the force-close-on-old-curl branch May 13, 2016 11:11
@schlessera schlessera mentioned this pull request Nov 9, 2023
13 tasks
@schlessera
Copy link
Member

@rmccue Do you remember any additional context here around the specific version number you've identified?

I'm asking because actively making use of that specific version for conditional code ended up breaking a lot of things. See #838 & https://core.trac.wordpress.org/ticket/59842

@rmccue
Copy link
Collaborator Author

rmccue commented Nov 9, 2023

@schlessera Don't recall the context off the top of my head.

I checked the cURL changelog for 7.22 and not seeing anything super relevant around keepalives, best I can see is:

when sending a request and an error is received before the (entire) request body is sent, stop sending the request and close the connection after having received the entire response. This is equally true if an Expect: 100-continue header was used.
fixed several memory leaks in OOM situations
bad expire(0) caused multi_socket API to hang

Looks like 7.21.7 did have this though:

Force connection close for HTTP 200 OK when time condition matched

Other issues I can see that could be relevant are jonashaag/bjoern#51 and this Stack Overflow question which links https://stackoverflow.com/questions/10285700/curl-error-recv-failure-connection-reset-by-peer-php-curl

Sounds like in any case based on #838, keepalive may be more generally still be broken, either with cURL in PHP or with w.org.

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