BUG/MINOR: cache: do not eat the request while waiting for a buffer

The cache applet's I/O handler leaves through the "exit" label when it
cannot allocate its input or output buffer, when one of these buffers is
still pending or when the output buffer is full. But "exit" eats the whole
request with b_reset(&appctx->inbuf), and the request can still be needed on
the next call. For instance, in the HTX_CACHE_HEADER state, the handler
reads the request start line to know whether it is a HEAD request.

The input buffer is empty at this point, and appctx_release_buffers() also
frees it while it has no data, so http_get_stline() may return NULL and the
process crashes. This requires the output buffer allocation to fail exactly
after the request was received, which is rare since the buffers reserve
covers most transient failures, hence a latent bug.

Add a dedicated "wait" label for these paths: it flushes the pending HTX
message to the output buffer and returns without touching the request.  The
"exit" label is kept for the states where the request must be consumed:
EOS/error, fast-forward blocked by the consumer, and the end of the
response.

This was introduced when the applet got its own buffers by commit 863417292
("MAJOR: cache: Update HTTP cache applet to handle its own buffers") in
3.0-dev3, so the fix should be backported to 3.0.

A debug patch forcing a single output buffer allocation failure on the
first call in the HTX_CACHE_HEADER state reproduced the crash with ASAN
on the htx_sl_req_meth() line; with this fix, the GET and HEAD requests
all succeed and the process stays alive.

diff --git a/src/cache.c b/src/cache.c
index 779bbce2bcd97e8999cdeaa62184c26bdb433636..2ab066a83510da117de8b43e1ee4e5647596e2f2 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -2259,7 +2259,7 @@ static void http_cache_io_handler(struct appctx *appctx)
        size_t ret;
 
        if (applet_fl_test(appctx, APPCTX_FL_INBLK_ALLOC|APPCTX_FL_OUTBLK_ALLOC|APPCTX_FL_OUTBLK_FULL))
-               goto exit;
+               goto wait;
 
        if (applet_fl_test(appctx, APPCTX_FL_FASTFWD) && se_fl_test(appctx->sedesc, SE_FL_MAY_FASTFWD_PROD))
                goto exit;
@@ -2276,7 +2276,7 @@ static void http_cache_io_handler(struct appctx *appctx)
        }
 
        if (!appctx_get_buf(appctx, &appctx->outbuf)) {
-               goto exit;
+               goto wait;
        }
 
        if (unlikely(applet_fl_test(appctx, APPCTX_FL_EOS|APPCTX_FL_ERROR))) {
@@ -2290,7 +2290,7 @@ static void http_cache_io_handler(struct appctx *appctx)
                struct ist meth;
 
                if (unlikely(applet_fl_test(appctx, APPCTX_FL_INBLK_ALLOC))) {
-                       goto exit;
+                       goto wait;
                }
 
                /* Headers must be dump at once. Otherwise it is an error */
@@ -2360,6 +2360,13 @@ static void http_cache_io_handler(struct appctx *appctx)
        appctx->sedesc->iobuf.flags &= ~IOBUF_FL_FF_BLOCKED;
        return;
 
+  wait:
+       /* The output buffer is not available yet. Leave the request in place:
+        * its start line is read to know whether it is a HEAD request. */
+       if (res_htx)
+               htx_to_buf(res_htx, &appctx->outbuf);
+       return;
+
   wait_request:
        /* Wait for the request before starting to deliver the response */
        applet_need_more_data(appctx);