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:
Daniel Stenberg 2017-05-12 16:29:06 +02:00
parent bc3866e3eb
commit 4cdb1be824

View File

@ -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. */