pipeline: fix mistakenly trying to pipeline POSTs
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 Closes #1483 Reported-by: stootill at github
This commit is contained in:
parent
bc3866e3eb
commit
4cdb1be824
50
lib/url.c
50
lib/url.c
@ -3109,12 +3109,16 @@ static bool SocketIsDead(curl_socket_t sock)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* IsPipeliningPossible() returns TRUE if the options set would allow
|
* IsPipeliningPossible()
|
||||||
* pipelining/multiplexing and the connection is using a HTTP protocol.
|
*
|
||||||
|
* Return a bitmask with the available pipelining and multiplexing options for
|
||||||
|
* the given requested connection.
|
||||||
*/
|
*/
|
||||||
static bool IsPipeliningPossible(const struct Curl_easy *handle,
|
static int IsPipeliningPossible(const struct Curl_easy *handle,
|
||||||
const struct connectdata *conn)
|
const struct connectdata *conn)
|
||||||
{
|
{
|
||||||
|
int avail = 0;
|
||||||
|
|
||||||
/* If a HTTP protocol and pipelining is enabled */
|
/* If a HTTP protocol and pipelining is enabled */
|
||||||
if((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
|
if((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
|
||||||
(!conn->bits.protoconnstart || !conn->bits.close)) {
|
(!conn->bits.protoconnstart || !conn->bits.close)) {
|
||||||
@ -3124,14 +3128,14 @@ static bool IsPipeliningPossible(const struct Curl_easy *handle,
|
|||||||
(handle->set.httpreq == HTTPREQ_GET ||
|
(handle->set.httpreq == HTTPREQ_GET ||
|
||||||
handle->set.httpreq == HTTPREQ_HEAD))
|
handle->set.httpreq == HTTPREQ_HEAD))
|
||||||
/* didn't ask for HTTP/1.0 and a GET or HEAD */
|
/* didn't ask for HTTP/1.0 and a GET or HEAD */
|
||||||
return TRUE;
|
avail |= CURLPIPE_HTTP1;
|
||||||
|
|
||||||
if(Curl_pipeline_wanted(handle->multi, CURLPIPE_MULTIPLEX) &&
|
if(Curl_pipeline_wanted(handle->multi, CURLPIPE_MULTIPLEX) &&
|
||||||
(handle->set.httpversion >= CURL_HTTP_VERSION_2))
|
(handle->set.httpversion >= CURL_HTTP_VERSION_2))
|
||||||
/* allows HTTP/2 */
|
/* allows HTTP/2 */
|
||||||
return TRUE;
|
avail |= CURLPIPE_MULTIPLEX;
|
||||||
}
|
}
|
||||||
return FALSE;
|
return avail;
|
||||||
}
|
}
|
||||||
|
|
||||||
int Curl_removeHandleFromPipeline(struct Curl_easy *handle,
|
int Curl_removeHandleFromPipeline(struct Curl_easy *handle,
|
||||||
@ -3418,7 +3422,7 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
struct connectdata *check;
|
struct connectdata *check;
|
||||||
struct connectdata *chosen = 0;
|
struct connectdata *chosen = 0;
|
||||||
bool foundPendingCandidate = FALSE;
|
bool foundPendingCandidate = FALSE;
|
||||||
bool canPipeline = IsPipeliningPossible(data, needle);
|
int pipe = IsPipeliningPossible(data, needle);
|
||||||
struct connectbundle *bundle;
|
struct connectbundle *bundle;
|
||||||
|
|
||||||
#ifdef USE_NTLM
|
#ifdef USE_NTLM
|
||||||
@ -3434,10 +3438,9 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
*force_reuse = FALSE;
|
*force_reuse = FALSE;
|
||||||
*waitpipe = FALSE;
|
*waitpipe = FALSE;
|
||||||
|
|
||||||
/* We can't pipe if the site is blacklisted */
|
/* We can't pipeline if the site is blacklisted */
|
||||||
if(canPipeline && Curl_pipeline_site_blacklisted(data, needle)) {
|
if((pipe & CURLPIPE_HTTP1) && Curl_pipeline_site_blacklisted(data, needle))
|
||||||
canPipeline = FALSE;
|
pipe &= ~ CURLPIPE_HTTP1;
|
||||||
}
|
|
||||||
|
|
||||||
/* Look up the bundle with all the connections to this
|
/* Look up the bundle with all the connections to this
|
||||||
particular host */
|
particular host */
|
||||||
@ -3457,8 +3460,8 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
(bundle->multiuse == BUNDLE_MULTIPLEX ?
|
(bundle->multiuse == BUNDLE_MULTIPLEX ?
|
||||||
"can multiplex" : "serially")));
|
"can multiplex" : "serially")));
|
||||||
|
|
||||||
/* We can't pipe if we don't know anything about the server */
|
/* We can't pipeline if we don't know anything about the server */
|
||||||
if(canPipeline) {
|
if(pipe) {
|
||||||
if(bundle->multiuse <= BUNDLE_UNKNOWN) {
|
if(bundle->multiuse <= BUNDLE_UNKNOWN) {
|
||||||
if((bundle->multiuse == BUNDLE_UNKNOWN) && data->set.pipewait) {
|
if((bundle->multiuse == BUNDLE_UNKNOWN) && data->set.pipewait) {
|
||||||
infof(data, "Server doesn't support multi-use yet, wait\n");
|
infof(data, "Server doesn't support multi-use yet, wait\n");
|
||||||
@ -3467,18 +3470,18 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
}
|
}
|
||||||
|
|
||||||
infof(data, "Server doesn't support multi-use (yet)\n");
|
infof(data, "Server doesn't support multi-use (yet)\n");
|
||||||
canPipeline = FALSE;
|
pipe = 0;
|
||||||
}
|
}
|
||||||
if((bundle->multiuse == BUNDLE_PIPELINING) &&
|
if((bundle->multiuse == BUNDLE_PIPELINING) &&
|
||||||
!Curl_pipeline_wanted(data->multi, CURLPIPE_HTTP1)) {
|
!Curl_pipeline_wanted(data->multi, CURLPIPE_HTTP1)) {
|
||||||
/* not asked for, switch off */
|
/* not asked for, switch off */
|
||||||
infof(data, "Could pipeline, but not asked to!\n");
|
infof(data, "Could pipeline, but not asked to!\n");
|
||||||
canPipeline = FALSE;
|
pipe = 0;
|
||||||
}
|
}
|
||||||
else if((bundle->multiuse == BUNDLE_MULTIPLEX) &&
|
else if((bundle->multiuse == BUNDLE_MULTIPLEX) &&
|
||||||
!Curl_pipeline_wanted(data->multi, CURLPIPE_MULTIPLEX)) {
|
!Curl_pipeline_wanted(data->multi, CURLPIPE_MULTIPLEX)) {
|
||||||
infof(data, "Could multiplex, but not asked to!\n");
|
infof(data, "Could multiplex, but not asked to!\n");
|
||||||
canPipeline = FALSE;
|
pipe = 0;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3499,20 +3502,21 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
|
|
||||||
pipeLen = check->send_pipe.size + check->recv_pipe.size;
|
pipeLen = check->send_pipe.size + check->recv_pipe.size;
|
||||||
|
|
||||||
if(canPipeline) {
|
if(pipe) {
|
||||||
if(check->bits.protoconnstart && check->bits.close)
|
if(check->bits.protoconnstart && check->bits.close)
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
if(!check->bits.multiplex) {
|
if(!check->bits.multiplex) {
|
||||||
/* If not multiplexing, make sure the pipe has only GET requests */
|
/* If not multiplexing, make sure the connection is fine for HTTP/1
|
||||||
|
pipelining */
|
||||||
struct Curl_easy* sh = gethandleathead(&check->send_pipe);
|
struct Curl_easy* sh = gethandleathead(&check->send_pipe);
|
||||||
struct Curl_easy* rh = gethandleathead(&check->recv_pipe);
|
struct Curl_easy* rh = gethandleathead(&check->recv_pipe);
|
||||||
if(sh) {
|
if(sh) {
|
||||||
if(!IsPipeliningPossible(sh, check))
|
if(!(IsPipeliningPossible(sh, check) & CURLPIPE_HTTP1))
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
else if(rh) {
|
else if(rh) {
|
||||||
if(!IsPipeliningPossible(rh, check))
|
if(!(IsPipeliningPossible(rh, check) & CURLPIPE_HTTP1))
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -3620,7 +3624,7 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if(!canPipeline && check->inuse)
|
if(!pipe && check->inuse)
|
||||||
/* this request can't be pipelined but the checked connection is
|
/* this request can't be pipelined but the checked connection is
|
||||||
already in use so we skip it */
|
already in use so we skip it */
|
||||||
continue;
|
continue;
|
||||||
@ -3751,7 +3755,7 @@ ConnectionExists(struct Curl_easy *data,
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
if(canPipeline) {
|
if(pipe) {
|
||||||
/* We can pipeline if we want to. Let's continue looking for
|
/* We can pipeline if we want to. Let's continue looking for
|
||||||
the optimal connection to use, i.e the shortest pipe that is not
|
the optimal connection to use, i.e the shortest pipe that is not
|
||||||
blacklisted. */
|
blacklisted. */
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user