BUG/MEDIUM: http-client: Drain the request if an early response is received

When a large request is sent, it is possible to have a response before the
end of the request. It is valid from HTTP perspective but it is an issue
with the current design of the http-client. Indded, the request and the
response are handled sequentially. So the response will be blocked, waiting
for the end of the request. Most of time, it is not an issue, except when
the request transfer is blocked. In that case, the applet is blocked.

With the current API, it is not possible to handle early response and
continue the request transfer. So, this case cannot be handle. In that case,
it seems reasonnable to drain the request if a response is received. This
way, the request transfer, from the caller point of view, is never blocked
and the response can be properly processed.

To do so, the action flag HTTPCLIENT_FA_DRAIN_REQ is added to the
http-client. When it is set, the request payload is just dropped. In that
case, we take care to not report the end of input to properly report the
request was truncated, especially in logs.

It is only an issue with large POSTs, when the payload is streamed.

This patch must be backported as far as 2.6.

diff --git a/include/haproxy/http_client-t.h b/include/haproxy/http_client-t.h
index f60dee0b5538134a2a8af8ed8e208b74db40da6b..ef75a336d1d6ab286e59fc4a46da14315d53b7f6 100644 (file)
--- a/include/haproxy/http_client-t.h
+++ b/include/haproxy/http_client-t.h
@@ -43,6 +43,7 @@ struct httpclient {
 /* Action (FA) to do */
 #define    HTTPCLIENT_FA_STOP         0x00000001   /* stops the httpclient at the next IO handler call */
 #define    HTTPCLIENT_FA_AUTOKILL     0x00000002   /* sets the applet to destroy the httpclient struct itself */
+#define    HTTPCLIENT_FA_DRAIN_REQ    0x00000004   /* drains the request */
 
 /* status (FS) */
 #define    HTTPCLIENT_FS_STARTED      0x00010000 /* the httpclient was started */

diff --git a/src/http_client.c b/src/http_client.c
index cd41d359d0198adda4942ee745a08318773707cf..0adfb1d22fa60768cc8cf69efe700419fcc1303d 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -205,13 +205,17 @@ int httpclient_req_xfer(struct httpclient *hc, struct ist src, int end)
        int ret = 0;
        struct htx *htx;
 
+       if (hc->flags & HTTPCLIENT_FA_DRAIN_REQ) {
+               ret = istlen(src);
+               goto end;
+       }
+
        if (!b_alloc(&hc->req.buf, DB_CHANNEL))
-               goto error;
+               goto end;
 
        htx = htx_from_buf(&hc->req.buf);
        if (!htx)
-               goto error;
-
+               goto end;
        ret += htx_add_data(htx, src);
 
        if (ret && hc->appctx)
@@ -227,13 +231,13 @@ int httpclient_req_xfer(struct httpclient *hc, struct ist src, int end)
                 */
                if (htx_is_empty(htx)) {
                        if (!htx_add_endof(htx, HTX_BLK_EOT))
-                               goto error;
+                               goto end;
                }
                htx->flags |= HTX_FL_EOM;
        }
        htx_to_buf(htx, &hc->req.buf);
 
-error:
+  end:
 
        return ret;
 }
@@ -558,8 +562,11 @@ void httpclient_applet_io_handler(struct appctx *appctx)
 
                                channel_add_input(req, htx->data);
 
-                               if (htx->flags & HTX_FL_EOM) /* check if a body need to be added */
+                               if (htx->flags & HTX_FL_EOM) { /* check if a body need to be added */
                                        appctx->st0 = HTTPCLIENT_S_RES_STLINE;
+                                       se_fl_set(appctx->sedesc, SE_FL_EOI);
+                                       break;
+                               }
                                else
                                        appctx->st0 = HTTPCLIENT_S_REQ_BODY;
 
@@ -572,6 +579,17 @@ void httpclient_applet_io_handler(struct appctx *appctx)
                                        if (hc->ops.req_payload) {
                                                struct htx *hc_htx;
 
+                                               if (co_data(res)) {
+                                                       /* A response was received but we are still process the request.
+                                                        * It is unexpected and not really supported with the current API.
+                                                        * So lets drain the request to avoid any issue.
+                                                        */
+                                                       b_reset(&hc->req.buf);
+                                                       hc->flags |= HTTPCLIENT_FA_DRAIN_REQ;
+                                                       appctx->st0 = HTTPCLIENT_S_RES_STLINE;
+                                                       break;
+                                               }
+
                                                /* call the request callback */
                                                hc->ops.req_payload(hc);
 
@@ -618,8 +636,11 @@ void httpclient_applet_io_handler(struct appctx *appctx)
                                        htx = htxbuf(&req->buf);
 
                                        /* if the request contains the HTX_FL_EOM, we finished the request part. */
-                                       if (htx->flags & HTX_FL_EOM)
+                                       if (htx->flags & HTX_FL_EOM) {
                                                appctx->st0 = HTTPCLIENT_S_RES_STLINE;
+                                               se_fl_set(appctx->sedesc, SE_FL_EOI);
+                                               break;
+                                       }
 
                                        goto process_data; /* we need to leave the IO handler once we wrote the request */
                                }
@@ -633,9 +654,6 @@ void httpclient_applet_io_handler(struct appctx *appctx)
                                        goto out;
                                }
 
-                               /* Request is finished, report EOI */
-                               se_fl_set(appctx->sedesc, SE_FL_EOI);
-
                                /* copy the start line in the hc structure,then remove the htx block */
                                if (!co_data(res))
                                        goto out;
@@ -978,7 +996,12 @@ int httpclient_applet_init(struct appctx *appctx)
        /* The request was transferred when the stream was created. So switch
         * directly to REQ_BODY or RES_STLINE state
         */
-       appctx->st0 = (hc->ops.req_payload ? HTTPCLIENT_S_REQ_BODY : HTTPCLIENT_S_RES_STLINE);
+       if (hc->ops.req_payload)
+               appctx->st0 = HTTPCLIENT_S_REQ_BODY;
+       else {
+               appctx->st0 =  HTTPCLIENT_S_RES_STLINE;
+               se_fl_set(appctx->sedesc, SE_FL_EOI);
+       }
        return 0;
 
  out_free_addr: