MINOR: http-ana: Handle HTX errors first during message analysis
When an error occurred in a mux, most of time, an error is also reported on the
conn-stream, leading to an error (read and/or write) on the channel. When a
parsing or a processing error is reported for the HTX message, it is better to
handle it first.
diff --git a/src/http_ana.c b/src/http_ana.c
index 6fff39b..595f33b 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -98,11 +98,14 @@
htx = htxbuf(&req->buf);
/* Parsing errors are caught here */
- if (htx->flags & HTX_FL_PARSING_ERROR) {
+ if (htx->flags & (HTX_FL_PARSING_ERROR|HTX_FL_PROCESSING_ERROR)) {
stream_inc_http_req_ctr(s);
stream_inc_http_err_ctr(s);
proxy_inc_fe_req_ctr(sess->fe);
- goto return_bad_req;
+ if (htx->flags & HTX_FL_PARSING_ERROR)
+ goto return_bad_req;
+ else
+ goto return_int_err;
}
/* we're speaking HTTP here, so let's speak HTTP to the client */
@@ -349,7 +352,6 @@
if (ret) {
/* we fail this request, let's return 503 service unavail */
txn->status = 503;
- http_reply_and_close(s, txn->status, http_error_message(s));
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_LOCAL; /* we don't want a real error here */
goto return_prx_cond;
@@ -358,7 +360,6 @@
/* nothing to fail, let's reply normally */
txn->status = 200;
- http_reply_and_close(s, txn->status, http_error_message(s));
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_LOCAL; /* we don't want a real error here */
goto return_prx_cond;
@@ -431,16 +432,29 @@
return 1;
+ return_int_err:
+ txn->status = 500;
+ txn->req.err_state = txn->req.msg_state;
+ txn->req.msg_state = HTTP_MSG_ERROR;
+ if (!(s->flags & SF_ERR_MASK))
+ s->flags |= SF_ERR_INTERNAL;
+ _HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1);
+ if (sess->listener->counters)
+ _HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1);
+ goto return_prx_cond;
+
return_bad_req:
txn->status = 400;
txn->req.err_state = txn->req.msg_state;
txn->req.msg_state = HTTP_MSG_ERROR;
- http_reply_and_close(s, txn->status, http_error_message(s));
_HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1);
if (sess->listener->counters)
_HA_ATOMIC_ADD(&sess->listener->counters->failed_req, 1);
+ /* fall through */
return_prx_cond:
+ http_reply_and_close(s, txn->status, http_error_message(s));
+
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_PRXCOND;
if (!(s->flags & SF_FINST_MASK))
@@ -1016,6 +1030,8 @@
if (htx->flags & HTX_FL_PARSING_ERROR)
goto return_bad_req;
+ if (htx->flags & HTX_FL_PROCESSING_ERROR)
+ goto return_int_err;
if (msg->msg_state < HTTP_MSG_BODY)
goto missing_data;
@@ -1043,7 +1059,6 @@
missing_data:
if ((req->flags & CF_READ_TIMEOUT) || tick_is_expired(req->analyse_exp, now_ms)) {
txn->status = 408;
- http_reply_and_close(s, txn->status, http_error_message(s));
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_CLITO;
@@ -1073,18 +1088,31 @@
req->analyse_exp = TICK_ETERNITY;
return 1;
+ return_int_err:
+ txn->req.err_state = txn->req.msg_state;
+ txn->req.msg_state = HTTP_MSG_ERROR;
+ txn->status = 500;
+
+ if (!(s->flags & SF_ERR_MASK))
+ s->flags |= SF_ERR_INTERNAL;
+ if (!(s->flags & SF_FINST_MASK))
+ s->flags |= SF_FINST_R;
+ goto return_err_msg;
+
return_bad_req: /* let's centralize all bad requests */
txn->req.err_state = txn->req.msg_state;
txn->req.msg_state = HTTP_MSG_ERROR;
txn->status = 400;
- http_reply_and_close(s, txn->status, http_error_message(s));
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_PRXCOND;
if (!(s->flags & SF_FINST_MASK))
s->flags |= SF_FINST_R;
+ /* fall through */
return_err_msg:
+ http_reply_and_close(s, txn->status, http_error_message(s));
+
req->analysers &= AN_REQ_FLT_END;
_HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1);
if (sess->listener->counters)
@@ -1122,11 +1150,17 @@
htx = htxbuf(&req->buf);
+ if (htx->flags & HTX_FL_PARSING_ERROR)
+ goto return_bad_req;
+ if (htx->flags & HTX_FL_PROCESSING_ERROR)
+ goto return_int_err;
+
if ((req->flags & (CF_READ_ERROR|CF_READ_TIMEOUT|CF_WRITE_ERROR|CF_WRITE_TIMEOUT)) ||
((req->flags & CF_SHUTW) && (req->to_forward || co_data(req)))) {
/* Output closed while we were sending data. We must abort and
* wake the other side up.
*/
+
/* Don't abort yet if we had L7 retries activated and it
* was a write error, we may recover.
*/
@@ -1261,9 +1295,6 @@
if (req->flags & CF_SHUTW)
goto return_srv_abort;
- if (htx->flags & HTX_FL_PARSING_ERROR)
- goto return_bad_req;
-
/* When TE: chunked is used, we need to get there again to parse remaining
* chunks even if the client has closed, so we don't want to set CF_DONTCLOSE.
* And when content-length is used, we never want to let the possible
@@ -1308,6 +1339,12 @@
status = 502;
goto return_error;
+ return_int_err:
+ if (!(s->flags & SF_ERR_MASK))
+ s->flags |= SF_ERR_INTERNAL;
+ status = 500;
+ goto return_error;
+
return_bad_req:
_HA_ATOMIC_ADD(&sess->fe->fe_counters.failed_req, 1);
if (sess->listener->counters)
@@ -1415,6 +1452,8 @@
/* Parsing errors are caught here */
if (htx->flags & HTX_FL_PARSING_ERROR)
goto return_bad_res;
+ if (htx->flags & HTX_FL_PROCESSING_ERROR)
+ goto return_int_err;
/*
* Now we quickly check if we have found a full valid response.
@@ -1752,7 +1791,14 @@
channel_auto_close(rep);
return 1;
- return_bad_res:
+ return_int_err:
+ _HA_ATOMIC_ADD(&s->be->be_counters.failed_resp, 1);
+ txn->status = 500;
+ if (!(s->flags & SF_ERR_MASK))
+ s->flags |= SF_ERR_INTERNAL;
+ goto return_error;
+
+ return_bad_res:
_HA_ATOMIC_ADD(&s->be->be_counters.failed_resp, 1);
if (objt_server(s->target)) {
_HA_ATOMIC_ADD(&__objt_server(s->target)->counters.failed_resp, 1);
@@ -1763,14 +1809,19 @@
do_l7_retry(s, si_b) == 0)
return 0;
txn->status = 502;
- s->si[1].flags |= SI_FL_NOLINGER;
+ /* fall through */
+
+ return_error:
http_reply_and_close(s, txn->status, http_error_message(s));
- rep->analysers &= AN_RES_FLT_END;
if (!(s->flags & SF_ERR_MASK))
s->flags |= SF_ERR_PRXCOND;
if (!(s->flags & SF_FINST_MASK))
s->flags |= SF_FINST_H;
+
+ s->si[1].flags |= SI_FL_NOLINGER;
+ rep->analysers &= AN_RES_FLT_END;
+ rep->analyse_exp = TICK_ETERNITY;
return 0;
abort_keep_alive:
@@ -2105,6 +2156,11 @@
htx = htxbuf(&res->buf);
+ if (htx->flags & HTX_FL_PARSING_ERROR)
+ goto return_bad_res;
+ if (htx->flags & HTX_FL_PROCESSING_ERROR)
+ goto return_int_err;
+
if ((res->flags & (CF_READ_ERROR|CF_READ_TIMEOUT|CF_WRITE_ERROR|CF_WRITE_TIMEOUT)) ||
((res->flags & CF_SHUTW) && (res->to_forward || co_data(res)))) {
/* Output closed while we were sending data. We must abort and
@@ -2208,9 +2264,6 @@
if (res->flags & CF_SHUTW)
goto return_cli_abort;
- if (htx->flags & HTX_FL_PARSING_ERROR)
- goto return_bad_res;
-
/* stop waiting for data if the input is closed before the end. If the
* client side was already closed, it means that the client has aborted,
* so we don't want to count this as a server abort. Otherwise it's a
@@ -2265,6 +2318,12 @@
s->flags |= SF_ERR_CLICL;
goto return_error;
+ return_int_err:
+ _HA_ATOMIC_ADD(&s->be->be_counters.failed_resp, 1);
+ if (!(s->flags & SF_ERR_MASK))
+ s->flags |= SF_ERR_INTERNAL;
+ goto return_error;
+
return_bad_res:
_HA_ATOMIC_ADD(&s->be->be_counters.failed_resp, 1);
if (objt_server(s->target)) {