From: Willy Tarreau Date: Wed, 20 Dec 2017 15:56:50 +0000 (+0100) Subject: BUG/MEDIUM: stream: don't consider abortonclose on muxes which close cleanly X-Git-Tag: v1.9-dev1~560 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7aa15b072ea752399144c8193a85c55e26893591;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: stream: don't consider abortonclose on muxes which close cleanly The H2 mux can cleanly report an error when a client closes, which is not the case for the pass-through mux which only reports shutr. That was the reason why "option abortonclose" was created since there was no way to distinguish a clean shutdown after sending the request from an abort. The problem is that in case of H2, the streams are always shut read after the request is complete (when the END_STREAM flag is received), and that when this lands on a backend configured with "option abortonclose", this aborts the request. Disabling abortonclose is not always an option when H1 and H2 have to coexist. This patch makes use of the newly introduced mux capabilities reported via the stream interface's SI_FL_CLEAN_ABRT indicating that the mux is safe and that there is no need to turn a clean shutread into an abort. This way abortonclose has no effect on requests initiated from an H2 mux. This patch as well as these 3 previous ones need to be backported to 1.8 : - BUG/MINOR: h2: properly report a stream error on RST_STREAM - MINOR: mux: add flags to describe a mux's capabilities - MINOR: stream-int: set flag SI_FL_CLEAN_ABRT when mux supports clean aborts --- diff --git a/src/proto_http.c b/src/proto_http.c index 481688fdf6..c226d37bd9 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -2708,7 +2708,10 @@ resume_execution: } case ACT_CUSTOM: - if ((px->options & PR_O_ABRT_CLOSE) && (s->req.flags & (CF_SHUTR|CF_READ_NULL|CF_READ_ERROR))) + if ((s->req.flags & CF_READ_ERROR) || + ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) && + !(s->si[0].flags & SI_FL_CLEAN_ABRT) && + (px->options & PR_O_ABRT_CLOSE))) act_flags |= ACT_FLAG_FINAL; switch (rule->action_ptr(rule, px, s->sess, s, act_flags)) { @@ -3069,7 +3072,10 @@ resume_execution: break; case ACT_CUSTOM: - if ((px->options & PR_O_ABRT_CLOSE) && (s->req.flags & (CF_SHUTR|CF_READ_NULL|CF_READ_ERROR))) + if ((s->req.flags & CF_READ_ERROR) || + ((s->req.flags & (CF_SHUTR|CF_READ_NULL)) && + !(s->si[0].flags & SI_FL_CLEAN_ABRT) && + (px->options & PR_O_ABRT_CLOSE))) act_flags |= ACT_FLAG_FINAL; switch (rule->action_ptr(rule, px, s->sess, s, act_flags)) { @@ -4435,7 +4441,8 @@ int http_sync_req_state(struct stream *s) */ if (((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_SCL) && ((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_KAL) && - !(s->be->options & PR_O_ABRT_CLOSE) && + (!(s->be->options & PR_O_ABRT_CLOSE) || + (s->si[0].flags & SI_FL_CLEAN_ABRT)) && txn->meth != HTTP_METH_POST) channel_dont_read(chn); @@ -4530,7 +4537,8 @@ int http_sync_req_state(struct stream *s) /* see above in MSG_DONE why we only do this in these states */ if (((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_SCL) && ((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_KAL) && - !(s->be->options & PR_O_ABRT_CLOSE)) + (!(s->be->options & PR_O_ABRT_CLOSE) || + (s->si[0].flags & SI_FL_CLEAN_ABRT))) channel_dont_read(chn); goto wait_other_side; } @@ -4894,7 +4902,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) * server, which will decide whether to close or to go on processing the * request. We only do that in tunnel mode, and not in other modes since * it can be abused to exhaust source ports. */ - if (s->be->options & PR_O_ABRT_CLOSE) { + if ((s->be->options & PR_O_ABRT_CLOSE) && !(s->si[0].flags & SI_FL_CLEAN_ABRT)) { channel_auto_read(req); if ((req->flags & (CF_SHUTR|CF_READ_NULL)) && ((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN)) diff --git a/src/stream.c b/src/stream.c index ba5dbffee6..39ee9ba558 100644 --- a/src/stream.c +++ b/src/stream.c @@ -622,7 +622,7 @@ static int sess_update_st_con_tcp(struct stream *s) unlikely((rep->flags & CF_SHUTW) || ((req->flags & CF_SHUTW_NOW) && /* FIXME: this should not prevent a connection from establishing */ ((!(req->flags & (CF_WRITE_ACTIVITY|CF_WRITE_EVENT)) && channel_is_empty(req)) || - s->be->options & PR_O_ABRT_CLOSE)))) { + ((s->be->options & PR_O_ABRT_CLOSE) && !(s->si[0].flags & SI_FL_CLEAN_ABRT)))))) { /* give up */ si_shutw(si); si->err_type |= SI_ET_CONN_ABRT; @@ -834,7 +834,8 @@ static int check_req_may_abort(struct channel *req, struct stream *s) { return ((req->flags & (CF_READ_ERROR)) || ((req->flags & CF_SHUTW_NOW) && /* empty and client aborted */ - (channel_is_empty(req) || s->be->options & PR_O_ABRT_CLOSE))); + (channel_is_empty(req) || + ((s->be->options & PR_O_ABRT_CLOSE) && !(s->si[0].flags & SI_FL_CLEAN_ABRT))))); } /* Update back stream interface status for input states SI_ST_ASS, SI_ST_QUE,