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.

(cherry picked from commit 25b0625d5c27a209c38dcac6a81c58495e5360af)
[ad: context adjustement due to missing HTTPCLIENT_O_RES_HTX in 3.2]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit eadb777452f8470ddaeb0ffdb59c130ce1621823)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit fff555a50b229a32341931e7928d19fc7b023789)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/include/haproxy/http_client-t.h b/include/haproxy/http_client-t.h
index 350a301..5b5be55 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 3a239a8..027dc09 100644 (file)
--- a/src/http_client.c
+++ b/src/http_client.c
@@ -403,13 +403,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)
@@ -425,13 +429,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;
 }
@@ -751,8 +755,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;
 
@@ -765,6 +772,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);
 
@@ -811,17 +829,17 @@ 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 */
                                }
                                break;
 
                        case HTTPCLIENT_S_RES_STLINE:
-                               /* 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;
@@ -1124,7 +1142,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: