Skip to content

Commit

Permalink
lib-http: client: Fixed pipelining bug: client sometimes sent new req…
Browse files Browse the repository at this point in the history
…uest while still waiting for 100-continue.
  • Loading branch information
stephanbosch authored and sirainen committed Feb 10, 2016
1 parent 79f8a20 commit e1d8d18
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 11 deletions.
18 changes: 10 additions & 8 deletions src/lib-http/http-client-connection.c
Expand Up @@ -369,7 +369,7 @@ http_client_connection_continue_timeout(struct http_client_connection *conn)
array_count(&conn->request_wait_list)-1);
req = req_idx[0];

conn->payload_continue = TRUE;
req->payload_sync_continue = TRUE;
if (http_client_request_send_more(req, &error) < 0) {
http_client_connection_abort_temp_error(&conn,
HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
Expand Down Expand Up @@ -401,7 +401,7 @@ int http_client_connection_next_request(struct http_client_connection *conn)
timeout_remove(&conn->to_idle);

req->conn = conn;
conn->payload_continue = FALSE;
req->payload_sync_continue = FALSE;
if (conn->peer->no_payload_sync)
req->payload_sync = FALSE;

Expand Down Expand Up @@ -742,15 +742,15 @@ static void http_client_connection_input(struct connection *_conn)
user agent MAY ignore unexpected 1xx responses.
*/
if (req->payload_sync && response.status == 100) {
if (conn->payload_continue) {
if (req->payload_sync_continue) {
http_client_connection_debug(conn,
"Got 100-continue response after timeout");
continue;
}

conn->peer->no_payload_sync = FALSE;
conn->peer->seen_100_response = TRUE;
conn->payload_continue = TRUE;
req->payload_sync_continue = TRUE;

http_client_connection_debug(conn,
"Got expected 100-continue response");
Expand Down Expand Up @@ -792,8 +792,10 @@ static void http_client_connection_input(struct connection *_conn)
timeval_diff_msecs(&req->sent_time, &req->submit_time));

/* make sure connection output is unlocked if 100-continue failed */
if (req->payload_sync && !conn->payload_continue)
conn->output_locked = FALSE;
if (req->payload_sync && !req->payload_sync_continue) {
http_client_connection_debug(conn, "Unlocked output");
conn->output_locked = FALSE;
}

/* remove request from queue */
array_delete(&conn->request_wait_list, 0, 1);
Expand Down Expand Up @@ -822,7 +824,7 @@ static void http_client_connection_input(struct connection *_conn)
blocks via http_client_request_send_payload()
and we're not waiting for 100 continue */
if (!req->payload_wait ||
(req->payload_sync && !conn->payload_continue)) {
(req->payload_sync && !req->payload_sync_continue)) {
/* failed Expect: */
if (response.status == 417 && req->payload_sync) {
/* drop Expect: continue */
Expand Down Expand Up @@ -971,7 +973,7 @@ int http_client_connection_output(struct http_client_connection *conn)
return 1;
}

if (!req->payload_sync || conn->payload_continue) {
if (!req->payload_sync || req->payload_sync_continue) {
if (http_client_request_send_more(req, &error) < 0) {
http_client_connection_abort_temp_error(&conn,
HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
Expand Down
3 changes: 1 addition & 2 deletions src/lib-http/http-client-private.h
Expand Up @@ -115,6 +115,7 @@ struct http_client_request {
unsigned int have_hdr_user_agent:1;

unsigned int payload_sync:1;
unsigned int payload_sync_continue:1;
unsigned int payload_chunked:1;
unsigned int payload_wait:1;
unsigned int urgent:1;
Expand Down Expand Up @@ -160,8 +161,6 @@ struct http_client_connection {
unsigned int close_indicated:1;
unsigned int output_locked:1; /* output is locked; no pipelining */
unsigned int output_broken:1; /* output is broken; no more requests */
unsigned int payload_continue:1; /* received 100-continue for current
request */
unsigned int in_req_callback:1; /* performin request callback (busy) */
};

Expand Down
2 changes: 1 addition & 1 deletion src/lib-http/http-client-request.c
Expand Up @@ -1239,7 +1239,7 @@ bool http_client_request_try_retry(struct http_client_request *req)
100 continue (there's no way to rewind the payload for a retry)
*/
if (req->payload_wait &&
(!req->payload_sync || req->conn->payload_continue))
(!req->payload_sync || req->payload_sync_continue))
return FALSE;
/* limit the number of attempts for each request */
if (req->attempts+1 >= req->client->set.max_attempts)
Expand Down

0 comments on commit e1d8d18

Please sign in to comment.