From 2d56500826d54fca445de2c0170384763b1cfdb1 Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Fri, 10 Sep 2021 09:17:50 +0200 Subject: [PATCH] BUG/MEDIUM: http-ana: Reset channels analysers when returning an error When an error is returned to the client, via a call to http_reply_and_close(), the request channel is flushed and shut down and HTTP analysis on both direction is finished. So it is safer to centralize reset of channels analysers at this place. It is especially important when a filter is attached to the stream when a client abort is detected. Because, otherwise, the stream remains blocked because request analysers are not reset. This bug was hidden for a while. But since the fix 6fcd2d328 ("BUG/MINOR: stream: Don't release a stream if FLT_END is still registered"), it is possible to trigger it. This patch must be backported everywhere the above commit was backported. --- src/http_ana.c | 29 +++++++---------------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/src/http_ana.c b/src/http_ana.c index 2b0c690f0..3480e9620 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -336,8 +336,6 @@ int http_wait_for_request(struct stream *s, struct channel *req, int an_bit) if (!(s->flags & SF_FINST_MASK)) s->flags |= SF_FINST_R; - req->analysers &= AN_REQ_FLT_END; - req->analyse_exp = TICK_ETERNITY; DBG_TRACE_DEVEL("leaving on error", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_HTTP_ERR, s, txn); return 0; @@ -809,8 +807,6 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit) if (!(s->flags & SF_FINST_MASK)) s->flags |= SF_FINST_R; - req->analysers &= AN_REQ_FLT_END; - req->analyse_exp = TICK_ETERNITY; DBG_TRACE_DEVEL("leaving on error", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_HTTP_ERR, s, txn); return 0; @@ -849,10 +845,6 @@ int http_process_tarpit(struct stream *s, struct channel *req, int an_bit) http_reply_and_close(s, txn->status, (!(req->flags & CF_READ_ERROR) ? http_error_message(s) : NULL)); - end: - req->analysers &= AN_REQ_FLT_END; - req->analyse_exp = TICK_ETERNITY; - if (!(s->flags & SF_ERR_MASK)) s->flags |= SF_ERR_PRXCOND; if (!(s->flags & SF_FINST_MASK)) @@ -1211,8 +1203,6 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) txn->status = status; http_reply_and_close(s, txn->status, http_error_message(s)); } - req->analysers &= AN_REQ_FLT_END; - s->res.analysers &= AN_RES_FLT_END; /* we're in data phase, we want to abort both directions */ if (!(s->flags & SF_ERR_MASK)) s->flags |= SF_ERR_PRXCOND; if (!(s->flags & SF_FINST_MASK)) @@ -1357,7 +1347,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) stream_inc_http_fail_ctr(s); } - rep->analysers &= AN_RES_FLT_END; s->si[1].flags |= SI_FL_NOLINGER; http_reply_and_close(s, txn->status, http_error_message(s)); @@ -1386,7 +1375,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) health_adjust(__objt_server(s->target), HANA_STATUS_HTTP_READ_TIMEOUT); } - rep->analysers &= AN_RES_FLT_END; txn->status = 504; stream_inc_http_fail_ctr(s); s->si[1].flags |= SI_FL_NOLINGER; @@ -1410,7 +1398,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) if (objt_server(s->target)) _HA_ATOMIC_INC(&__objt_server(s->target)->counters.cli_aborts); - rep->analysers &= AN_RES_FLT_END; txn->status = 400; http_reply_and_close(s, txn->status, http_error_message(s)); @@ -1445,7 +1432,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) health_adjust(__objt_server(s->target), HANA_STATUS_HTTP_BROKEN_PIPE); } - rep->analysers &= AN_RES_FLT_END; txn->status = 502; stream_inc_http_fail_ctr(s); s->si[1].flags |= SI_FL_NOLINGER; @@ -1744,9 +1730,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) s->flags |= SF_FINST_H; s->si[1].flags |= SI_FL_NOLINGER; - rep->analysers &= AN_RES_FLT_END; - s->req.analysers &= AN_REQ_FLT_END; - rep->analyse_exp = TICK_ETERNITY; DBG_TRACE_DEVEL("leaving on error", STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_HTTP_ERR, s, txn); return 0; @@ -1757,8 +1740,6 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) * any other information so that the client retries. */ txn->status = 0; - rep->analysers &= AN_RES_FLT_END; - s->req.analysers &= AN_REQ_FLT_END; s->logs.logwait = 0; s->logs.level = 0; s->res.flags &= ~CF_EXPECT_MORE; /* speed up sending a previous response */ @@ -2328,8 +2309,6 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit return_error: /* don't send any error message as we're in the body */ http_reply_and_close(s, txn->status, NULL); - res->analysers &= AN_RES_FLT_END; - s->req.analysers &= AN_REQ_FLT_END; /* we're in data phase, we want to abort both directions */ if (!(s->flags & SF_FINST_MASK)) s->flags |= SF_FINST_D; DBG_TRACE_DEVEL("leaving on error", @@ -2563,7 +2542,6 @@ int http_apply_redirect_rule(struct redirect_rule *rule, struct stream *s, struc if (rule->flags & REDIRECT_FLAG_FROM_REQ) { /* let's log the request time */ s->logs.tv_request = now; - req->analysers &= AN_REQ_FLT_END; if (s->sess->fe == s->be) /* report it if the request was intercepted by the frontend */ _HA_ATOMIC_INC(&s->sess->fe->fe_counters.intercepted_req); @@ -4564,6 +4542,13 @@ void http_reply_and_close(struct stream *s, short status, struct http_reply *msg end: s->res.wex = tick_add_ifset(now_ms, s->res.wto); + /* At this staged, HTTP analysis is finished */ + s->req.analysers &= AN_REQ_FLT_END; + s->req.analyse_exp = TICK_ETERNITY; + + s->res.analysers &= AN_RES_FLT_END; + s->res.analyse_exp = TICK_ETERNITY; + channel_auto_read(&s->req); channel_abort(&s->req); channel_auto_close(&s->req);