From 3a78aa6e95693c9102248a70ecc21f29a7489f5d Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Wed, 27 Feb 2019 16:19:48 +0100 Subject: [PATCH] BUG/MINOR: stats: Fully consume large requests in the stats applet In the stats applet (in HTX and legacy HTTP), after a response is fully sent to a client, the request is consumed. It is done at the end, after all the response was copied into the channel's buffer. But only outgoing data at time the applet is called are consumed. Then the applet is closed. If a request with a huge body is sent, an error is triggerred because a SHUTW is catched for an unfinisehd request. Now, we consume request data until the end. In fact, we don't try to shutdown the request's channel for write anymore. This patch must be backported to 1.9 after some observation period. It should probably be backported in prior versions too. But honnestly, with refactoring on the connection layer and the stream interface in 1.9, it is probably safer to not do so. --- include/types/stats.h | 4 ++- src/stats.c | 58 +++++++++++++++++++++---------------------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/include/types/stats.h b/include/types/stats.h index f416af1de..043c37747 100644 --- a/include/types/stats.h +++ b/include/types/stats.h @@ -38,11 +38,13 @@ /* HTTP stats : applet.st0 */ enum { - STAT_HTTP_DONE = 0, /* finished */ + STAT_HTTP_INIT = 0, /* Initial state */ STAT_HTTP_HEAD, /* send headers before dump */ STAT_HTTP_DUMP, /* dumping stats */ STAT_HTTP_POST, /* waiting post data */ STAT_HTTP_LAST, /* sending last chunk of response */ + STAT_HTTP_DONE, /* dump is finished */ + STAT_HTTP_END, /* finished */ }; /* status codes available for the stats admin page */ diff --git a/src/stats.c b/src/stats.c index 5e32ffc19..965fe4299 100644 --- a/src/stats.c +++ b/src/stats.c @@ -3312,8 +3312,8 @@ static void htx_stats_io_handler(struct appctx *appctx) } /* check that the output is not closed */ - if (res->flags & (CF_SHUTW|CF_SHUTW_NOW)) - appctx->st0 = STAT_HTTP_DONE; + if (res->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_SHUTR)) + appctx->st0 = STAT_HTTP_END; /* all states are processed in sequence */ if (appctx->st0 == STAT_HTTP_HEAD) { @@ -3333,7 +3333,7 @@ static void htx_stats_io_handler(struct appctx *appctx) if (appctx->st0 == STAT_HTTP_POST) { if (stats_process_http_post(si)) appctx->st0 = STAT_HTTP_LAST; - else if (si_oc(si)->flags & CF_SHUTR) + else if (req->flags & CF_SHUTR) appctx->st0 = STAT_HTTP_DONE; } @@ -3348,25 +3348,24 @@ static void htx_stats_io_handler(struct appctx *appctx) si_rx_room_blk(si); goto out; } + channel_add_input(&s->res, 1); + appctx->st0 = STAT_HTTP_END; + } + + if (appctx->st0 == STAT_HTTP_END) { + if (!(res->flags & CF_SHUTR)) { + res->flags |= CF_READ_NULL; + si_shutr(si); + } /* eat the whole request */ - req_htx = htxbuf(&req->buf); - htx_reset(req_htx); - htx_to_buf(req_htx, &req->buf); - co_set_data(req, 0); - res->flags |= CF_READ_NULL; - si_shutr(si); - } - - if ((res->flags & CF_SHUTR) && (si->state == SI_ST_EST)) - si_shutw(si); - - if (appctx->st0 == STAT_HTTP_DONE) { - if ((req->flags & CF_SHUTW) && (si->state == SI_ST_EST)) { - si_shutr(si); - res->flags |= CF_READ_NULL; + if (co_data(req)) { + req_htx = htx_from_buf(&req->buf); + co_htx_skip(req, req_htx, co_data(req)); + htx_to_buf(req_htx, &req->buf); } } + out: /* we have left the request in the buffer for the case where we * process a POST, and this automatically re-enables activity on @@ -3408,8 +3407,8 @@ static void http_stats_io_handler(struct appctx *appctx) } /* check that the output is not closed */ - if (res->flags & (CF_SHUTW|CF_SHUTW_NOW)) - appctx->st0 = STAT_HTTP_DONE; + if (res->flags & (CF_SHUTW|CF_SHUTW_NOW|CF_SHUTR)) + appctx->st0 = STAT_HTTP_END; /* all states are processed in sequence */ if (appctx->st0 == STAT_HTTP_HEAD) { @@ -3493,21 +3492,20 @@ static void http_stats_io_handler(struct appctx *appctx) goto out; } } - /* eat the whole request */ - co_skip(si_oc(si), co_data(si_oc(si))); - res->flags |= CF_READ_NULL; - si_shutr(si); + appctx->st0 = STAT_HTTP_END; } - if ((res->flags & CF_SHUTR) && (si->state == SI_ST_EST)) - si_shutw(si); - - if (appctx->st0 == STAT_HTTP_DONE) { - if ((req->flags & CF_SHUTW) && (si->state == SI_ST_EST)) { - si_shutr(si); + if (appctx->st0 == STAT_HTTP_END) { + if (!(res->flags & CF_SHUTR)) { res->flags |= CF_READ_NULL; + si_shutr(si); } + + /* eat the whole request */ + if (co_data(req)) + co_skip(si_oc(si), co_data(si_oc(si))); } + out: /* we have left the request in the buffer for the case where we * process a POST, and this automatically re-enables activity on