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

Enabling CURLPIPE_HTTP1 and CURLPIPE_MULTIPLEX, and requesting HTTP2 causes POST requests to get pipelined #1481

Closed
stootill opened this issue May 12, 2017 · 3 comments
Labels

Comments

@stootill
Copy link

I have setup libcurl to enable HTTP2, and am configuring multi to enable pipelining and multiplexing as follows:-

curl_multi_setopt(_curlMultiHandle, CURLMOPT_PIPELINING, CURLPIPE_HTTP1 | CURLPIPE_MULTIPLEX);

On each request I am requesting HTTP2 as follows:-

curl_easy_setopt(_curlHandle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2_0);

When I run this in my app which is using a mixture of HTTP/1.1 and HTTP/2 endpoints, I see POST requests failing on calls to HTTP/1.1 endpoints. I've dug into the code and found that it seems that IsPipeliningPossible is ignoring the request method if the request asked for HTTP/2 upgrading and multiplexing is enabled. IsPipeliningPossible only checks the HTTP version that was asked for in the request, not the version that was negotiated in the connection when testing for multiplex support, so it seems that pipelining is wrongly being enabled for requests that do not have GET or HEAD methods on HTTP/1.1 channels.

I have fixed this locally by applying the following change:-

`
@@ -2902,7 +2902,8 @@ static bool IsPipeliningPossible(const struct Curl_easy *handle,
return TRUE;

 if(Curl_pipeline_wanted(handle->multi, CURLPIPE_MULTIPLEX) &&
  •   (handle->set.httpversion >= CURL_HTTP_VERSION_2))
    
  •   (handle->set.httpversion >= CURL_HTTP_VERSION_2) &&
    
  •   (conn->httpversion == 20))
     /* allows HTTP/2 */
     return TRUE;
    
    }
    `

but I've found other bug reports (#584) that suggest that this caused regressions elsewhere (https://curl.haxx.se/mail/lib-2016-01/0031.html) so am not sure how to proceed.

Thanks!

@bagder bagder added the HTTP label May 12, 2017
bagder added a commit that referenced this issue May 12, 2017
The function IsPipeliningPossible() would return TRUE if either
pipelining OR HTTP/2 were possible on a connection, which would lead to
it returning TRUE even for POSTs on HTTP/1 connections.

It now returns a bitmask so that the caller can differentiate which kind
the connection allows.

Fixes #1481
Reported-by: stootill at github
@bagder
Copy link
Member

bagder commented May 12, 2017

See #1483 for my attempted fix for this issue. Would be great if you could try it out!

@stootill
Copy link
Author

stootill commented May 12, 2017 via email

@bagder
Copy link
Member

bagder commented May 12, 2017

Excellent!

@bagder bagder closed this as completed in 4cdb1be May 12, 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.
Labels
Development

No branches or pull requests

2 participants