From: Christopher Faulet Date: Tue, 9 Apr 2024 06:19:01 +0000 (+0200) Subject: BUG/MEDIUM: http-ana: Deliver 502 on keep-alive for fressh server connection X-Git-Tag: v3.0-dev8~102 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3fc38593d513e14674d15f72d5831c39da90e8ef;p=thirdparty%2Fhaproxy.git BUG/MEDIUM: http-ana: Deliver 502 on keep-alive for fressh server connection In HTTP keep-alive, if we face a connection error to the server while sending the request, the error should not be reported, and the client-side connection should simply be closed, so that client knows it can retry. If the error happens during the connection stage, there is two cases. We have a connection timeout or an allocation error. In this case, the 503 response must be skipped if it is not the first request on the client-side connection. Or we have a connection error. In this case, the 503 response must be skipped if it is a reused server connection. Otherwise, during the connection stage, the 503-Service-unavailable response is delivered to the client. The part works properly. If the error happens after this stage, the 502-Bad-gateway response delivering should only be based on the server-side connection status. For a reused server connection, the client-side connection must be closed with no reponses. However, for a fresh server-side connection, a 502-Bad-gateway response must be delivered to the client. Unfortunately, This part is buggy. Only the client-side connection state is considered and the response is skipped if it is not the first request for the same client connection. The bug is not so visbile in HTTP/1.1 but in H2 and H3 it is pretty annoying because for a connection, requests are multiplexed, in parallels. It means there is no first request. So, because of this bug, for H2 and H3, 502-Bad-gateway responses because of a connection error before receiveing the response are always skipped. To fix the issue, in http_wait_for_response() analyser, we must only rely on SF_SRV_REUSED stream flag to skip the 502 response or not. This flag is set if the server connection was reused. The bug is their since a while. SF_SRV_REUSED flag was added in the version 1.5 especially to fix this kind of bug. But only the 503 case was fixed. This patch should fix the issue #2285. It must be backported to every stable versions. --- diff --git a/src/http_ana.c b/src/http_ana.c index 5249b557f4..4c1ff98eb4 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -1241,7 +1241,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) return 0; } - if (txn->flags & TX_NOT_FIRST) + if (s->flags & SF_SRV_REUSED) goto abort_keep_alive; _HA_ATOMIC_INC(&s->be->be_counters.failed_resp); @@ -1335,7 +1335,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) } } - if (txn->flags & TX_NOT_FIRST) + if (s->flags & SF_SRV_REUSED) goto abort_keep_alive; _HA_ATOMIC_INC(&s->be->be_counters.failed_resp); @@ -1360,7 +1360,7 @@ int http_wait_for_response(struct stream *s, struct channel *rep, int an_bit) /* 5: write error to client (we don't send any message then) */ else if (sc_ep_test(s->scf, SE_FL_ERR_PENDING)) { - if (txn->flags & TX_NOT_FIRST) + if (s->flags & SF_SRV_REUSED) goto abort_keep_alive; _HA_ATOMIC_INC(&s->be->be_counters.failed_resp);