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

Jump to bottom

CURLE_UNSUPPORTED_PROTOCOL if curl_multi_remove_handle(multi, easy) called in the middle and easy is reused. #7018

Closed
sergio-nsk opened this issue May 6, 2021 · 5 comments
Closed

CURLE_UNSUPPORTED_PROTOCOL if curl_multi_remove_handle(multi, easy) called in the middle and easy is reused. #7018

sergio-nsk opened this issue May 6, 2021 · 5 comments
Assignees
@bagder
Labels
HTTP

Comments

@sergio-nsk
Copy link
Contributor

sergio-nsk commented May 6, 2021 •
edited

Download a big enough data using the http protocol.

  1. curl_multi_add_handle() -> curl_multi_perform() in a loop -> write_callback() is being raised as expected.
  2. curl_easy is not yet done (no CURLMSG_DONE message), curl_multi_remove_handle() is called. According to the manual:

    You can remove handles at any point in time during transfers.

  3. a) curl_multi_add_handle() -> curl_multi_perform() in a loop -> got CURLMSG_DONE message with the error CURLE_UNSUPPORTED_PROTOCOL.
    b) Or curl_easy_perform() returns the error CURLE_UNSUPPORTED_PROTOCOL.

I expected the following

Once removed from the multi handle, you can again use other easy interface functions like curl_easy_perform on the handle or whatever you think is necessary.

Curl_dyn_reset(&data->state.headerb); is not called after curl_multi_add_handle() and data->state.headerb points to the end header mark "\r\n" left after the first request. Thus, curl tries to parse "\r\nHTTP/1.1 200 OK\r\n" when the response on the second request is being processed.

curl/libcurl version

libcurl v7.74.0

operating system

Any
@bagder bagder added the HTTP label May 6, 2021
@bagder bagder self-assigned this May 6, 2021
bagder added a commit that referenced this issue May 6, 2021
@bagder
http: reset the header buffer when sending the request
f340a08 
A reused transfer handle could otherwise reuse the previous buffer's and
havoc could ensue.

Reported-by: sergio-nsk on github
Fixes #7018
@bagder bagder mentioned this issue May 6, 2021
Closed
@bagder
Copy link
Member

bagder commented May 6, 2021

Does #7021 perhaps fix the issue for you?

@sergio-nsk
Copy link
Contributor Author

sergio-nsk commented May 6, 2021 •
edited

Yes, that pull request fixes the issue.
I am not sure if this is a complete fix. I am afraid the issue can persist in case of using a proxy. It seems garbage "\r\nHTTP/1.1 200 Connection Established\r\n" is still possible. I can not test the fix with using a proxy right now.

@bagder
Copy link
Member

bagder commented May 6, 2021 •
edited

I am afraid the issue can persist in case of using a proxy

Why do you think that? A proxy won't change how the code gets headers and this patch resets the header buffer before the first one can arrive so no, I can't see how this won't fix the proxy case as well!

@bagder bagder closed this as completed in 04cc274 May 6, 2021
@sergio-nsk
Copy link
Contributor Author

sergio-nsk commented May 6, 2021 •
edited

I looked at multi.c and supposed the issue might be in a proxy stuff also:
protocol_connect() -> conn->handler->connect_it() -> Curl_proxy_connect() is called before multi_do() -> conn->handler->do_it() -> Curl_http() -> Curl_dyn_reset(&data->state.headerb);.

You are right, Curl_proxy_connect() does not use data->state.headerb, it uses s->rcvbuf with proxy headers.

@bagder
Copy link
Member

bagder commented May 6, 2021

and that's only for CONNECT requests, not other ones done over proxy. They use the same code path as non-proxies.

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

@bagder bagder

Labels
HTTP
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http: reset the header buffer when sending the request curl/curl
2 participants
@bagder @sergio-nsk