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

HTTP pipelining causes encrypted transfers to hang #1387

Closed
ahatala opened this issue Apr 4, 2017 · 3 comments
Closed

HTTP pipelining causes encrypted transfers to hang #1387

ahatala opened this issue Apr 4, 2017 · 3 comments

Comments

@ahatala
Copy link

ahatala commented Apr 4, 2017

When doing a large number of HTTPS requests and using HTTP/1.1 pipelining CURL occasionally enters a state where multi_perform is not able to advance any of the requests. I haven't verified with other configurations, but this is true for Windows using the schannel implementation.

The cause for this seems to be that Curl_readwrite() checks the read socket state to see if there's incoming data but fails to account for data already buffered by the schannel system.

In transfer.c:1099 the check is:

if((k->keepon & KEEP_RECV) &&
     ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound)) {

The code checks if the stream was rewound, due to more data being read then was necessary to fulfill a previous request BUT if fails to account for other locations where incoming data might be buffered - including schannel.

Changing that line to

  if((k->keepon & KEEP_RECV) &&
     ((select_res & CURL_CSELECT_IN) || conn->bits.stream_was_rewound || Curl_ssl_data_pending(conn, 0))) {

fixes the issue, but may not be the correct way of going about it.

In connect.c there is a function that accounts for the possible places that may have buffered data:

/* Data received can be cached at various levels, so check them all here. */
bool Curl_conn_data_pending(struct connectdata *conn, int sockindex)
{
  int readable;

  if(Curl_ssl_data_pending(conn, sockindex) ||
     Curl_recv_has_postponed_data(conn, sockindex))
    return true;

  readable = SOCKET_READABLE(conn->sock[sockindex], 0);
  return (readable > 0 && (readable & CURL_CSELECT_IN));
}

Tested with current github master (commit 55f4aba).

@jay
Copy link
Member

jay commented Apr 6, 2017

If you can reproduce this can you try the branch in #1392 with your same workaround, and see if that workaround still works. Thanks

@ahatala
Copy link
Author

ahatala commented Apr 7, 2017

I don't have an easy repro that triggers 100% of the time (when I get one I'll share it). I've tested #1392 for a while and I can say with pretty high confidence that it works ok with my workaround, and indeed probably makes things better by early-outing in the case that there is buffered encrypted data but not enough of it.

jay added a commit that referenced this issue Apr 23, 2017
- Track when the cached encrypted data contains only a partial record
  that can't be decrypted without more data (SEC_E_INCOMPLETE_MESSAGE).

- Change Curl_schannel_data_pending to return false in such a case.

Other SSL libraries have pending data functions that behave similarly.

Ref: #1387

Closes #1392
@stale
Copy link

stale bot commented Oct 4, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 4, 2017
@stale stale bot closed this as completed Oct 18, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants