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

Curl_easy_perform returns CURLE_OK when HTTP/1.1 connection reuse fails #2801

Closed
djelinski opened this Issue Jul 27, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@djelinski
Contributor

djelinski commented Jul 27, 2018

I did this

I'm uploading files to HTTP/1.1 endpoint over SSL using lubcurl. I'm reusing the same easy handle to allow for connection reuse. Every now and then I'm getting CURLE_OK without accompanying HTTP response. Debug logs show the following:

We are completely uploaded and fine
TLSv1.2 (IN), TLS alert, Client hello (1):
Connection died, retrying a fresh connect
necessary data rewind wasn't possible
Closing connection 17
TLSv1.2 (OUT), TLS alert, Client hello (1):

I expected the following

curl_easy_perform should return CURLE_SEND_FAIL_REWIND

curl/libcurl version

[curl -V output]
curl 7.60.0 (i386-pc-win32) libcurl/7.60.0 OpenSSL/1.1.0h (WinSSL) zlib/1.2.11 brotli/1.0.4 WinIDN libssh2/1.8.0 nghttp2/1.32.0
Release-Date: 2018-05-16
Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp
Features: AsynchDNS IDN IPv6 Largefile SSPI Kerberos SPNEGO NTLM SSL libz brotli TLS-SRP HTTP2 HTTPS-proxy MultiSSL

operating system

Windows 2008R2

@bagder bagder added the HTTP label Jul 27, 2018

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Jul 27, 2018

Member

Can you please help us understand how to reproduce this? I've read the code a few times now and I can't see how it can log that message and not return an error... can you?

Member

bagder commented Jul 27, 2018

Can you please help us understand how to reproduce this? I've read the code a few times now and I can't see how it can log that message and not return an error... can you?

@djelinski

This comment has been minimized.

Show comment
Hide comment
@djelinski

djelinski Jul 28, 2018

Contributor

This is a tricky one to reproduce in lab, but happens regularly in practice; the other end needs to close the connection after the initial check for dead connection but before the upload completes.
I'm reading the code now and I think this may happen when we enter this:

curl/lib/multi.c

Line 1944 in acefdd0

if(done || (result == CURLE_RECV_ERROR)) {

with done=true and result=CURLE_OK. I'll verify this on Monday.

Contributor

djelinski commented Jul 28, 2018

This is a tricky one to reproduce in lab, but happens regularly in practice; the other end needs to close the connection after the initial check for dead connection but before the upload completes.
I'm reading the code now and I think this may happen when we enter this:

curl/lib/multi.c

Line 1944 in acefdd0

if(done || (result == CURLE_RECV_ERROR)) {

with done=true and result=CURLE_OK. I'll verify this on Monday.

@djelinski

This comment has been minimized.

Show comment
Hide comment
@djelinski

djelinski Jul 30, 2018

Contributor

Gist of code used to reproduce:

CURL *curl;
curl = curl_easy_init();
curl_easy_setopt(curl, CURLOPT_URL, "https://s3.amazonaws.com");
curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
curl_easy_perform(curl);
Sleep(5950); // experimental delay that usually results in the server closing connection while we upload
struct curl_slist *headers = NULL;
headers = curl_slist_append( headers, "Expect:");
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, 1L);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, read_callback_dummy );
CURLcode result;
result = curl_easy_perform(curl);
printf("%d",(int)result); // result: 0

With read callback:

size_t read_callback_dummy(void *ptr, size_t size, size_t nmemb, void *stream)
{
static int i = 1;
if(i) {i=0;return 1;}
return 0;
}

Contributor

djelinski commented Jul 30, 2018

Gist of code used to reproduce:

CURL *curl;
curl = curl_easy_init();
curl_easy_setopt(curl, CURLOPT_URL, "https://s3.amazonaws.com");
curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
curl_easy_perform(curl);
Sleep(5950); // experimental delay that usually results in the server closing connection while we upload
struct curl_slist *headers = NULL;
headers = curl_slist_append( headers, "Expect:");
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, headers);
curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
curl_easy_setopt(curl, CURLOPT_INFILESIZE, 1L);
curl_easy_setopt(curl, CURLOPT_READFUNCTION, read_callback_dummy );
CURLcode result;
result = curl_easy_perform(curl);
printf("%d",(int)result); // result: 0

With read callback:

size_t read_callback_dummy(void *ptr, size_t size, size_t nmemb, void *stream)
{
static int i = 1;
if(i) {i=0;return 1;}
return 0;
}

@djelinski

This comment has been minimized.

Show comment
Hide comment
@djelinski

djelinski Jul 30, 2018

Contributor

Also my comment above was apparently right; the following patch gives me CURLE_SEND_FAIL_REWIND:
djelinski@dfa8181

I don't understand the code enough to know if this is the right place for this fix or not; let me know if I should file a pull request.

Contributor

djelinski commented Jul 30, 2018

Also my comment above was apparently right; the following patch gives me CURLE_SEND_FAIL_REWIND:
djelinski@dfa8181

I don't understand the code enough to know if this is the right place for this fix or not; let me know if I should file a pull request.

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder Jul 30, 2018

Member

Thanks! I think it looks like a good fix. Please submit a PR for it!

Member

bagder commented Jul 30, 2018

Thanks! I think it looks like a good fix. Please submit a PR for it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment