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.
(cherry picked from commit 2d56500826d54fca445de2c0170384763b1cfdb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/http_ana.c b/src/http_ana.c
index 0a1af6d..096d78d 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -331,8 +331,6 @@
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;
@@ -847,8 +845,6 @@
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;
@@ -887,10 +883,6 @@
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))
@@ -1249,8 +1241,6 @@
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))
@@ -1395,7 +1385,6 @@
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));
@@ -1424,7 +1413,6 @@
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;
@@ -1448,7 +1436,6 @@
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));
@@ -1483,7 +1470,6 @@
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;
@@ -1782,9 +1768,6 @@
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;
@@ -1795,8 +1778,6 @@
* 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 */
@@ -2366,8 +2347,6 @@
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",
@@ -2592,7 +2571,6 @@
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);
@@ -4614,6 +4592,13 @@
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);